From 815e49bb7c005ebfe4b4c356b64de53cb074c3ad Mon Sep 17 00:00:00 2001 From: scotttong Date: Tue, 11 Aug 2026 09:06:21 -0700 Subject: [PATCH] feat: make chat-style tasks the default experience (#11101) --- .../adapter-utils/src/server-utils.test.ts | 3 + packages/adapter-utils/src/server-utils.ts | 1 + .../claude-local/src/server/acp.test.ts | 9 +- .../0212_onboarding_first_task_unique.sql | 3 + packages/db/src/migrations/meta/_journal.json | 7 + packages/db/src/schema/issues.ts | 7 + packages/shared/src/constants.ts | 5 + packages/shared/src/feature-catalog.ts | 6 +- packages/shared/src/index.ts | 1 + packages/shared/src/types/instance.ts | 2 +- packages/shared/src/types/issue.ts | 13 +- packages/shared/src/validators/instance.ts | 2 +- packages/shared/src/validators/issue.ts | 35 +- server/src/__tests__/built-in-agents.test.ts | 56 +- .../src/__tests__/companies-service.test.ts | 29 +- .../instance-settings-service.test.ts | 16 +- ...issue-onboarding-first-task-routes.test.ts | 168 ++++ .../issue-thread-interactions-service.test.ts | 185 +++++ server/src/routes/issues.ts | 110 ++- server/src/services/built-in-agents.ts | 16 + server/src/services/instance-settings.ts | 4 +- .../src/services/issue-thread-interactions.ts | 30 +- .../src/services/onboarding-greeting.test.ts | 43 ++ server/src/services/onboarding-greeting.ts | 39 + .../e2e/conference-room-typing-intro.spec.ts | 228 +++--- tests/e2e/helpers/onboarding-landing.ts | 80 ++ tests/e2e/nux-phase4-screenshots.spec.ts | 4 +- tests/e2e/onboarding.spec.ts | 6 +- .../planning-mode-visual-verification.spec.ts | 31 +- ui/src/App.tsx | 9 +- ui/src/components/IssueChatThread.tsx | 19 +- ui/src/components/IssueProperties.test.tsx | 2 +- .../IssueThreadInteractionCard.test.tsx | 191 ++++- .../components/IssueThreadInteractionCard.tsx | 731 ++++++++++-------- ui/src/components/OnboardingWizard.tsx | 121 +-- ui/src/components/PropertiesPanel.test.tsx | 8 +- ui/src/components/PropertiesPanel.tsx | 10 +- ui/src/components/SidebarCompanyMenu.test.tsx | 10 +- ui/src/components/SidebarCompanyMenu.tsx | 23 +- .../components/TaskChatRedesignGate.test.tsx | 95 --- ui/src/components/TaskChatRedesignGate.tsx | 18 - ui/src/components/TaskChatThread.test.tsx | 115 ++- ui/src/components/TaskChatThread.tsx | 254 ++++-- .../IssuePlanConfirmationActionBar.test.tsx | 117 --- .../IssuePlanConfirmationActionBar.tsx | 216 ------ .../issue-properties/IssueProperties.tsx | 44 +- .../IssuePropertiesArtifactsTab.tsx | 2 +- .../IssuePropertiesPlansTab.tsx | 59 +- .../task-chat/TaskChatBubbleActions.tsx | 34 +- .../TaskChatInteractionCard.test.tsx | 12 +- .../task-chat/TaskChatLiveRunPill.test.tsx | 92 +++ .../task-chat/TaskChatLiveRunPill.tsx | 88 +++ .../task-chat/TaskChatLiveTail.test.tsx | 138 ++++ .../components/task-chat/TaskChatLiveTail.tsx | 86 +++ .../task-chat/TaskChatThreadView.tsx | 13 +- .../interaction-thread-order.test.ts | 125 +++ .../task-chat/interaction-thread-order.ts | 79 ++ .../components/task-chat/task-chat-model.ts | 4 +- .../components/task-chat/task-chat-states.ts | 4 +- .../transcript/RunTranscriptView.test.tsx | 46 +- .../transcript/RunTranscriptView.tsx | 83 +- .../transcript/useLiveRunTranscripts.test.tsx | 108 +++ .../transcript/useLiveRunTranscripts.ts | 99 ++- .../issueThreadInteractionFixtures.ts | 47 +- ...d.ts => useClassicTaskInterfaceEnabled.ts} | 19 +- ui/src/index.css | 35 +- ui/src/lib/ceo-instructions.ts | 2 +- ui/src/lib/issue-chat-messages.test.ts | 70 ++ ui/src/lib/issue-chat-messages.ts | 6 + ui/src/lib/issue-thread-interactions.test.ts | 218 +++++- ui/src/lib/issue-thread-interactions.ts | 78 ++ ui/src/lib/onboarding-launch.test.ts | 2 + ui/src/lib/onboarding-launch.ts | 3 + ui/src/lib/run-log-chunks.test.ts | 78 ++ ui/src/lib/run-log-chunks.ts | 124 ++- .../InstanceExperimentalSettings.test.tsx | 24 +- ui/src/pages/InstanceExperimentalSettings.tsx | 16 +- ui/src/pages/IssueDetail.test.tsx | 341 ++++---- ui/src/pages/IssueDetail.tsx | 94 ++- ui/src/pages/TaskChatLab.tsx | 4 +- 80 files changed, 3858 insertions(+), 1497 deletions(-) create mode 100644 packages/db/src/migrations/0212_onboarding_first_task_unique.sql create mode 100644 server/src/__tests__/issue-onboarding-first-task-routes.test.ts create mode 100644 server/src/services/onboarding-greeting.test.ts create mode 100644 server/src/services/onboarding-greeting.ts create mode 100644 tests/e2e/helpers/onboarding-landing.ts delete mode 100644 ui/src/components/TaskChatRedesignGate.test.tsx delete mode 100644 ui/src/components/TaskChatRedesignGate.tsx delete mode 100644 ui/src/components/issue-properties/IssuePlanConfirmationActionBar.test.tsx delete mode 100644 ui/src/components/issue-properties/IssuePlanConfirmationActionBar.tsx create mode 100644 ui/src/components/task-chat/TaskChatLiveRunPill.test.tsx create mode 100644 ui/src/components/task-chat/TaskChatLiveRunPill.tsx create mode 100644 ui/src/components/task-chat/TaskChatLiveTail.test.tsx create mode 100644 ui/src/components/task-chat/TaskChatLiveTail.tsx create mode 100644 ui/src/components/task-chat/interaction-thread-order.test.ts create mode 100644 ui/src/components/task-chat/interaction-thread-order.ts rename ui/src/hooks/{useTaskChatRedesignEnabled.ts => useClassicTaskInterfaceEnabled.ts} (62%) diff --git a/packages/adapter-utils/src/server-utils.test.ts b/packages/adapter-utils/src/server-utils.test.ts index 2dd38ccdf4..81ec8458b5 100644 --- a/packages/adapter-utils/src/server-utils.test.ts +++ b/packages/adapter-utils/src/server-utils.test.ts @@ -887,6 +887,9 @@ describe("renderPaperclipWakePrompt", () => { expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).not.toContain( "for request_confirmation this resumes only after acceptance", ); + expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain( + "Never create probe or throwaway issue-thread interactions to discover the interactions API shape or your permissions", + ); expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("confirmation:{issueId}:plan:{revisionId}"); expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("Wait for acceptance before creating implementation subtasks"); expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain( diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index 6c51a3cd32..7f4cf5cd70 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -170,6 +170,7 @@ export const DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE = [ "- Create child issues directly when you know what needs to be done; use issue-thread interactions when the board/user must choose suggested tasks, answer structured questions, or confirm a proposal.", "- Use `PAPERCLIP_SCRATCH_DIR` / `PAPERCLIP_RUN_SCRATCH_DIR` for temporary scratch files instead of ad hoc `/tmp` paths; Paperclip removes that run-owned directory after the run ends.", "- To ask for that input, create an interaction on the current issue with POST /api/issues/{issueId}/interactions using kind suggest_tasks, ask_user_questions, or request_confirmation. Use continuationPolicy wake_assignee when you need to resume after a response (it wakes on acceptance and rejection alike; only expiry does not wake); use wake_assignee_on_accept when you want to resume only after acceptance.", + "- Never create probe or throwaway issue-thread interactions to discover the interactions API shape or your permissions; schema discovery goes through the OpenAPI spec and explicit validation errors, not placeholder cards. Every ask_user_questions, suggest_tasks, or request_confirmation you post must carry a real, answerable prompt; withdraw one you no longer need instead of leaving it pending.", "- When you intentionally restart follow-up work on a completed assigned issue, include structured `resume: true` with the POST /api/issues/{issueId}/comments or PATCH /api/issues/{issueId} comment payload. Generic agent comments on closed issues are inert by default.", "- For plan approval, update the plan document first, then create request_confirmation targeting the latest plan revision with idempotencyKey confirmation:{issueId}:plan:{revisionId}. Wait for acceptance before creating implementation subtasks, and create a fresh confirmation after superseding board/user comments if approval is still needed.", "- If blocked, mark the issue blocked and name the unblock owner and action.", diff --git a/packages/adapters/claude-local/src/server/acp.test.ts b/packages/adapters/claude-local/src/server/acp.test.ts index cf988297a9..45acb00db0 100644 --- a/packages/adapters/claude-local/src/server/acp.test.ts +++ b/packages/adapters/claude-local/src/server/acp.test.ts @@ -85,7 +85,14 @@ afterEach(async () => { if (value === undefined) delete process.env[key]; else process.env[key] = value; } - await Promise.all(tempRoots.splice(0).map((root) => fs.rm(root, { recursive: true, force: true }))); + // The sandbox process-session bridge writes event files asynchronously; on slow + // CI shards a final write can race the recursive rm (ENOTEMPTY on the events + // dir), so let fs.rm retry until the writer has quiesced. + await Promise.all( + tempRoots + .splice(0) + .map((root) => fs.rm(root, { recursive: true, force: true, maxRetries: 10, retryDelay: 200 })), + ); }); class FakeRuntime { diff --git a/packages/db/src/migrations/0212_onboarding_first_task_unique.sql b/packages/db/src/migrations/0212_onboarding_first_task_unique.sql new file mode 100644 index 0000000000..405c57c9e0 --- /dev/null +++ b/packages/db/src/migrations/0212_onboarding_first_task_unique.sql @@ -0,0 +1,3 @@ +CREATE UNIQUE INDEX IF NOT EXISTS "issues_onboarding_first_task_uq" + ON "issues" USING btree ("company_id") + WHERE "origin_kind" = 'onboarding_first_task'; diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json index ed4a7ade92..395fb9a1ff 100644 --- a/packages/db/src/migrations/meta/_journal.json +++ b/packages/db/src/migrations/meta/_journal.json @@ -1471,6 +1471,13 @@ "when": 1786129601533, "tag": "0211_bright_morg", "breakpoints": true + }, + { + "idx": 212, + "version": "7", + "when": 1786388759523, + "tag": "0212_onboarding_first_task_unique", + "breakpoints": true } ] } diff --git a/packages/db/src/schema/issues.ts b/packages/db/src/schema/issues.ts index ae40befba9..55b3ed3493 100644 --- a/packages/db/src/schema/issues.ts +++ b/packages/db/src/schema/issues.ts @@ -168,5 +168,12 @@ export const issues = pgTable( and ${table.hiddenAt} is null and ${table.status} not in ('done', 'cancelled')`, ), + // The onboarding first-task origin grants privileged behavior (agent-attributed + // greeting, description suppression), so at most one issue per company may ever + // carry it — concurrent creates race on the pre-insert count check and this + // index is what atomically rejects the loser. + onboardingFirstTaskIdx: uniqueIndex("issues_onboarding_first_task_uq") + .on(table.companyId) + .where(sql`${table.originKind} = 'onboarding_first_task'`), }), ); diff --git a/packages/shared/src/constants.ts b/packages/shared/src/constants.ts index 75c16a1536..bcce7f48db 100644 --- a/packages/shared/src/constants.ts +++ b/packages/shared/src/constants.ts @@ -300,6 +300,10 @@ export type IssueThreadInteractionContinuationPolicy = export const TASK_WATCHDOG_PRODUCT_BUG_ORIGIN_KIND = "task_watchdog_product_bug"; +// Marks the single onboarding "first task" so surfaces can special-case it +// (e.g. suppress the seeded-description bubble and rely on a seeded greeting). +export const ONBOARDING_FIRST_TASK_ORIGIN_KIND = "onboarding_first_task"; + export const ISSUE_ORIGIN_KINDS = [ "manual", "routine_execution", @@ -309,6 +313,7 @@ export const ISSUE_ORIGIN_KINDS = [ "stranded_issue_recovery", "task_watchdog", TASK_WATCHDOG_PRODUCT_BUG_ORIGIN_KIND, + ONBOARDING_FIRST_TASK_ORIGIN_KIND, ] as const; export type BuiltInIssueOriginKind = (typeof ISSUE_ORIGIN_KINDS)[number]; export type PluginIssueOriginKind = `plugin:${string}`; diff --git a/packages/shared/src/feature-catalog.ts b/packages/shared/src/feature-catalog.ts index 9916d916f6..986b1f6922 100644 --- a/packages/shared/src/feature-catalog.ts +++ b/packages/shared/src/feature-catalog.ts @@ -96,10 +96,10 @@ export const INSTANCE_FEATURE_CATALOG: Record; @@ -759,6 +770,12 @@ export const askUserQuestionsQuestionOptionSchema = z.object({ id: z.string().trim().min(1).max(120), label: z.string().trim().min(1).max(120), description: z.string().trim().max(500).nullable().optional(), + freeText: z + .boolean() + .optional() + .describe( + "When true, selecting this option reveals an inline text field; the typed value is returned as the question's otherText. Use this for a real \"I'll describe it\" choice instead of authoring a dead option that does nothing. At most one free-text option per question.", + ), }); export const askUserQuestionsQuestionSchema = z.object({ @@ -789,6 +806,7 @@ export const askUserQuestionsPayloadSchema = z.object({ seenQuestionIds.add(question.id); const seenOptionIds = new Set(); + let freeTextOptionCount = 0; for (const [optionIndex, option] of question.options.entries()) { if (seenOptionIds.has(option.id)) { ctx.addIssue({ @@ -798,6 +816,16 @@ export const askUserQuestionsPayloadSchema = z.object({ }); } seenOptionIds.add(option.id); + if (option.freeText) { + freeTextOptionCount += 1; + if (freeTextOptionCount > 1) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: "A question may declare at most one free-text option", + path: ["questions", questionIndex, "options", optionIndex, "freeText"], + }); + } + } } } }); @@ -815,8 +843,11 @@ export const askUserQuestionsResultSchema = z.object({ answers: z.array(askUserQuestionsAnswerSchema).max(20), cancelled: z.literal(true).optional(), cancellationReason: z.string().trim().max(4000).nullable().optional(), - expirationReason: z.literal("superseded_by_comment").optional(), + expirationReason: z.enum(["superseded_by_comment", "superseded_by_newer_interaction"]).optional(), commentId: z.string().uuid().nullable().optional(), + // Set alongside expirationReason "superseded_by_newer_interaction": the id of + // the newer sibling ask_user_questions that replaced this one (PAP-437). + supersededByInteractionId: z.string().uuid().nullable().optional(), summaryMarkdown: z.string().max(20000).nullable().optional(), }); diff --git a/server/src/__tests__/built-in-agents.test.ts b/server/src/__tests__/built-in-agents.test.ts index 5ab8b6dde0..880c53b30d 100644 --- a/server/src/__tests__/built-in-agents.test.ts +++ b/server/src/__tests__/built-in-agents.test.ts @@ -569,7 +569,7 @@ describeEmbeddedPostgres("built-in agents", () => { }); }); - it("auto-provisions a paused Reflection Coach bundle with skill sync and a disabled routine", async () => { + it("reconciles an enabled Reflection Coach bundle with skill sync and a disabled routine", async () => { const companyId = await seedCompany({ requireApproval: false }); const root = await agentService(db).create(companyId, { name: "CEO", @@ -581,6 +581,17 @@ describeEmbeddedPostgres("built-in agents", () => { permissions: {}, }); + // The Reflection Coach is opt-in (not auto-created). Enabling it on demand + // materializes its managed bundle in a single pass. + const enabled = await builtInAgentService(db).ensure(companyId, "reflection-coach"); + expect(enabled.agent?.adapterConfig).toMatchObject({ + instructionsBundleMode: "managed", + instructionsEntryFile: "AGENTS.md", + }); + expect(enabled.agent?.adapterConfig).not.toMatchObject({ model: "gpt-5.4", apiKey: "do-not-copy" }); + + // Startup reconcile keeps the enabled bundle tracking stock and re-grants + // the root/company default permissions. const result = await reconcileBuiltInAgentsOnStartup(db); expect(result.autoEnsured).toBeGreaterThanOrEqual(1); expect(result.defaultGrantsEnsured).toBeGreaterThanOrEqual(4); @@ -606,11 +617,6 @@ describeEmbeddedPostgres("built-in agents", () => { }, }, }); - expect(state.agent?.adapterConfig).toMatchObject({ - instructionsBundleMode: "managed", - instructionsEntryFile: "AGENTS.md", - }); - expect(state.agent?.adapterConfig).not.toMatchObject({ model: "gpt-5.4", apiKey: "do-not-copy" }); expect(state.resources.map((resource) => [resource.resourceKind, resource.stockStatus])).toEqual([ ["instructions", "stock_current"], ["skill", "stock_current"], @@ -693,7 +699,7 @@ describeEmbeddedPostgres("built-in agents", () => { )).size).toBe(3); }); - it("preserves new-agent approval gates during automatic Reflection Coach provisioning", async () => { + it("preserves new-agent approval gates during on-demand Reflection Coach provisioning", async () => { const companyId = await seedCompany({ requireApproval: true }); const root = await agentService(db).create(companyId, { name: "CEO", @@ -710,12 +716,11 @@ describeEmbeddedPostgres("built-in agents", () => { applyInSeparateFollowUpRun: true, }; - const result = await reconcileBuiltInAgentsOnStartup(db); - - expect(result).toMatchObject({ - autoEnsured: 2, - pendingApprovals: 2, - }); + // The Reflection Coach is opt-in, so it's enabled on demand. With board + // approval required, provisioning it must leave a pending agent + a + // hire_agent approval rather than an active agent. + const provisioned = await builtInAgentService(db).provision(companyId, "reflection-coach"); + expect(provisioned.approval).not.toBeNull(); const state = await builtInAgentService(db).get(companyId, "reflection-coach"); expect(state).toMatchObject({ status: "pending_approval", @@ -751,7 +756,7 @@ describeEmbeddedPostgres("built-in agents", () => { }); const pendingReconcile = await reconcileBuiltInAgentsOnStartup(db); - expect(pendingReconcile.pendingApprovals).toBe(2); + expect(pendingReconcile.pendingApprovals).toBe(1); const stillPending = await builtInAgentService(db).get(companyId, "reflection-coach"); expect(stillPending).toMatchObject({ status: "pending_approval", @@ -787,7 +792,7 @@ describeEmbeddedPostgres("built-in agents", () => { const agentRows = await db.select().from(agents).where(eq(agents.companyId, companyId)); expect(agentRows.filter((row) => readBuiltInAgentMarker(row.metadata)?.key === "reflection-coach")).toHaveLength(1); const approvalRows = await db.select().from(approvals).where(eq(approvals.companyId, companyId)); - expect(approvalRows).toHaveLength(2); + expect(approvalRows).toHaveLength(1); }); it("preserves Reflection Coach instruction drift on reconcile and restores it on reset", async () => { @@ -1062,7 +1067,21 @@ describeEmbeddedPostgres("built-in agents", () => { const { olderId, newerId } = await seedLegacyDuplicateBriefs(affectedCompanyId); // A second company created after the affected one — previously skipped // entirely because the duplicate error escaped the reconciliation loop. + // Give it a drifted built-in row so we can prove reconcile still reached it. const healthyCompanyId = await seedCompany({ requireApproval: false }); + const healthyBriefsId = randomUUID(); + await db.insert(agents).values({ + id: healthyBriefsId, + companyId: healthyCompanyId, + name: "Stale Briefs", + role: "engineer", + status: "idle", + adapterType: "codex_local", + adapterConfig: { model: "gpt-5.4" }, + runtimeConfig: {}, + permissions: {}, + metadata: withBuiltInAgentMarker({}, { key: "briefs", featureKeys: ["briefs"] }), + }); const result = await reconcileBuiltInAgentsOnStartup(db); expect(result.companyFailures).toBe(0); @@ -1077,9 +1096,10 @@ describeEmbeddedPostgres("built-in agents", () => { ), ).toHaveLength(1); - // The company after the affected one still had its bundled agents provisioned. - const healthyCoach = await builtInAgentService(db).get(healthyCompanyId, "reflection-coach"); - expect(healthyCoach.agentId).toBeTruthy(); + // The company after the affected one was still reconciled (its drifted + // built-in row was repaired to stock) rather than skipped. + const [healthyBriefs] = await db.select().from(agents).where(eq(agents.id, healthyBriefsId)); + expect(healthyBriefs?.name).toBe("Briefs Agent"); }); it("automatically materializes the Reflection Coach bundle without enabling background work", async () => { diff --git a/server/src/__tests__/companies-service.test.ts b/server/src/__tests__/companies-service.test.ts index c8b48ddb74..e3aa0c7ae4 100644 --- a/server/src/__tests__/companies-service.test.ts +++ b/server/src/__tests__/companies-service.test.ts @@ -24,7 +24,7 @@ import { } from "./helpers/embedded-postgres.js"; import { companyService } from "../services/companies.js"; import { readBuiltInAgentMarker } from "../services/built-in-agent-metadata.js"; -import { reconcileBuiltInAgentsOnStartup } from "../services/built-in-agents.js"; +import { builtInAgentService, reconcileBuiltInAgentsOnStartup } from "../services/built-in-agents.js"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; @@ -81,19 +81,28 @@ describeEmbeddedPostgres("companyService", () => { expect(rows.map((row) => row.issuePrefix).sort()).toEqual(["ARO", "AROA"]); }); - it("auto-provisions one paused Reflection Coach bundle for a freshly created company", async () => { + it("does not auto-provision bundled built-in agents for a freshly created company", async () => { const created = await companyService(db).create({ name: "Fresh Company", }); + // A new company starts clean: the Reflection Coach and Summarizer are + // opt-in, not seeded by default for a new user. const agentRows = await db.select().from(agents).where(eq(agents.companyId, created.id)); - const reflectionRows = agentRows.filter((row) => readBuiltInAgentMarker(row.metadata)?.key === "reflection-coach"); - expect(reflectionRows).toHaveLength(1); - expect(reflectionRows[0]).toMatchObject({ + expect(agentRows.filter((row) => readBuiltInAgentMarker(row.metadata))).toHaveLength(0); + + // Startup reconcile leaves a fresh company untouched — nothing is created. + await reconcileBuiltInAgentsOnStartup(db); + const afterReconcileRows = await db.select().from(agents).where(eq(agents.companyId, created.id)); + expect(afterReconcileRows.filter((row) => readBuiltInAgentMarker(row.metadata))).toHaveLength(0); + + // The Reflection Coach remains available to enable on demand, and enabling + // it materializes its bundled skill + paused routine. + const enabled = await builtInAgentService(db).ensure(created.id, "reflection-coach"); + expect(enabled.agent).toMatchObject({ name: "Reflection Coach", status: "paused", budgetMonthlyCents: 0, - spentMonthlyCents: 0, }); const [skill] = await db @@ -110,10 +119,10 @@ describeEmbeddedPostgres("companyService", () => { const [routine] = await db .select() .from(routines) - .where(and(eq(routines.companyId, created.id), eq(routines.assigneeAgentId, reflectionRows[0]!.id))); + .where(and(eq(routines.companyId, created.id), eq(routines.assigneeAgentId, enabled.agentId!))); expect(routine).toMatchObject({ status: "paused", - assigneeAgentId: reflectionRows[0]!.id, + assigneeAgentId: enabled.agentId, originKind: "built_in_agent_bundle", originId: "reflection-coach:recent-agent-reflection", }); @@ -122,10 +131,6 @@ describeEmbeddedPostgres("companyService", () => { kind: "schedule", enabled: false, }); - - await reconcileBuiltInAgentsOnStartup(db); - const afterReconcileRows = await db.select().from(agents).where(eq(agents.companyId, created.id)); - expect(afterReconcileRows.filter((row) => readBuiltInAgentMarker(row.metadata)?.key === "reflection-coach")).toHaveLength(1); }); it("archives companies by pausing runnable agents and cancelling active runs", async () => { diff --git a/server/src/__tests__/instance-settings-service.test.ts b/server/src/__tests__/instance-settings-service.test.ts index a31adbf4cb..342eacb0a0 100644 --- a/server/src/__tests__/instance-settings-service.test.ts +++ b/server/src/__tests__/instance-settings-service.test.ts @@ -29,7 +29,7 @@ describe("instance settings service", () => { enableStreamlinedLeftNavigation: true, enableApps: false, enableConferenceRoomChat: false, - enableTaskChatRedesign: false, + enableClassicTaskInterface: false, enableExternalObjects: false, enableSmokeLab: false, enablePipelines: false, @@ -72,6 +72,20 @@ describe("instance settings service", () => { ).toBe(false); }); + it("defaults enableClassicTaskInterface to false for empty and legacy stored settings", () => { + expect(normalizeExperimentalSettings(undefined).enableClassicTaskInterface).toBe(false); + expect(normalizeExperimentalSettings({}).enableClassicTaskInterface).toBe(false); + // The retired enableTaskChatRedesign key must not bleed into the new flag: + // an install that had the chat redesign ON opted into chat-style, which is + // now the default — not into the classic view. + expect( + normalizeExperimentalSettings({ enableTaskChatRedesign: true }).enableClassicTaskInterface, + ).toBe(false); + expect( + normalizeExperimentalSettings({ enableClassicTaskInterface: true }).enableClassicTaskInterface, + ).toBe(true); + }); + it("defaults enableSimplifiedEnglishInteractions to false for empty and legacy stored settings", () => { expect(normalizeExperimentalSettings(undefined).enableSimplifiedEnglishInteractions).toBe(false); expect(normalizeExperimentalSettings({}).enableSimplifiedEnglishInteractions).toBe(false); diff --git a/server/src/__tests__/issue-onboarding-first-task-routes.test.ts b/server/src/__tests__/issue-onboarding-first-task-routes.test.ts new file mode 100644 index 0000000000..01b3078ceb --- /dev/null +++ b/server/src/__tests__/issue-onboarding-first-task-routes.test.ts @@ -0,0 +1,168 @@ +import { randomUUID } from "node:crypto"; +import express from "express"; +import request from "supertest"; +import { and, eq } from "drizzle-orm"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + activityLog, + agents, + agentWakeupRequests, + companies, + createDb, + issueComments, + issues, +} from "@paperclipai/db"; +import { ONBOARDING_FIRST_TASK_ORIGIN_KIND } from "@paperclipai/shared"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; +import { actorMiddleware } from "../middleware/auth.js"; +import { errorHandler } from "../middleware/index.js"; +import { issueRoutes } from "../routes/issues.js"; + +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +if (!embeddedPostgresSupport.supported) { + console.warn( + `Skipping embedded Postgres onboarding first-task route tests on this host: ${ + embeddedPostgresSupport.reason ?? "unsupported environment" + }`, + ); +} + +describeEmbeddedPostgres("issue create onboarding first-task routes", () => { + let db!: ReturnType; + let tempDb: Awaited> | null = null; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-issue-onboarding-first-task-routes-"); + db = createDb(tempDb.connectionString); + }, 20_000); + + afterEach(async () => { + await db.delete(activityLog); + await db.delete(agentWakeupRequests); + await db.delete(issueComments); + await db.delete(issues); + await db.delete(agents); + await db.delete(companies); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + function createApp() { + const app = express(); + app.use(express.json()); + app.use(actorMiddleware(db, { deploymentMode: "local_trusted" })); + app.use("/api", issueRoutes(db, {} as any)); + app.use(errorHandler); + return app; + } + + async function seedCompany() { + const companyId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `D${companyId.replace(/-/g, "").slice(0, 5).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + return companyId; + } + + async function seedAgent(companyId: string) { + const agentId = randomUUID(); + await db.insert(agents).values({ + id: agentId, + companyId, + name: "CEO", + role: "engineer", + status: "running", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }); + return agentId; + } + + function listOnboardingIssues(companyId: string) { + return db + .select() + .from(issues) + .where(and( + eq(issues.companyId, companyId), + eq(issues.originKind, ONBOARDING_FIRST_TASK_ORIGIN_KIND), + )); + } + + it("stamps the onboarding origin and seeds the agent-attributed greeting on the first task", async () => { + const companyId = await seedCompany(); + const agentId = await seedAgent(companyId); + const app = createApp(); + + const created = await request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ title: "Get started", onboardingFirstTask: true, assigneeAgentId: agentId }) + .expect(201); + + expect(created.body.originKind).toBe(ONBOARDING_FIRST_TASK_ORIGIN_KIND); + const comments = await db + .select() + .from(issueComments) + .where(eq(issueComments.issueId, created.body.id)); + expect(comments).toHaveLength(1); + expect(comments[0]).toMatchObject({ authorType: "agent", authorAgentId: agentId }); + }); + + it("fails closed to an ordinary issue when the onboarding origin is already claimed", async () => { + const companyId = await seedCompany(); + const agentId = await seedAgent(companyId); + const app = createApp(); + // Simulate the losing side of the count-vs-create race: the winning issue + // already claimed the onboarding origin but is hidden, so the zero-count + // fast path still passes and only issues_onboarding_first_task_uq rejects + // the privileged insert. + await db.insert(issues).values({ + companyId, + title: "Race winner", + status: "todo", + priority: "medium", + originKind: ONBOARDING_FIRST_TASK_ORIGIN_KIND, + hiddenAt: new Date(), + }); + + const created = await request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ title: "Race loser", onboardingFirstTask: true, assigneeAgentId: agentId }) + .expect(201); + + expect(created.body.originKind).toBe("manual"); + const comments = await db + .select() + .from(issueComments) + .where(eq(issueComments.issueId, created.body.id)); + expect(comments).toHaveLength(0); + expect(await listOnboardingIssues(companyId)).toHaveLength(1); + }); + + it("allows at most one onboarding first task across concurrent creates", async () => { + const companyId = await seedCompany(); + const app = createApp(); + + const responses = await Promise.all( + ["Kick off A", "Kick off B", "Kick off C"].map((title) => + request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ title, onboardingFirstTask: true }), + ), + ); + + for (const response of responses) expect(response.status).toBe(201); + expect(await listOnboardingIssues(companyId)).toHaveLength(1); + }); +}); diff --git a/server/src/__tests__/issue-thread-interactions-service.test.ts b/server/src/__tests__/issue-thread-interactions-service.test.ts index 1403e642cb..3358935d1d 100644 --- a/server/src/__tests__/issue-thread-interactions-service.test.ts +++ b/server/src/__tests__/issue-thread-interactions-service.test.ts @@ -24,6 +24,7 @@ import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase, } from "./helpers/embedded-postgres.js"; +import { ONBOARDING_FIRST_TASK_ORIGIN_KIND } from "@paperclipai/shared"; import { instanceSettingsService } from "../services/instance-settings.js"; import { issueService } from "../services/issues.js"; import { issueThreadInteractionService } from "../services/issue-thread-interactions.js"; @@ -1262,6 +1263,190 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { expect(interactions.find((interaction) => interaction.id === otherKind.id)?.status).toBe("pending"); }); + it("supersedes an agent's own older pending ask_user_questions without crossing agent, kind, or issue", async () => { + const { companyId, goalId, issueId } = await seedConfirmationIssue("Question supersedes older sibling"); + const otherIssueId = randomUUID(); + await db.insert(issues).values({ + id: otherIssueId, + companyId, + goalId, + title: "Other issue", + status: "in_progress", + priority: "medium", + }); + + const probingAgentId = randomUUID(); + const otherAgentId = randomUUID(); + await db.insert(agents).values([ + { + id: probingAgentId, + companyId, + name: "Probing agent", + role: "engineer", + status: "active", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }, + { + id: otherAgentId, + companyId, + name: "Other agent", + role: "engineer", + status: "active", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }, + ]); + + const question = (prompt: string) => ({ + kind: "ask_user_questions" as const, + payload: { + version: 1 as const, + questions: [{ + id: "q", + prompt, + selectionMode: "single" as const, + options: [{ id: "opt", label: "Option" }], + }], + }, + }); + + const older = await interactionsSvc.create( + { id: issueId, companyId }, question("Older question"), { agentId: probingAgentId }, + ); + const otherKind = await interactionsSvc.create({ id: issueId, companyId }, { + kind: "request_confirmation", + payload: { version: 1, prompt: "Approve the draft?" }, + }, { agentId: probingAgentId }); + const otherAgentQuestion = await interactionsSvc.create( + { id: issueId, companyId }, question("Other agent question"), { agentId: otherAgentId }, + ); + const otherIssueQuestion = await interactionsSvc.create( + { id: otherIssueId, companyId }, question("Other issue question"), { agentId: probingAgentId }, + ); + const replacement = await interactionsSvc.create( + { id: issueId, companyId }, question("Newer question"), { agentId: probingAgentId }, + ); + + const interactions = await interactionsSvc.listForIssue(issueId); + expect(interactions.find((interaction) => interaction.id === older.id)).toMatchObject({ + status: "expired", + resolvedByAgentId: probingAgentId, + result: { + answers: [], + expirationReason: "superseded_by_newer_interaction", + supersededByInteractionId: replacement.id, + }, + }); + expect(interactions.find((interaction) => interaction.id === replacement.id)?.status).toBe("pending"); + // A different agent's pending question is untouched. + expect(interactions.find((interaction) => interaction.id === otherAgentQuestion.id)?.status).toBe("pending"); + // A different kind from the same agent is untouched. + expect(interactions.find((interaction) => interaction.id === otherKind.id)?.status).toBe("pending"); + + // The same agent's question on a different issue is untouched. + const otherIssueInteractions = await interactionsSvc.listForIssue(otherIssueId); + expect(otherIssueInteractions.find((interaction) => interaction.id === otherIssueQuestion.id)?.status) + .toBe("pending"); + }); + + it("leaves exactly one pending ask_user_questions on the onboarding first task after probe cards and the real question arrive", async () => { + const companyId = randomUUID(); + const goalId = randomUUID(); + const issueId = randomUUID(); + const agentId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + await db.insert(agents).values({ + id: agentId, + companyId, + name: "Chief of staff", + role: "chief_of_staff", + status: "active", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }); + await db.insert(goals).values({ + id: goalId, + companyId, + title: "Your first task", + level: "task", + status: "active", + }); + await db.insert(issues).values({ + id: issueId, + companyId, + goalId, + title: "Your first task", + status: "in_progress", + priority: "medium", + originKind: ONBOARDING_FIRST_TASK_ORIGIN_KIND, + assigneeAgentId: agentId, + }); + + // Reproduces PAP-436: the assigned agent posts two throwaway schema probes + // (title/prompt/option "t"/"p"/"L") before the genuine question. + const probe = (prompt: string) => ({ + kind: "ask_user_questions" as const, + payload: { + version: 1 as const, + questions: [{ + id: "q", + prompt, + selectionMode: "single" as const, + options: [{ id: "L", label: "L" }], + }], + }, + }); + await interactionsSvc.create({ id: issueId, companyId }, probe("t"), { agentId }); + await interactionsSvc.create({ id: issueId, companyId }, probe("p"), { agentId }); + await interactionsSvc.create({ id: issueId, companyId }, { + kind: "ask_user_questions", + payload: { + version: 1, + questions: [{ + id: "focus", + prompt: "What would you like your team to focus on first?", + selectionMode: "single", + options: [ + { id: "mvp", label: "Ship the MVP" }, + { id: "bugs", label: "Fix bugs" }, + ], + }], + }, + }, { agentId }); + + const interactions = await interactionsSvc.listForIssue(issueId); + const pendingQuestions = interactions.filter( + (interaction) => interaction.kind === "ask_user_questions" && interaction.status === "pending", + ); + expect(pendingQuestions).toHaveLength(1); + expect(pendingQuestions[0]?.kind).toBe("ask_user_questions"); + const [remaining] = pendingQuestions; + if (remaining?.kind === "ask_user_questions") { + expect(remaining.payload.questions[0]?.prompt).toContain("focus on first"); + } + + // Both probe cards auto-expired with the sibling-supersede reason. + const expiredQuestions = interactions.filter( + (interaction) => interaction.kind === "ask_user_questions" && interaction.status === "expired", + ); + expect(expiredQuestions).toHaveLength(2); + for (const card of expiredQuestions) { + expect(card.result).toMatchObject({ expirationReason: "superseded_by_newer_interaction" }); + } + }); + it("sweeps historical confirmation pile-ups idempotently per issue, kind, and agent", async () => { const { companyId, issueId } = await seedConfirmationIssue("Historical confirmation sweep"); const firstAgentId = randomUUID(); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 02084fe7cf..ac605bd6a5 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -56,6 +56,7 @@ import { ISSUE_CONTINUATION_SUMMARY_DOCUMENT_KEY, ISSUE_WATCHDOG_DISCOVERY_KINDS, TASK_WATCHDOG_PRODUCT_BUG_ORIGIN_KIND, + ONBOARDING_FIRST_TASK_ORIGIN_KIND, rejectIssueThreadInteractionSchema, restoreIssueDocumentRevisionSchema, respondIssueThreadInteractionSchema, @@ -166,6 +167,10 @@ import { SVG_CONTENT_TYPE, } from "../attachment-types.js"; import { queueIssueAssignmentWakeup } from "../services/issue-assignment-wakeup.js"; +import { + buildOnboardingGreeting, + ONBOARDING_GREETING_AUTHORIZATION_REASON, +} from "../services/onboarding-greeting.js"; import { ISSUE_BLOCKERS_RESOLVED_WAKE_REASON, buildIssueBlockersResolvedWakeIdempotencyKey, @@ -592,6 +597,27 @@ function authenticatedActorResponsibleUserId(req: Request) { return req.actor.type === "agent" ? req.actor.onBehalfOfUserId ?? null : undefined; } +// Matches the partial unique index that guarantees at most one onboarding +// first-task issue per company (packages/db/src/schema/issues.ts). +function isOnboardingFirstTaskConflict(error: unknown): boolean { + for ( + let current = error, depth = 0; + current && typeof current === "object" && depth < 5; + current = (current as { cause?: unknown }).cause, depth += 1 + ) { + const candidate = current as { code?: string; constraint?: string; message?: string }; + if ( + candidate.code === "23505" && + (candidate.constraint === "issues_onboarding_first_task_uq" || + (typeof candidate.message === "string" && + candidate.message.includes("issues_onboarding_first_task_uq"))) + ) { + return true; + } + } + return false; +} + function issueWriteAuthorizationReason( req: Request, decision: true | { reason?: string | null }, @@ -7625,7 +7651,34 @@ export function issueRoutes( surface: "issues.create", }); if (!sanitizedBody) return; - const { watchdogDiscovery: rawWatchdogDiscovery, ...rawCreateBody } = sanitizedBody; + const { + watchdogDiscovery: rawWatchdogDiscovery, + onboardingFirstTask: rawOnboardingFirstTask, + ...rawCreateBody + } = sanitizedBody; + // The onboarding first-task marker grants privileged, server-owned behavior: + // it stamps the onboarding origin (which suppresses the seeded description in + // the UI) and seeds a comment authored *as the assigned agent*. Honor it only + // when the request is genuinely the onboarding wizard creating a company's + // very first task, verified server-side so a client marker alone cannot + // trigger it: + // 1. the caller is a human board/user session (the wizard never runs as an + // agent), and + // 2. the company has no existing issues yet — i.e. this really is the first + // task. An established company creating an ordinary issue can never reach + // the greeting/description-suppression path, so no board caller can + // fabricate a statement attributed to an assigned agent on a normal task. + // Fails closed: if it is not verifiably the first task, the flag is ignored + // and an ordinary issue is created. The zero-count read below is only a + // fast-path gate — overlapping requests could both observe zero — so the + // partial unique index issues_onboarding_first_task_uq is what atomically + // enforces at most one onboarding first task per company; the create call + // handles losing that race by degrading to an ordinary issue. + const onboardingFirstTaskRequested = + rawOnboardingFirstTask === true && req.actor.type === "board"; + let isOnboardingFirstTask = onboardingFirstTaskRequested + ? (await svc.count(companyId)) === 0 + : false; const watchdogDiscovery = normalizeWatchdogDiscovery(rawWatchdogDiscovery); const watchdogProductBugFollowUp = await resolveTaskWatchdogProductBugFollowUp( req, @@ -7680,6 +7733,9 @@ export function issueRoutes( ...(runWorkspaceInheritanceSourceIssueId ? { inheritExecutionWorkspaceFromIssueId: runWorkspaceInheritanceSourceIssueId } : {}), + ...(isOnboardingFirstTask && !watchdogProductBugFollowUp + ? { originKind: ONBOARDING_FIRST_TASK_ORIGIN_KIND } + : {}), ...(watchdogProductBugFollowUp ? { description: appendWatchdogDiscoveryContext({ @@ -7734,7 +7790,7 @@ export function issueRoutes( executionPolicy, }, actor); let deduplicationReason: "idempotency_key" | "recent_open_title" | null = null; - const issue = await svc.create(companyId, { + const createInput = { ...createBody, ...(taskBridgeOriginForActor(req) ?? {}), id: issueId, @@ -7747,10 +7803,23 @@ export function issueRoutes( actorResponsibleUserId: authenticatedActorResponsibleUserId(req), trustExplicitResponsibleUserId: actor.actorType === "user", watchdogActorRunId: actor.runId, - onDeduplicated: (reason) => { + onDeduplicated: (reason: "idempotency_key" | "recent_open_title") => { deduplicationReason = reason; }, - }); + }; + let issue: Awaited>; + try { + issue = await svc.create(companyId, createInput); + } catch (error) { + // Concurrent onboarding creates can both pass the zero-count fast path; + // the issues_onboarding_first_task_uq index rejects the loser here. Fail + // closed: drop the privileged origin (and with it the agent-attributed + // greeting) and create an ordinary issue instead. + if (!(isOnboardingFirstTask && isOnboardingFirstTaskConflict(error))) throw error; + isOnboardingFirstTask = false; + const { originKind: _onboardingOriginKind, ...ordinaryCreateInput } = createInput; + issue = await svc.create(companyId, ordinaryCreateInput); + } if (deduplicationReason) { const referenceSummary = await issueReferencesSvc.listIssueReferenceSummary(issue.id); res.status(200).json({ @@ -7849,6 +7918,39 @@ export function issueRoutes( }); } + // Seed the onboarding first-task greeting as an agent-authored comment so the + // user lands on a waiting greeting (instead of a right-aligned "user" bubble + // showing the seeded description). Deterministic template — no LLM call — and + // best-effort: a greeting failure must not fail issue creation. + if (isOnboardingFirstTask && issue.assigneeAgentId) { + try { + const [company, goal, assigneeAgent] = await Promise.all([ + companiesSvc.getById(companyId), + createBody.goalId ? goalsSvc.getById(createBody.goalId) : Promise.resolve(null), + agentsSvc.getById(issue.assigneeAgentId), + ]); + const greetingBody = buildOnboardingGreeting({ + agentName: assigneeAgent?.name ?? null, + teamName: company?.name ?? null, + goals: goal?.description ?? goal?.title ?? null, + }); + await svc.addComment( + issue.id, + greetingBody, + { agentId: issue.assigneeAgentId }, + { + authorType: "agent", + authorizationReason: ONBOARDING_GREETING_AUTHORIZATION_REASON, + }, + ); + } catch (err) { + logger.warn( + { err, issueId: issue.id, companyId }, + "failed to seed onboarding first-task greeting", + ); + } + } + void queueIssueAssignmentWakeup({ heartbeat, issue, diff --git a/server/src/services/built-in-agents.ts b/server/src/services/built-in-agents.ts index 40c46efe72..718263668e 100644 --- a/server/src/services/built-in-agents.ts +++ b/server/src/services/built-in-agents.ts @@ -471,6 +471,12 @@ const DEFINITIONS = validateBuiltInAgentDefinitions([ const DEFINITIONS_BY_KEY = new Map(DEFINITIONS.map((definition) => [definition.key, definition])); +// Bundled built-in agents that should be provisioned automatically when a +// company is created (and re-ensured on startup reconcile). Empty by default so +// a new user starts clean — the Reflection Coach and Summarizer are opt-in, not +// seeded. Add a definition key here to restore automatic provisioning. +const AUTO_PROVISION_ON_COMPANY_CREATE_KEYS = new Set([]); + const ROOT_AGENT_DEFAULT_CHANGE_GRANTS: PermissionKey[] = ["agents:configure", "skills:create"]; const BUILT_IN_AGENT_DEFAULT_GRANTS: Record = { "reflection-coach": ["agents:suggest-changes", "skills:suggest-changes"], @@ -1917,7 +1923,17 @@ export function builtInAgentService(db: Db) { const company = await ensureCompany(companyId); let autoEnsured = 0; let pendingApprovals = 0; + // A fresh company starts with only its own lead agent — the Reflection + // Coach and Summarizer are no longer auto-created for new users. They stay + // available to enable on demand (via ensure / provision / the built-in + // bundle panel). We still reconcile any bundled agent that already exists + // (e.g. one an operator enabled) so its instructions/skill/routine keep + // tracking stock. Add a key to AUTO_PROVISION_ON_COMPANY_CREATE_KEYS to + // restore automatic creation for that definition. for (const definition of DEFINITIONS.filter((entry) => entry.bundle)) { + const existing = await findSingleAgent(companyId, definition); + const shouldProvision = existing !== null || AUTO_PROVISION_ON_COMPANY_CREATE_KEYS.has(definition.key); + if (!shouldProvision) continue; if (company.requireBoardApprovalForNewAgents) { const result = await provision(companyId, definition.key); if (result.approval) pendingApprovals += 1; diff --git a/server/src/services/instance-settings.ts b/server/src/services/instance-settings.ts index 9be8b2e1a0..8f3f392bf5 100644 --- a/server/src/services/instance-settings.ts +++ b/server/src/services/instance-settings.ts @@ -214,7 +214,7 @@ export function normalizeExperimentalSettings(raw: unknown): InstanceExperimenta enablePipelines: parsed.data.enablePipelines ?? false, enableCases: parsed.data.enableCases ?? false, enableConferenceRoomChat: parsed.data.enableConferenceRoomChat ?? false, - enableTaskChatRedesign: parsed.data.enableTaskChatRedesign ?? false, + enableClassicTaskInterface: parsed.data.enableClassicTaskInterface ?? false, enableIssuePlanDecompositions: parsed.data.enableIssuePlanDecompositions ?? false, enableExperimentalFileViewer: parsed.data.enableExperimentalFileViewer ?? false, enableTaskWatchdogs: parsed.data.enableTaskWatchdogs ?? false, @@ -250,7 +250,7 @@ export function normalizeExperimentalSettings(raw: unknown): InstanceExperimenta enablePipelines: false, enableCases: false, enableConferenceRoomChat: false, - enableTaskChatRedesign: false, + enableClassicTaskInterface: false, enableTaskWatchdogs: false, enableIssuePlanDecompositions: false, enableExperimentalFileViewer: false, diff --git a/server/src/services/issue-thread-interactions.ts b/server/src/services/issue-thread-interactions.ts index 351da2a346..57171354a9 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -565,6 +565,21 @@ function buildSupersededByNewerRequestResult(replacementInteractionId: string) { } as const; } +// An agent that posts a fresh ask_user_questions while its own earlier ones on +// the same issue are still pending has replaced them — the newer card carries +// the real ask, so the stale siblings auto-expire (PAP-437). Mirrors the +// `superseded_by_comment` shape (ask_user_questions results key expiry off +// `expirationReason`, not `outcome`) so the UI can hide them cleanly. +function buildSupersededByNewerInteractionResult(replacementInteractionId: string) { + return { + version: 1, + answers: [], + expirationReason: "superseded_by_newer_interaction", + supersededByInteractionId: replacementInteractionId, + summaryMarkdown: null, + } as const; +} + function buildAdministrativeOutcomeResult( row: IssueThreadInteractionRow, outcome: "withdrawn" | "issue_closed" | "addressee_deleted", @@ -1888,16 +1903,27 @@ export function issueThreadInteractionService(db: Db, opts: IssueThreadInteracti }) .returning(); - if (data.kind !== "request_confirmation" || !actor.agentId) { + // An agent replacing its own still-pending card supersedes the older + // one so the thread never accumulates stale sibling cards. This covers + // request_confirmation drafts and ask_user_questions (PAP-437: probe + // question cards that agents never withdrew). Each kind keeps its own + // result shape. Scoped strictly to the same agent + issue + kind, so + // other agents' or other kinds' pending cards are untouched. + const canSupersedeSiblingCards = + data.kind === "request_confirmation" || data.kind === "ask_user_questions"; + if (!actor.agentId || !canSupersedeSiblingCards) { return { row, supersededRows: [] }; } const now = new Date(); + const supersededResult = data.kind === "ask_user_questions" + ? buildSupersededByNewerInteractionResult(row.id) + : buildSupersededByNewerRequestResult(row.id); const supersededRows = await tx .update(issueThreadInteractions) .set({ status: "expired", - result: buildSupersededByNewerRequestResult(row.id), + result: supersededResult, resolvedByAgentId: actor.agentId, resolvedByUserId: actor.userId ?? null, resolvedAt: now, diff --git a/server/src/services/onboarding-greeting.test.ts b/server/src/services/onboarding-greeting.test.ts new file mode 100644 index 0000000000..4bb5f2550e --- /dev/null +++ b/server/src/services/onboarding-greeting.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from "vitest"; +import { buildOnboardingGreeting } from "./onboarding-greeting.js"; + +describe("buildOnboardingGreeting", () => { + it("introduces the agent by name as the user's first teammate and reflects the goals", () => { + const greeting = buildOnboardingGreeting({ + agentName: "Nova", + teamName: "Acme", + goals: "Launch a marketplace for local makers.", + }); + + expect(greeting).toContain( + "Welcome! I'm Nova, your first agent teammate on Paperclip.", + ); + expect(greeting).toContain("Here's what I understand you're aiming for:"); + expect(greeting).toContain("> Launch a marketplace for local makers."); + expect(greeting).toContain("propose a team of agents"); + expect(greeting).toContain("few focused questions"); + }); + + it("falls back to a generic teammate intro when no agent name is set", () => { + const greeting = buildOnboardingGreeting({ agentName: null, goals: null }); + + expect(greeting).toContain( + "Welcome! I'm your first agent teammate on Paperclip.", + ); + }); + + it("collapses whitespace in the reflected goals", () => { + const greeting = buildOnboardingGreeting({ + goals: " Build\n\n a SaaS product. ", + }); + + expect(greeting).toContain("> Build a SaaS product."); + }); + + it("omits the reflect-back block when no goals are provided", () => { + const greeting = buildOnboardingGreeting({ agentName: "Nova", goals: null }); + + expect(greeting).not.toContain("aiming for"); + expect(greeting).toContain("propose a team of agents"); + }); +}); diff --git a/server/src/services/onboarding-greeting.ts b/server/src/services/onboarding-greeting.ts new file mode 100644 index 0000000000..4c4ae5dc5e --- /dev/null +++ b/server/src/services/onboarding-greeting.ts @@ -0,0 +1,39 @@ +// Deterministic, template-driven greeting seeded as an agent-authored comment on +// the onboarding first task. No LLM call: it reflects back the onboarding context +// (team name + goals) so the user lands on a waiting greeting instead of a +// right-aligned "user" bubble showing the agent's own seeded instructions. + +export const ONBOARDING_GREETING_AUTHORIZATION_REASON = "onboarding first-task greeting"; + +export function buildOnboardingGreeting(input: { + agentName?: string | null; + teamName?: string | null; + goals?: string | null; +}): string { + const agentName = input.agentName?.trim(); + const goals = input.goals?.replace(/\s+/g, " ").trim(); + + // Introduce the agent by the name the user chose in onboarding when we have + // it, so the first message reads as coming from *their* first teammate rather + // than a generic agent. Fall back to the generic phrasing otherwise. + const identity = agentName + ? `Welcome! I'm ${agentName}, your first agent teammate on Paperclip.` + : "Welcome! I'm your first agent teammate on Paperclip."; + + const lines: string[] = []; + lines.push(identity); + + if (goals) { + lines.push(""); + lines.push("Here's what I understand you're aiming for:"); + lines.push(""); + lines.push(`> ${goals}`); + } + + lines.push(""); + lines.push( + "I want to gather more context so I can come up with a plan and propose a team of agents to help execute it. I'm putting together a few focused questions so we can settle on a concrete goal to tackle first. Please give me one moment...", + ); + + return lines.join("\n"); +} diff --git a/tests/e2e/conference-room-typing-intro.spec.ts b/tests/e2e/conference-room-typing-intro.spec.ts index e1a7096ed6..6d528bc99f 100644 --- a/tests/e2e/conference-room-typing-intro.spec.ts +++ b/tests/e2e/conference-room-typing-intro.spec.ts @@ -1,105 +1,157 @@ -import { test, expect } from "@playwright/test"; +import { test, expect, type Page } from "@playwright/test"; +import { + expectLandsOnFirstTaskWithoutDashboardBounce, + instrumentNavLog, +} from "./helpers/onboarding-landing"; /** * E2E: post-wizard onboarding launch. * * Completing the onboarding wizard now creates the first assigned task and - * lands the user on the company dashboard. The chat intro still has unit - * coverage in BoardChat tests; the wizard handoff no longer routes there. + * drops the user straight onto that task's detail page (not the dashboard), + * so they land in the conversation the agent will start in. The chat intro + * still has unit coverage in BoardChat tests. + * + * PAP-404: onboarding used to intermittently bounce to the company dashboard. + * The bounce only reproduces when the instance already has ≥1 company (the + * board's test ports), so the second test seeds a company first to exercise + * exactly that failing condition. */ -const COMPANY_NAME = `E2E-TypingIntro-${Date.now()}`; -const MISSION = "Verify the dashboard launch survives the wizard handoff."; -const FIRST_TASK_TITLE = "Hire your first engineer and create a hiring plan"; +const MISSION = "Verify the first-task launch survives the wizard handoff."; +const FIRST_TASK_TITLE = "Paperclip onboarding"; -test.describe("Dashboard launch after onboarding wizard", () => { - test("creates the first task and opens the dashboard", async ({ +/** + * Intercept the two side-effecting calls the wizard makes so no real CLI check + * runs and no real agent process spawns (the hire still happens server-side + * with an inert http adapter). + */ +async function installLaunchIntercepts(page: Page, baseURL?: string) { + await page.route("**/test-environment", (route) => + route.fulfill({ + contentType: "application/json", + body: JSON.stringify({ status: "pass", checks: [] }), + }), + ); + + await page.route("**/agent-hires", async (route) => { + const req = route.request(); + const body = JSON.parse(req.postData() || "{}"); + const auth = req.headers().authorization; + const real = await fetch(new URL(req.url(), baseURL).toString(), { + method: "POST", + headers: { + "Content-Type": "application/json", + ...(auth ? { Authorization: auth } : {}), + }, + body: JSON.stringify({ + name: body.name, + role: body.role, + adapterType: "http", + adapterConfig: { url: "http://127.0.0.1:1/dead" }, + runtimeConfig: { heartbeat: { enabled: false } }, + }), + }); + await route.fulfill({ + status: real.status, + contentType: "application/json", + body: await real.text(), + }); + }); +} + +/** Drive the wizard from the /onboarding route through to "Get started". */ +async function runOnboardingWizard(page: Page, companyName: string) { + await page.goto("/onboarding"); + + // Launcher card path (existing companies) — enter the wizard if the + // route shows a launcher instead of opening the wizard directly. + const startBtn = page.getByRole("button", { name: /Start Onboarding/i }); + if (await startBtn.count()) await startBtn.first().click(); + + // Step 0: front door (skipped when the wizard opens on the create path). + const frontDoor = page.getByText("Build a new company"); + if (await frontDoor.count()) await frontDoor.first().click(); + + // Step 1: company name. + await page.getByPlaceholder("Acme Corp").fill(companyName); + await page.getByRole("button", { name: /^Next/ }).click(); + + // Step 2: mission (direct path default). + await page.getByPlaceholder("What is your team trying to achieve?").fill(MISSION); + await page.getByRole("button", { name: /Confirm mission/ }).click(); + + // Step 3: lead name (prefilled) → Next. + await page.waitForSelector('input[placeholder="Chief of staff"]', { + timeout: 15_000, + }); + await page.getByRole("button", { name: /^Next/ }).click(); + + // Step 4: adapter (claude_local default); heartbeat is intercepted. + await page.getByRole("button", { name: /^Connect$/ }).click(); + + // Step 5: review → Get started creates the first task and opens its + // detail page. + const getStarted = page.getByRole("button", { name: /Get started/ }); + await getStarted.waitFor({ timeout: 20_000 }); + await getStarted.click(); +} + +async function assertFirstTaskExists(page: Page, companyName: string) { + const companiesRes = await page.request.get("/api/companies"); + expect(companiesRes.ok()).toBe(true); + const companies = await companiesRes.json(); + const company = companies.find( + (candidate: { name: string }) => candidate.name === companyName, + ); + expect(company).toBeTruthy(); + + const issuesRes = await page.request.get(`/api/companies/${company.id}/issues`); + expect(issuesRes.ok()).toBe(true); + const issues = await issuesRes.json(); + const firstTask = issues.find( + (candidate: { title: string }) => candidate.title === FIRST_TASK_TITLE, + ); + expect(firstTask).toBeTruthy(); + await expect(page.getByText(FIRST_TASK_TITLE).first()).toBeVisible({ + timeout: 15_000, + }); +} + +test.describe("First-task launch after onboarding wizard", () => { + test("creates the first task and opens its detail page", async ({ page, baseURL }) => { + await instrumentNavLog(page); + await installLaunchIntercepts(page, baseURL); + + const companyName = `E2E-TypingIntro-${Date.now()}`; + await runOnboardingWizard(page, companyName); + + await expectLandsOnFirstTaskWithoutDashboardBounce(page); + await assertFirstTaskExists(page, companyName); + }); + + // PAP-404 regression: the dashboard bounce only fires when the instance + // already has a company for the route-sync effect to reset selection to. + // Seed one first, then onboard a brand-new company and assert we still land + // on the first task without a dashboard bounce. + test("lands on the first task even when a company already exists", async ({ page, baseURL, }) => { - // Intercept env-test → instant pass (avoid running a real CLI check). - await page.route("**/test-environment", (route) => - route.fulfill({ - contentType: "application/json", - body: JSON.stringify({ status: "pass", checks: [] }), - }), - ); + await instrumentNavLog(page); + await installLaunchIntercepts(page, baseURL); - // Intercept hire → perform a REAL hire server-side with an inert http - // adapter so no real agent process spawns. - await page.route("**/agent-hires", async (route) => { - const req = route.request(); - const body = JSON.parse(req.postData() || "{}"); - const auth = req.headers().authorization; - const real = await fetch(new URL(req.url(), baseURL).toString(), { - method: "POST", - headers: { - "Content-Type": "application/json", - ...(auth ? { Authorization: auth } : {}), - }, - body: JSON.stringify({ - name: body.name, - role: body.role, - adapterType: "http", - adapterConfig: { url: "http://127.0.0.1:1/dead" }, - runtimeConfig: { heartbeat: { enabled: false } }, - }), - }); - await route.fulfill({ - status: real.status, - contentType: "application/json", - body: await real.text(), - }); + // Seed a pre-existing company so the companies list is non-empty when the + // wizard launches — the exact condition that reproduced the bounce. + const seedRes = await page.request.post("/api/companies", { + data: { name: `E2E-Seed-${Date.now()}` }, }); + expect(seedRes.ok()).toBe(true); - await page.goto("/onboarding"); + const companyName = `E2E-TypingIntro-Existing-${Date.now()}`; + await runOnboardingWizard(page, companyName); - // Launcher card path (existing companies) — enter the wizard if the - // route shows a launcher instead of opening the wizard directly. - const startBtn = page.getByRole("button", { name: /Start Onboarding/i }); - if (await startBtn.count()) await startBtn.first().click(); - - // Step 0: front door (skipped when the wizard opens on the create path). - const frontDoor = page.getByText("Build a new company"); - if (await frontDoor.count()) await frontDoor.first().click(); - - // Step 1: company name. - await page.getByPlaceholder("Acme Corp").fill(COMPANY_NAME); - await page.getByRole("button", { name: /^Next/ }).click(); - - // Step 2: mission (direct path default). - await page - .getByPlaceholder("What is your team trying to achieve?") - .fill(MISSION); - await page.getByRole("button", { name: /Confirm mission/ }).click(); - - // Step 3: lead name (prefilled) → Next. - await page.waitForSelector('input[placeholder="Chief of staff"]', { - timeout: 15_000, - }); - await page.getByRole("button", { name: /^Next/ }).click(); - - // Step 4: adapter (claude_local default); heartbeat is intercepted. - await page.getByRole("button", { name: /Give it a heartbeat/ }).click(); - - // Step 5: review → Get started creates the first task and opens dashboard. - const getStarted = page.getByRole("button", { name: /Get started/ }); - await getStarted.waitFor({ timeout: 20_000 }); - await getStarted.click(); - - await expect(page).toHaveURL(/\/dashboard$/, { timeout: 30_000 }); - - const companiesRes = await page.request.get("/api/companies"); - expect(companiesRes.ok()).toBe(true); - const companies = await companiesRes.json(); - const company = companies.find((candidate: { name: string }) => candidate.name === COMPANY_NAME); - expect(company).toBeTruthy(); - - const issuesRes = await page.request.get(`/api/companies/${company.id}/issues`); - expect(issuesRes.ok()).toBe(true); - const issues = await issuesRes.json(); - const firstTask = issues.find((candidate: { title: string }) => candidate.title === FIRST_TASK_TITLE); - expect(firstTask).toBeTruthy(); - await expect(page.getByText(FIRST_TASK_TITLE).first()).toBeVisible({ timeout: 15_000 }); + await expectLandsOnFirstTaskWithoutDashboardBounce(page); + await assertFirstTaskExists(page, companyName); }); }); diff --git a/tests/e2e/helpers/onboarding-landing.ts b/tests/e2e/helpers/onboarding-landing.ts new file mode 100644 index 0000000000..3d3700502c --- /dev/null +++ b/tests/e2e/helpers/onboarding-landing.ts @@ -0,0 +1,80 @@ +import { expect, type Page } from "@playwright/test"; + +/** + * Regression guard for PAP-404: onboarding must land the user on the newly + * created first-task detail page and must NOT bounce to the company dashboard. + * + * The bounce is a navigation race: the wizard `navigate(/…/issues/…)` used to + * be clobbered by `useCompanyPageMemory`, which restores a company's remembered + * page (falling back to `/dashboard`) on a non-`route_sync` selection change. + * + * react-router-dom drives client-side navigation through the History API, so a + * Playwright `framenavigated` listener never fires for these SPA transitions. + * Instead we hook `history.pushState`/`replaceState` before the app boots and + * record every path the router visits — a `/dashboard` bounce (even a transient + * one that later self-corrects) leaves a trace in the log. + */ + +const ISSUE_URL = /\/issues\/[^/]+$/; +const DASHBOARD_PATH = /\/dashboard(\/|$)/; + +/** + * Install a History-API tap that records every client-side path change into + * `window.__navLog`. Must be called BEFORE the first `page.goto` so the init + * script is present when the SPA boots. + */ +export async function instrumentNavLog(page: Page): Promise { + await page.addInitScript(() => { + const w = window as unknown as { __navLog?: string[] }; + if (w.__navLog) return; + const log: string[] = []; + w.__navLog = log; + const record = () => log.push(window.location.pathname); + const wrap = unknown>(fn: T): T => + function (this: unknown, ...args: never[]) { + const result = fn.apply(this, args); + record(); + return result; + } as unknown as T; + history.pushState = wrap(history.pushState.bind(history)); + history.replaceState = wrap(history.replaceState.bind(history)); + window.addEventListener("popstate", record); + record(); + }); +} + +async function readNavLog(page: Page): Promise { + return page.evaluate( + () => (window as unknown as { __navLog?: string[] }).__navLog ?? [], + ); +} + +/** + * Assert the wizard settled on the first-task detail page and never rested on + * (or bounced through) the dashboard. + * + * Fails if a dashboard bounce is reintroduced: the History-API log will contain + * a `/dashboard` entry and/or the settled URL will not be the issue page. + */ +export async function expectLandsOnFirstTaskWithoutDashboardBounce( + page: Page, +): Promise { + // The wizard's launch handler does async work (hire + create issue) before + // navigating, so give reaching the issue page a generous budget. + await expect(page).toHaveURL(ISSUE_URL, { timeout: 30_000 }); + const settledUrl = page.url(); + + // Short settle window: the page-memory effect fires on the selection change + // that accompanies the launch navigate, so any bounce lands within ~1s. If + // the URL is still the issue after this window it has genuinely settled. + await page.waitForTimeout(1_500); + expect(page.url(), "onboarding bounced away from the first task").toBe(settledUrl); + await expect(page).toHaveURL(ISSUE_URL); + + const navLog = await readNavLog(page); + const bounced = navLog.filter((path) => DASHBOARD_PATH.test(path)); + expect( + bounced, + `onboarding navigated to the dashboard (nav log: ${navLog.join(" -> ")})`, + ).toEqual([]); +} diff --git a/tests/e2e/nux-phase4-screenshots.spec.ts b/tests/e2e/nux-phase4-screenshots.spec.ts index 4c3a3481be..c872f5712b 100644 --- a/tests/e2e/nux-phase4-screenshots.spec.ts +++ b/tests/e2e/nux-phase4-screenshots.spec.ts @@ -66,7 +66,7 @@ test.describe("NUX Phase 4 visual QA", () => { await createCard.first().click(); } await expect( - page.getByRole("heading", { name: "Name your company" }), + page.getByRole("heading", { name: "Name your organization" }), ).toBeVisible({ timeout: 15_000 }); await page.getByPlaceholder("Acme Corp").fill("QA Robotics"); await page.screenshot({ path: shot("02-create-name.png") }); @@ -120,7 +120,7 @@ test.describe("NUX Phase 4 visual QA", () => { await page.getByRole("button", { name: /Add agents to your org/ }).click(); // The grow path shares step 1 (company name) before its step-2 intake. await expect( - page.getByRole("heading", { name: "Name your company" }), + page.getByRole("heading", { name: "Name your organization" }), ).toBeVisible({ timeout: 10_000 }); await page.getByPlaceholder("Acme Corp").fill("QA Robotics Grow"); await page.getByRole("button", { name: /^Next/ }).click(); diff --git a/tests/e2e/onboarding.spec.ts b/tests/e2e/onboarding.spec.ts index eb46e76e50..a29078a724 100644 --- a/tests/e2e/onboarding.spec.ts +++ b/tests/e2e/onboarding.spec.ts @@ -6,7 +6,7 @@ import { test, expect } from "@playwright/test"; * The wizard now opens on a front door (path picker) and the "Create a new * company" path runs: * Step 0 — Front door (Create a new company / Level up existing) - * Step 1a — Name your company + * Step 1a — Name your organization * Step 1b — Define your mission (direct or guided) * Step 2 — Hire your team lead (adapter picker) * Step 3+ — Launch celebration → CEO chat → hiring plan → orientation @@ -53,9 +53,9 @@ test.describe("Onboarding wizard", () => { await createCard.first().click(); } - // Step 1 — Name your company. + // Step 1 — Name your organization. await expect( - page.getByRole("heading", { name: "Name your company" }), + page.getByRole("heading", { name: "Name your organization" }), ).toBeVisible({ timeout: 15_000 }); await page.getByPlaceholder("Acme Corp").fill(COMPANY_NAME); await page.getByRole("button", { name: /^Next/ }).click(); diff --git a/tests/e2e/planning-mode-visual-verification.spec.ts b/tests/e2e/planning-mode-visual-verification.spec.ts index 65be76312d..398fe24f0d 100644 --- a/tests/e2e/planning-mode-visual-verification.spec.ts +++ b/tests/e2e/planning-mode-visual-verification.spec.ts @@ -1,13 +1,19 @@ import { expect, test } from "@playwright/test"; +import { + expectLandsOnFirstTaskWithoutDashboardBounce, + instrumentNavLog, +} from "./helpers/onboarding-landing"; const AGENT_NAME = "Chief of staff"; -const TASK_TITLE = "Hire your first engineer and create a hiring plan"; +const TASK_TITLE = "Paperclip onboarding"; test("captures planning mode UI for desktop and mobile", async ({ page }) => { const timestamp = Date.now(); const companyName = `PAP-3413-${timestamp}`; const screenshotDir = "test-results/planning-mode"; + await instrumentNavLog(page); + await page.route("**/test-environment", (route) => route.fulfill({ contentType: "application/json", @@ -47,7 +53,7 @@ test("captures planning mode UI for desktop and mobile", async ({ page }) => { const createCard = page.getByRole("button", { name: /Build a new company/ }); if (await createCard.count()) await createCard.first().click(); - await expect(page.getByRole("heading", { name: "Name your company" })).toBeVisible({ timeout: 15_000 }); + await expect(page.getByRole("heading", { name: "Name your organization" })).toBeVisible({ timeout: 15_000 }); await page.locator('input[placeholder="Acme Corp"]').fill(companyName); await page.getByRole("button", { name: /^Next/ }).click(); @@ -62,11 +68,13 @@ test("captures planning mode UI for desktop and mobile", async ({ page }) => { await expect(page.locator('input[placeholder="Chief of staff"]')).toHaveValue(AGENT_NAME); await page.getByRole("button", { name: /^Next/ }).click(); - await page.getByRole("button", { name: /Give it a heartbeat/ }).click(); + await page.getByRole("button", { name: /^Connect$/ }).click(); await expect(page.getByRole("heading", { name: "Review" })).toBeVisible({ timeout: 30_000 }); await page.getByRole("button", { name: /Get started/ }).click(); - await expect(page).toHaveURL(/\/dashboard$/, { timeout: 30_000 }); + // The wizard now drops the user straight onto the first task's detail page, + // and must not bounce through the dashboard (PAP-404). + await expectLandsOnFirstTaskWithoutDashboardBounce(page); const baseOrigin = new URL(page.url()).origin; const companyRes = await page.request.get(`${baseOrigin}/api/companies`); @@ -108,11 +116,9 @@ test("captures planning mode UI for desktop and mobile", async ({ page }) => { await page.goto(issuePath); await expect(page.getByText("Plan mode").first()).toBeVisible(); - await expect(page.getByTestId("issue-chat-composer")).toHaveAttribute("data-pending-work-mode", "planning"); - const desktopPlanningToggle = page.getByTestId("issue-chat-composer-work-mode-toggle"); + const desktopPlanningToggle = page.getByTestId("task-chat-composer-mode"); await expect(desktopPlanningToggle).toBeVisible(); await expect(desktopPlanningToggle).toHaveAttribute("data-pending-work-mode", "planning"); - await expect(desktopPlanningToggle).toHaveAttribute("aria-pressed", "true"); await page.screenshot({ path: `${screenshotDir}/desktop-planning-detail-${timestamp}.png`, @@ -128,11 +134,9 @@ test("captures planning mode UI for desktop and mobile", async ({ page }) => { }); await page.goto(issuePath); - await page.getByTestId("issue-chat-composer-work-mode-toggle").click(); - await page.getByTestId("issue-chat-composer-work-mode-menu-standard").click(); - await expect(page.getByTestId("issue-chat-composer")).toHaveAttribute("data-pending-work-mode", "standard"); - await expect(page.getByTestId("issue-chat-composer-work-mode-toggle")).toHaveAttribute("data-pending-work-mode", "standard"); - await expect(page.getByTestId("issue-chat-composer-work-mode-toggle")).toHaveAttribute("aria-pressed", "false"); + await page.getByTestId("task-chat-composer-mode").click(); + await page.getByRole("menuitem", { name: /Agent mode/ }).click(); + await expect(page.getByTestId("task-chat-composer-mode")).toHaveAttribute("data-pending-work-mode", "standard"); await page.screenshot({ path: `${screenshotDir}/desktop-standard-toggle-${timestamp}.png`, fullPage: true, @@ -142,10 +146,9 @@ test("captures planning mode UI for desktop and mobile", async ({ page }) => { await page.setViewportSize({ width: 390, height: 844 }); await page.goto(issuePath); await expect(page.getByText("Plan mode").first()).toBeVisible(); - const mobilePlanningToggle = page.getByTestId("issue-chat-composer-work-mode-toggle"); + const mobilePlanningToggle = page.getByTestId("task-chat-composer-mode"); await expect(mobilePlanningToggle).toBeVisible(); await expect(mobilePlanningToggle).toHaveAttribute("data-pending-work-mode", "planning"); - await expect(mobilePlanningToggle).toHaveAttribute("aria-pressed", "true"); await page.screenshot({ path: `${screenshotDir}/mobile-planning-detail-${timestamp}.png`, fullPage: true, diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 3a5c387323..31a7107086 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -3,7 +3,6 @@ import { Button } from "@/components/ui/button"; import { useTranslation } from "@/i18n"; import { Layout } from "./components/Layout"; import { ConferenceRoomChatGate } from "./components/ConferenceRoomChatGate"; -import { TaskChatRedesignGate } from "./components/TaskChatRedesignGate"; import { TaskChatLab } from "./pages/TaskChatLab"; import { PipelinesExperimentalGate } from "./components/PipelinesExperimentalGate"; import { CasesExperimentalGate } from "./components/CasesExperimentalGate"; @@ -276,13 +275,9 @@ function boardRoutes() { } /> } /> - {/* Task Chat Redesign dev harness — dev builds only, and additionally - gated by enableTaskChatRedesign (redirects to /dashboard when the - flag is off). */} + {/* Task chat dev harness — dev builds only. */} {import.meta.env.DEV ? ( - }> - } /> - + } /> ) : null} } /> } /> diff --git a/ui/src/components/IssueChatThread.tsx b/ui/src/components/IssueChatThread.tsx index 729b64bd36..abb3d14957 100644 --- a/ui/src/components/IssueChatThread.tsx +++ b/ui/src/components/IssueChatThread.tsx @@ -439,10 +439,10 @@ interface IssueChatThreadProps { linkedRuns?: IssueChatLinkedRun[]; timelineEvents?: IssueTimelineEvent[]; /** - * Work-mode switch history from the activity feed. Only the redesigned - * TaskChatThread consumes this (flag: enableTaskChatRedesign) to tag each - * agent reply with the mode its request ran under; the legacy thread - * ignores it. + * Work-mode switch history from the activity feed. Only the chat-style + * TaskChatThread consumes this to tag each agent reply with the mode its + * request ran under; this thread — the classic task view behind + * enableClassicTaskInterface — ignores it. */ workModeChanges?: IssueWorkModeChange[]; liveRuns?: LiveRunForIssue[]; @@ -513,16 +513,15 @@ interface IssueChatThreadProps { footer?: ReactNode; /** * Issue header content (title row, badges, plugin toolbars) rendered INSIDE - * the thread's scroll viewport so it scrolls away with the messages. Only the - * redesigned TaskChatThread consumes this (flag: enableTaskChatRedesign); - * the legacy thread ignores it — its header stays in the page flow. + * the thread's scroll viewport so it scrolls away with the messages. Only + * the chat-style TaskChatThread consumes this; this thread ignores it — its + * header stays in the page flow. */ threadHeader?: ReactNode; /** * The task description rendered as the requester's first chat bubble - * (PAP-375). Only the redesigned TaskChatThread consumes it (flag: - * enableTaskChatRedesign); the legacy thread ignores it — its description - * stays in the page header via InlineEditor. + * (PAP-375). Only the chat-style TaskChatThread consumes it; this thread + * ignores it — its description stays in the page header via InlineEditor. */ issueBrief?: TaskChatIssueBrief; variant?: "full" | "embedded"; diff --git a/ui/src/components/IssueProperties.test.tsx b/ui/src/components/IssueProperties.test.tsx index 40d8a2be4d..f9ac92b65c 100644 --- a/ui/src/components/IssueProperties.test.tsx +++ b/ui/src/components/IssueProperties.test.tsx @@ -481,7 +481,7 @@ describe("IssueProperties", () => { it("keeps the Plan tab visible for a planning-mode issue without a plan document", async () => { mockInstanceSettingsApi.getExperimental.mockResolvedValue({ enableTaskWatchdogs: false, - enableTaskChatRedesign: true, + enableClassicTaskInterface: false, }); mockIssuesApi.listInteractions.mockResolvedValue([ { diff --git a/ui/src/components/IssueThreadInteractionCard.test.tsx b/ui/src/components/IssueThreadInteractionCard.test.tsx index 2091aeacfd..2f655bcb01 100644 --- a/ui/src/components/IssueThreadInteractionCard.test.tsx +++ b/ui/src/components/IssueThreadInteractionCard.test.tsx @@ -9,6 +9,7 @@ import { ThemeProvider } from "../context/ThemeContext"; import { TooltipProvider } from "./ui/tooltip"; import { pendingAskUserQuestionsInteraction, + pendingAskUserQuestionsWithFreeTextOption, commentExpiredAskUserQuestionsInteraction, commentExpiredRequestConfirmationInteraction, declinedToolActionInteraction, @@ -185,6 +186,104 @@ describe("IssueThreadInteractionCard", () => { ); }); + it("reveals an inline field when a free-text option is selected and hides the standalone Other link", async () => { + const onSubmitInteractionAnswers = vi.fn(async () => undefined); + const host = renderCard({ + interaction: pendingAskUserQuestionsWithFreeTextOption, + onSubmitInteractionAnswers, + }); + + // A first-class free-text option suppresses the built-in "Other" link. + const otherLink = Array.from(host.querySelectorAll("button")).find( + (button) => button.textContent === "Other", + ); + expect(otherLink).toBeUndefined(); + + // No text field until the free-text option is selected. + expect(host.querySelector("textarea")).toBeNull(); + + const describeOption = Array.from(host.querySelectorAll('[role="radio"]')).find( + (button) => button.textContent?.includes("I'll describe it"), + ) as HTMLButtonElement | undefined; + expect(describeOption).toBeTruthy(); + + await act(async () => { + describeOption?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + expect(describeOption?.getAttribute("aria-checked")).toBe("true"); + const textarea = host.querySelector("textarea") as HTMLTextAreaElement | null; + expect(textarea).toBeTruthy(); + + await act(async () => { + const valueSetter = Object.getOwnPropertyDescriptor( + HTMLTextAreaElement.prototype, + "value", + )?.set; + valueSetter?.call(textarea, "Call it Threads"); + textarea!.dispatchEvent(new Event("input", { bubbles: true })); + }); + + const submitButton = Array.from(host.querySelectorAll("button")).find((button) => + button.textContent?.includes("Send answers"), + ); + await act(async () => { + submitButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + expect(onSubmitInteractionAnswers).toHaveBeenCalledWith( + expect.objectContaining({ kind: "ask_user_questions" }), + [ + { + questionId: "surface-name", + optionIds: [], + otherText: "Call it Threads", + }, + ], + ); + }); + + it("renders nothing for a degenerate ask_user_questions card", () => { + // A truly unanswerable question: a prompt with no options and no free-text + // field, so there is nothing for the user to select or type. Hiding it + // strands nothing. + const degenerate = { + ...pendingAskUserQuestionsInteraction, + id: "interaction-questions-degenerate", + payload: { + version: 1 as const, + title: "Placeholder", + questions: [ + { + id: "q1", + prompt: "Anything?", + selectionMode: "single" as const, + options: [], + }, + ], + }, + }; + + const host = renderCard({ + interaction: degenerate, + onSubmitInteractionAnswers: vi.fn(), + }); + + // No card wrapper, no title, no controls — the component returns null. + expect(host.childElementCount).toBe(0); + expect(host.textContent).toBe(""); + }); + + it("still renders a legitimate ask_user_questions card", () => { + const host = renderCard({ + interaction: pendingAskUserQuestionsInteraction, + onSubmitInteractionAnswers: vi.fn(), + }); + + expect(host.childElementCount).toBeGreaterThan(0); + expect(host.querySelectorAll('[role="radio"]').length).toBeGreaterThan(0); + }); + it("only shows question cancellation when a cancel handler is wired", () => { const withoutHandler = renderCard({ interaction: pendingAskUserQuestionsInteraction, @@ -329,32 +428,37 @@ describe("IssueThreadInteractionCard", () => { expect(host.textContent).toContain("No reason provided."); }); - it("requires a decline reason when the request confirmation payload asks for one", async () => { + it("requires a revision note when the request confirmation payload asks for one", async () => { const onRejectInteraction = vi.fn(async () => undefined); const host = renderCard({ interaction: pendingRequestConfirmationInteraction, onRejectInteraction, }); - const declineButton = Array.from(host.querySelectorAll("button")).find((button) => - button.textContent?.includes("Request revisions"), + // rejectRequiresReason drops the bare Reject: the only send-back path is Revise… + expect(Array.from(host.querySelectorAll("button")).some((button) => + button.textContent?.trim() === "Reject", + )).toBe(false); + const reviseButton = Array.from(host.querySelectorAll("button")).find((button) => + button.textContent?.includes("Revise"), ); - expect(declineButton).toBeTruthy(); + expect(reviseButton).toBeTruthy(); await act(async () => { - declineButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + reviseButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); - const saveButton = Array.from(host.querySelectorAll("button")).filter((button) => - button.textContent?.includes("Request revisions"), - ).at(-1); - expect(saveButton?.hasAttribute("disabled")).toBe(false); + const sendButton = Array.from(host.querySelectorAll("button")).find((button) => + button.textContent?.includes("Send revision"), + ); + expect(sendButton?.hasAttribute("disabled")).toBe(false); await act(async () => { - saveButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + sendButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); - expect(host.textContent).toContain("A decline reason is required."); + expect(host.textContent).toContain("Add a note describing the changes you want."); + expect(onRejectInteraction).not.toHaveBeenCalled(); const textarea = host.querySelector("textarea") as HTMLTextAreaElement | null; expect(textarea).toBeTruthy(); @@ -368,12 +472,12 @@ describe("IssueThreadInteractionCard", () => { valueSetter?.call(textarea, "Needs a smaller phase split"); textarea!.dispatchEvent(new Event("input", { bubbles: true })); }); - const enabledSaveButton = Array.from(host.querySelectorAll("button")).filter((button) => - button.textContent?.includes("Request revisions"), - ).at(-1); - expect(enabledSaveButton?.hasAttribute("disabled")).toBe(false); + const enabledSendButton = Array.from(host.querySelectorAll("button")).find((button) => + button.textContent?.includes("Send revision"), + ); + expect(enabledSendButton?.hasAttribute("disabled")).toBe(false); await act(async () => { - enabledSaveButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + enabledSendButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); expect(onRejectInteraction).toHaveBeenCalledWith( @@ -403,6 +507,35 @@ describe("IssueThreadInteractionCard", () => { ); }); + it("standardizes the bare-reject button to Reject even when the payload carries a legacy rejectLabel", () => { + const host = renderCard({ + interaction: { + ...pendingRequestConfirmationInteraction, + payload: { + ...pendingRequestConfirmationInteraction.payload, + // Onboarding/plan-approval interactions are still seeded with the + // legacy "Request changes" reject label; it must not leak into the CTA. + rejectLabel: "Request changes", + rejectRequiresReason: false, + }, + }, + onAcceptInteraction: vi.fn(async () => undefined), + onRejectInteraction: vi.fn(async () => undefined), + }); + + const labels = Array.from(host.querySelectorAll("button")).map((button) => + button.textContent?.trim(), + ); + + // Canonical plan-approval grammar, right→left: Approve · Revise… · Reject. + // "Revise…" already carries the send-back-with-notes path, so a distinct + // "Request changes" word is redundant and must not render. + expect(labels).toContain("Reject"); + expect(labels).toContain("Revise…"); + expect(labels.some((label) => label?.includes("Approve"))).toBe(true); + expect(host.textContent).not.toContain("Request changes"); + }); + it("does not expose continuation wake policy labels in the card header", () => { const host = renderCard({ interaction: { @@ -450,8 +583,10 @@ describe("IssueThreadInteractionCard", () => { onRejectInteraction, }); + // The bare-reject button always renders the canonical "Reject", not the + // payload's "Keep it" — ConfirmationActionRow no longer honors the override. const declineButton = Array.from(host.querySelectorAll("button")).find((button) => - button.textContent?.includes("Keep it"), + button.textContent?.trim() === "Reject", ); expect(declineButton).toBeTruthy(); @@ -543,11 +678,11 @@ describe("IssueThreadInteractionCard", () => { onUploadImage, }); - const declineButton = Array.from(host.querySelectorAll("button")).find((button) => - button.textContent?.includes("Request revisions"), + const reviseButton = Array.from(host.querySelectorAll("button")).find((button) => + button.textContent?.includes("Revise"), ); await act(async () => { - declineButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + reviseButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); const attachButton = Array.from(host.querySelectorAll("button")).find((button) => @@ -570,11 +705,11 @@ describe("IssueThreadInteractionCard", () => { }); expect(onUploadImage).toHaveBeenCalledTimes(1); - const saveButton = Array.from(host.querySelectorAll("button")).filter((button) => - button.textContent?.includes("Request revisions"), - ).at(-1); + const sendButton = Array.from(host.querySelectorAll("button")).find((button) => + button.textContent?.includes("Send revision"), + ); await act(async () => { - saveButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + sendButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); expect(onRejectInteraction).toHaveBeenCalledWith( @@ -790,19 +925,19 @@ describe("IssueThreadInteractionCard tool-action card", () => { expect(host.textContent).not.toContain("Technical details"); }); - it("renders the agents-may-resolve policy badge and addressee chip", () => { + it("renders the addressee chip without the removed policy badge", () => { const host = renderCard({ interaction: agentAddressedRequestConfirmationInteraction, }); - const policyBadge = host.querySelector('[data-testid="interaction-policy-badge"]'); - expect(policyBadge?.textContent).toContain("Agents may resolve"); + // PAP-440: the "Agents may resolve" policy badge was pure noise — never rendered. + expect(host.querySelector('[data-testid="interaction-policy-badge"]')).toBeNull(); const addresseeBadge = host.querySelector('[data-testid="interaction-addressee-badge"]'); expect(addresseeBadge?.textContent).toContain("For "); }); - it("omits the policy and addressee badges for a board-only interaction", () => { + it("omits the addressee badge for a board-only interaction", () => { const host = renderCard({ interaction: pendingRequestConfirmationInteraction, }); diff --git a/ui/src/components/IssueThreadInteractionCard.tsx b/ui/src/components/IssueThreadInteractionCard.tsx index d705484268..804d2382eb 100644 --- a/ui/src/components/IssueThreadInteractionCard.tsx +++ b/ui/src/components/IssueThreadInteractionCard.tsx @@ -1,6 +1,6 @@ -import { useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useMemo, useRef, useState, type ReactNode } from "react"; import type { Agent } from "@paperclipai/shared"; -import { AlertTriangle, ArrowUpRight, Bot, Check, CheckCircle2, ChevronDown, ChevronRight, CircleDashed, Clock, ExternalLink, FileText, GitBranch, ImagePlus, Loader2, MessageSquareQuote, MinusCircle, ShieldAlert, ThumbsUp, TriangleAlert, Users, Wrench, X, XCircle } from "lucide-react"; +import { AlertTriangle, ArrowUpRight, Bot, Check, CheckCircle2, ChevronDown, ChevronRight, CircleDashed, Clock, ExternalLink, FileText, GitBranch, ImagePlus, Loader2, MessageSquareQuote, MinusCircle, ShieldAlert, ThumbsUp, TriangleAlert, Wrench, X, XCircle } from "lucide-react"; import { Link } from "@/lib/router"; import { formatAssigneeUserLabel } from "../lib/assignees"; import { @@ -10,6 +10,7 @@ import { getCheckboxConfirmationSelectedLabels, getItemVerdictProgress, getQuestionAnswerLabels, + shouldHideInteractionCard, normalizeRequestConfirmationTargetHref, type AskUserQuestionsAnswer, type AskUserQuestionsInteraction, @@ -989,8 +990,16 @@ function AskUserQuestionsCard({ ), ); - function toggleOption(questionId: string, optionId: string, selectionMode: "single" | "multi") { - if (optionId === OTHER_ANSWER_ID) { + function toggleOption( + questionId: string, + optionId: string, + selectionMode: "single" | "multi", + isFreeText = false, + ) { + // A free-text option is a first-class version of the built-in "Other" + // affordance: selecting it reveals the inline text field and its typed + // value is submitted as the question's `otherText`. + if (optionId === OTHER_ANSWER_ID || isFreeText) { setOtherActiveQuestions((current) => ({ ...current, [questionId]: !current[questionId], @@ -1064,7 +1073,11 @@ function AskUserQuestionsCard({ {interaction.status === "pending" ? (
- {questions.map((question, index) => ( + {questions.map((question, index) => { + const hasFreeTextOption = question.options.some( + (option) => option.freeText === true, + ); + return (
- {question.options.map((option) => ( - - toggleOption(question.id, option.id, question.selectionMode)} - /> - ))} + {question.options.map((option) => { + const isFreeText = option.freeText === true; + const optionSelected = isFreeText + ? otherActiveQuestions[question.id] === true + : (draftAnswers[question.id] ?? []).includes(option.id); + return ( +
+ + toggleOption(question.id, option.id, question.selectionMode, isFreeText)} + /> + {isFreeText && optionSelected ? ( +