From 4ad5520092ba32b9b0ef416dfc55bf6aae8ab3b4 Mon Sep 17 00:00:00 2001 From: scotttong Date: Fri, 7 Aug 2026 10:28:45 -0700 Subject: [PATCH] feat(onboarding): first-task onboarding experience Land onboarding on a real first task and make the opening exchange a conversation the user drives, rather than dropping them on the dashboard. - Land the wizard on a seeded first task instead of the dashboard; stop seeding default helper agents - Reframe the first-task prompt as welcome -> ask -> propose -> confirm -> act, and drop the boardroom/CEO framing - Seed a first-task greeting that addresses the user by the agent's chosen name and suppress the raw task-description bubble - Verify the first-task marker server-side: honor it only for a human (board) session creating a company's very first issue, so a client flag alone cannot make an ordinary issue author a comment as another agent or hide its description - Stop page-memory from bouncing the launch back to the dashboard - Task chat: bring back copy / thumbs-up / thumbs-down on the agent bubble footer, and anchor those actions to the summary line when a turn expands - Settle resolved interaction cards to their resolution time, hide withdrawn confirmations, and stabilize thread-card ordering - Collapse plan approval to a single checkbox card, surface the plan in the right sidebar, and auto-switch to the Plan tab - Unify plan-approval CTAs to Approve / Revise / Reject on one surface and remove the duplicate sticky action bar - Add a first-class free-text option to structured ask-user questions - Standardize the bare reject label to "Reject" - Hide the Properties sidebar on the first task until a plan exists - Skip drawing only unanswerable ask-user-question cards (no options and no free-text); answerable ones, including single-option, always render - Refine onboarding wizard copy and spacing; rename the org dropdown action to "Create new organization" --- packages/shared/src/constants.ts | 5 + packages/shared/src/index.ts | 1 + packages/shared/src/types/issue.ts | 8 + packages/shared/src/validators/issue.ts | 30 +- server/src/__tests__/built-in-agents.test.ts | 56 +- .../src/__tests__/companies-service.test.ts | 29 +- server/src/routes/issues.ts | 66 +- server/src/services/built-in-agents.ts | 16 + .../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 ++ .../planning-mode-visual-verification.spec.ts | 12 +- .../IssueThreadInteractionCard.test.tsx | 183 ++++- .../components/IssueThreadInteractionCard.tsx | 708 ++++++++++-------- ui/src/components/OnboardingWizard.tsx | 119 +-- ui/src/components/SidebarCompanyMenu.test.tsx | 6 +- ui/src/components/SidebarCompanyMenu.tsx | 7 +- ui/src/components/TaskChatThread.tsx | 63 +- .../IssuePlanConfirmationActionBar.tsx | 204 ----- .../issue-properties/IssueProperties.tsx | 16 +- .../IssuePropertiesPlansTab.tsx | 14 +- .../task-chat/TaskChatBubble.test.tsx | 59 ++ .../components/task-chat/TaskChatBubble.tsx | 25 +- .../task-chat/TaskChatBubbleActions.tsx | 66 ++ .../TaskChatInteractionCard.test.tsx | 12 +- .../task-chat/TaskChatThreadView.tsx | 29 +- .../task-chat/TaskChatTurn.test.tsx | 22 + ui/src/components/task-chat/TaskChatTurn.tsx | 85 ++- .../interaction-thread-order.test.ts | 125 ++++ .../task-chat/interaction-thread-order.ts | 79 ++ .../issueThreadInteractionFixtures.ts | 47 +- 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 | 138 +++- ui/src/lib/issue-thread-interactions.ts | 47 ++ ui/src/lib/onboarding-launch.test.ts | 2 + ui/src/lib/onboarding-launch.ts | 3 + ui/src/pages/IssueDetail.test.tsx | 82 ++ ui/src/pages/IssueDetail.tsx | 24 +- ui/src/pages/TaskChatLab.tsx | 28 +- 42 files changed, 2108 insertions(+), 776 deletions(-) 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/issue-properties/IssuePlanConfirmationActionBar.tsx create mode 100644 ui/src/components/task-chat/TaskChatBubbleActions.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 diff --git a/packages/shared/src/constants.ts b/packages/shared/src/constants.ts index a44dd79cb1b..9fa68c3b9d4 100644 --- a/packages/shared/src/constants.ts +++ b/packages/shared/src/constants.ts @@ -293,6 +293,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", @@ -302,6 +306,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/index.ts b/packages/shared/src/index.ts index 4073b8132f7..657cf0d9b3a 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -288,6 +288,7 @@ export { ISSUE_THREAD_INTERACTION_CONTINUATION_POLICIES, ISSUE_ORIGIN_KINDS, TASK_WATCHDOG_PRODUCT_BUG_ORIGIN_KIND, + ONBOARDING_FIRST_TASK_ORIGIN_KIND, ISSUE_WATCHDOG_DISCOVERY_KINDS, ISSUE_SURFACE_VISIBILITIES, ISSUE_RECOVERY_ACTION_KINDS, diff --git a/packages/shared/src/types/issue.ts b/packages/shared/src/types/issue.ts index 7ef906243b8..a1c87f386d4 100644 --- a/packages/shared/src/types/issue.ts +++ b/packages/shared/src/types/issue.ts @@ -1072,6 +1072,14 @@ export interface AskUserQuestionsQuestionOption { id: string; label: string; description?: string | null; + /** + * When true, selecting this option reveals an inline free-text field instead + * of acting as an inert choice. The typed answer is submitted as the + * question's `otherText`. Author at most one free-text option per question and + * do not add dead "I'll describe it" options that only duplicate the built-in + * free-text affordance. + */ + freeText?: boolean; } export interface AskUserQuestionsQuestion { diff --git a/packages/shared/src/validators/issue.ts b/packages/shared/src/validators/issue.ts index d50c88d2107..f000a7a11a1 100644 --- a/packages/shared/src/validators/issue.ts +++ b/packages/shared/src/validators/issue.ts @@ -489,13 +489,24 @@ const createIssueDuplicateGuardSchema = { .default(false), }; +// Narrow intent flag set by the onboarding wizard on the single first task. The +// server owns the resulting origin kind (clients cannot set arbitrary origin +// kinds) and uses it to seed the agent greeting instead of an LLM welcome step. +const onboardingFirstTaskMarkerSchema = { + onboardingFirstTask: z.boolean().optional(), +}; + export const createIssueInputSchema = createIssueBaseSchema.extend({ status: createIssueBaseSchema.shape.status.optional(), ...createIssueDuplicateGuardSchema, + ...onboardingFirstTaskMarkerSchema, }); export const createIssueSchema = withCreateIssueStatusDefault( - createIssueBaseSchema.extend(createIssueDuplicateGuardSchema), + createIssueBaseSchema.extend({ + ...createIssueDuplicateGuardSchema, + ...onboardingFirstTaskMarkerSchema, + }), ).superRefine(requireBlockedStatusForUnblockDescriptor); export type CreateIssue = z.infer; @@ -755,6 +766,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({ @@ -785,6 +802,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({ @@ -794,6 +812,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"], + }); + } + } } } }); diff --git a/server/src/__tests__/built-in-agents.test.ts b/server/src/__tests__/built-in-agents.test.ts index 5ab8b6dde07..880c53b30d4 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 c8b48ddb74b..e3aa0c7ae4d 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/routes/issues.ts b/server/src/routes/issues.ts index 036ea45b223..3e6ca87906a 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, @@ -164,6 +165,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, @@ -7520,7 +7525,30 @@ 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. + const onboardingFirstTaskRequested = + rawOnboardingFirstTask === true && req.actor.type === "board"; + const isOnboardingFirstTask = onboardingFirstTaskRequested + ? (await svc.count(companyId)) === 0 + : false; const watchdogDiscovery = normalizeWatchdogDiscovery(rawWatchdogDiscovery); const watchdogProductBugFollowUp = await resolveTaskWatchdogProductBugFollowUp( req, @@ -7575,6 +7603,9 @@ export function issueRoutes( ...(runWorkspaceInheritanceSourceIssueId ? { inheritExecutionWorkspaceFromIssueId: runWorkspaceInheritanceSourceIssueId } : {}), + ...(isOnboardingFirstTask && !watchdogProductBugFollowUp + ? { originKind: ONBOARDING_FIRST_TASK_ORIGIN_KIND } + : {}), ...(watchdogProductBugFollowUp ? { description: appendWatchdogDiscoveryContext({ @@ -7744,6 +7775,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 253686f7ba9..0714f3be3ec 100644 --- a/server/src/services/built-in-agents.ts +++ b/server/src/services/built-in-agents.ts @@ -470,6 +470,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"], @@ -1945,7 +1951,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/onboarding-greeting.test.ts b/server/src/services/onboarding-greeting.test.ts new file mode 100644 index 00000000000..4bb5f2550ee --- /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 00000000000..4c4ae5dc5e1 --- /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 e1a7096ed6b..3607dab5756 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: /Give it a heartbeat/ }).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 00000000000..3d3700502c2 --- /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/planning-mode-visual-verification.spec.ts b/tests/e2e/planning-mode-visual-verification.spec.ts index 65be76312de..dd38c8a4504 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", @@ -66,7 +72,9 @@ test("captures planning mode UI for desktop and mobile", async ({ page }) => { 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`); diff --git a/ui/src/components/IssueThreadInteractionCard.test.tsx b/ui/src/components/IssueThreadInteractionCard.test.tsx index 2091aeacfd1..b49bbd39ef5 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( diff --git a/ui/src/components/IssueThreadInteractionCard.tsx b/ui/src/components/IssueThreadInteractionCard.tsx index 633f2f650b8..a8f86d67444 100644 --- a/ui/src/components/IssueThreadInteractionCard.tsx +++ b/ui/src/components/IssueThreadInteractionCard.tsx @@ -1,4 +1,4 @@ -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 { Link } from "@/lib/router"; @@ -10,6 +10,7 @@ import { getCheckboxConfirmationSelectedLabels, getItemVerdictProgress, getQuestionAnswerLabels, + isDegenerateAskUserQuestions, normalizeRequestConfirmationTargetHref, type AskUserQuestionsAnswer, type AskUserQuestionsInteraction, @@ -987,8 +988,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], @@ -1062,7 +1071,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 ? ( +