Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { useSubmitCopilotFeedback } from '@/hooks/queries/copilot-feedback'
import { useForkMothershipChat } from '@/hooks/queries/mothership-chats'
import { useFolderStore } from '@/stores/folders/store'

const SPECIAL_TAGS = 'thinking|options|usage_upgrade|credential|mothership-error|file'
const SPECIAL_TAGS = 'thinking|options|usage_upgrade|credential|mothership-error|file|question'

function toPlainText(raw: string): string {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ export type { AgentGroupItem, NestedAgentGroup } from './agent-group'
export { AgentGroup, CircleStop, isAgentGroupResolved } from './agent-group'
export { ChatContent } from './chat-content'
export { Options } from './options'
export { QuestionDisplay } from './question'
export { PendingTagIndicator, parseSpecialTags, SpecialTags } from './special-tags'
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { formatQuestionAnswerMessage, QuestionDisplay } from './question'
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import { formatQuestionAnswerMessage } from '@/app/workspace/[workspaceId]/home/components/message-content/components/question/question'
import type { QuestionItem } from '@/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags'

const QUESTIONS: QuestionItem[] = [
{
type: 'single_select',
prompt: 'How should I handle the duplicates?',
options: [{ id: 'keep_newest', label: 'Keep the newest entry' }],
},
{
type: 'confirm',
prompt: 'Delete 4 archived workflows?',
options: [
{ id: 'yes', label: 'Delete them' },
{ id: 'no', label: 'Cancel' },
],
},
{ type: 'text', prompt: 'What time zone should the daily report run in?' },
]

describe('formatQuestionAnswerMessage', () => {
it('sends just the answer for a single question', () => {
expect(formatQuestionAnswerMessage([QUESTIONS[0]], ['Keep the newest entry'])).toBe(
'Keep the newest entry'
)
})

it('sends one prompt-answer line per question for multi-step batches', () => {
expect(formatQuestionAnswerMessage(QUESTIONS, ['Keep the newest entry', 'Cancel', 'EST'])).toBe(
'How should I handle the duplicates? — Keep the newest entry\n' +
'Delete 4 archived workflows? — Cancel\n' +
'What time zone should the daily report run in? — EST'
)
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
'use client'

import { useState } from 'react'
import { ArrowRight, Button, ChevronLeft, ChevronRight, cn, X } from '@sim/emcn'
import type { QuestionItem } from '@/app/workspace/[workspaceId]/home/components/message-content/components/special-tags'

/**
* Builds the single user message sent after the final question is answered.
* A lone question sends just the answer text; a multi-step batch sends one
* `Prompt — Answer` line per question.
*/
export function formatQuestionAnswerMessage(questions: QuestionItem[], answers: string[]): string {
if (questions.length === 1) return answers[0] ?? ''
return questions.map((q, i) => `${q.prompt} — ${answers[i] ?? ''}`).join('\n')
}
Comment on lines +12 to +15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unsafe array access in single-question branch: answers[0] returns undefined when the caller passes an empty array, but the declared return type is string. TypeScript won't catch this without noUncheckedIndexedAccess. Adding an explicit guard closes the gap and makes the intent self-documenting.

Suggested change
export function formatQuestionAnswerMessage(questions: QuestionItem[], answers: string[]): string {
if (questions.length === 1) return answers[0]
return questions.map((q, i) => `${q.prompt}${answers[i]}`).join('\n')
}
export function formatQuestionAnswerMessage(questions: QuestionItem[], answers: string[]): string {
if (questions.length === 1) return answers[0] ?? ''
return questions.map((q, i) => `${q.prompt}${answers[i] ?? ''}`).join('\n')
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in 85e8dec — the only caller passes a parallel non-empty array, but the guard is cheap and makes the exported helper safe standalone.


/**
* The free-text input's initial value when (re)visiting a question: restore a
* previously typed answer, but not one that matches an option row (that row is
* highlighted instead).
*/
function freeTextPrefillFor(question: QuestionItem, answer: string | null): string {
if (!answer) return ''
if (question.type === 'text') return answer
return question.options?.some((o) => o.label === answer) ? '' : answer
}

const OPTION_ROW_CLASSES =
'flex items-center gap-2 border-[var(--divider)] px-2 py-2 text-left transition-colors'

/** Ghost icon-button chrome shared by the stepper chevrons and the dismiss X. */
const ICON_BUTTON_CLASSES = 'relative size-[14px] flex-shrink-0 p-0'

/** Leading number slot matching the suggested follow-ups rows. */
function RowNumber({ value }: { value: number }) {
return (
<div className='flex size-[16px] flex-shrink-0 items-center justify-center'>
<span className='text-[var(--text-icon)] text-sm'>{value}</span>
</div>
)
}

type QuestionPhase = 'active' | 'answered' | 'dismissed'

interface QuestionDisplayProps {
data: QuestionItem[]
/** Sends the combined answer as a user message; undefined renders the div inert. */
onSelect?: (message: string) => void
}

/**
* Inline renderer for the `<question>` special tag: a chat-inline div with the
* user input's chrome, the current question's prompt at the top left, dismiss
* (and a `‹ N of M ›` stepper for multi-step batches) at the top right, and
* suggested-action option rows beneath. `single_select` always appends a
* free-text "Something else" row; `text` renders only the free-text row.
* Answers collect locally; answering the last question sends one combined
* user message and collapses the div to a question/answer recap.
*/
export function QuestionDisplay({ data, onSelect }: QuestionDisplayProps) {
const disabled = !onSelect
const [phase, setPhase] = useState<QuestionPhase>('active')
const [step, setStep] = useState(0)
const [answers, setAnswers] = useState<(string | null)[]>(() => data.map(() => null))
const [freeText, setFreeText] = useState('')

if (data.length === 0 || phase === 'dismissed') return null

const containerClasses =
'rounded-2xl border border-[var(--border-1)] bg-[var(--white)] px-2.5 py-2 dark:bg-[var(--surface-4)]'

if (phase === 'answered') {
return (
<div className={containerClasses}>
{data.map((question, i) => (
<div key={i} className='px-2 py-2'>
<p className='text-[var(--text-primary)] text-sm'>{question.prompt}</p>
<p className='mt-1.5 text-[var(--text-muted)] text-sm'>{answers[i]}</p>
</div>
))}
</div>
)
}

const question = data[step]
const isLast = step === data.length - 1
const options = question.type === 'text' ? [] : (question.options ?? [])
const hasFreeText = question.type !== 'confirm'

const goToStep = (next: number) => {
setStep(next)
setFreeText(freeTextPrefillFor(data[next], answers[next]))
}

const handleAnswer = (answer: string) => {
const next = [...answers]
next[step] = answer
setAnswers(next)
if (!isLast) {
goToStep(step + 1)
return
}
setPhase('answered')
onSelect?.(
formatQuestionAnswerMessage(
data,
next.map((a) => a ?? '')
)
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Earlier edits keep stale answers

Medium Severity

In a multi-step batch, changing an answer on an earlier step updates only that index in answers. Later steps keep prior selections, and the user can step forward via the chevrons and submit from the last question without revisiting them, so formatQuestionAnswerMessage may combine a new early answer with outdated later ones.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8a9be60. Configure here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intended behavior. Back-navigation is a review/edit affordance: changing an earlier answer shouldn't discard later answers the user already made deliberately (they stay visibly highlighted when stepping through, standard wizard semantics). Submit only fires by explicitly answering the last question, so nothing is sent without the user's final action.

}

const canSubmitFreeText = !disabled && freeText.trim().length > 0

return (
<div className={containerClasses}>
<div className='flex items-center justify-between gap-2 px-2 py-2'>
<p className='min-w-0 flex-1 break-words text-[var(--text-primary)] text-sm'>
{question.prompt}
</p>
<div className='flex items-center gap-3'>
{data.length > 1 && (
<div className='flex items-center gap-2'>
<Button
type='button'
variant='ghost'
onClick={() => goToStep(step - 1)}
disabled={step === 0}
className={cn(
ICON_BUTTON_CLASSES,
'before:absolute before:inset-[-8px] before:content-[""] disabled:opacity-50'
)}
>
<ChevronLeft className='h-[9px] w-[7px] text-[var(--text-icon)]' />
<span className='sr-only'>Previous question</span>
</Button>
<span className='whitespace-nowrap text-[var(--text-muted)] text-sm tabular-nums'>
{step + 1} of {data.length}
</span>
<Button
type='button'
variant='ghost'
onClick={() => goToStep(step + 1)}
// Inert renders (older messages) browse freely; interactive ones
// gate forward movement on the current question being answered.
disabled={isLast || (!disabled && answers[step] === null)}
className={cn(
ICON_BUTTON_CLASSES,
'before:absolute before:inset-[-8px] before:content-[""] disabled:opacity-50'
)}
>
<ChevronRight className='h-[9px] w-[7px] text-[var(--text-icon)]' />
Comment on lines +122 to +151

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Stepper chevrons not gated by the disabled prop

The back/forward chevrons only check step === 0 and isLast || answers[step] === null — they never check disabled. In inert mode (older messages, onSelect undefined) this works out because answers is all-null so the forward button is always disabled and step starts at 0 so back is also disabled, but the dependency is implicit. If the initialization logic ever changes (e.g. answers pre-populated from a persisted state), the stepper would become interactive on messages that should be read-only. Passing disabled={disabled || step === 0} and disabled={disabled || isLast || answers[step] === null} makes the invariant explicit.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional, and made explicit in a6890d3: in inert mode the stepper deliberately stays browsable so historical multi-step batches can show all their prompts (Bugbot flagged the opposite problem — inert renders trapping users on question 1). Stepping is pure local navigation; answering is what's disabled. The forward chevron now reads disabled={isLast || (!disabled && answers[step] === null)} with a comment stating the invariant.

<span className='sr-only'>Next question</span>
</Button>
</div>
)}
{!disabled && (
<Button
type='button'
variant='ghost'
onClick={() => setPhase('dismissed')}
className={cn(
ICON_BUTTON_CLASSES,
'before:absolute before:inset-[-14px] before:content-[""]'
)}
>
<X className='size-[14px] text-[var(--text-icon)]' />
<span className='sr-only'>Dismiss</span>
</Button>
)}
</div>
</div>
<div className='flex flex-col'>
{options.map((option, i) => (
<button
key={option.id}
type='button'
disabled={disabled}
onClick={() => handleAnswer(option.label)}
className={cn(
OPTION_ROW_CLASSES,
disabled ? 'cursor-not-allowed' : 'hover-hover:bg-[var(--surface-5)]',
i > 0 && 'border-t',
answers[step] === option.label && 'bg-[var(--surface-5)]'
)}
>
<RowNumber value={i + 1} />
<span className='flex-1 truncate text-[var(--text-body)] text-sm'>{option.label}</span>
<ArrowRight className='size-[16px] shrink-0 text-[var(--text-icon)]' />
</button>
))}
{hasFreeText && (
<div className={cn(OPTION_ROW_CLASSES, options.length > 0 && 'border-t')}>
<RowNumber value={options.length + 1} />
<input
type='text'
value={freeText}
disabled={disabled}
onChange={(e) => setFreeText(e.target.value)}
onKeyDown={(e) => {
if (e.key === 'Enter' && canSubmitFreeText) {
e.preventDefault()
handleAnswer(freeText.trim())
}
}}
placeholder={question.type === 'text' ? 'Type an answer' : 'Something else'}
aria-label={question.prompt}
className='min-w-0 flex-1 border-0 bg-transparent p-0 text-[var(--text-body)] text-sm outline-none placeholder:text-[var(--text-muted)] disabled:cursor-not-allowed'
/>
<button
type='button'
aria-label='Submit answer'
disabled={!canSubmitFreeText}
onClick={() => handleAnswer(freeText.trim())}
className='disabled:cursor-default'
>
<ArrowRight
className={cn(
'size-[16px] shrink-0 transition-colors',
canSubmitFreeText ? 'text-[var(--text-body)]' : 'text-[var(--text-icon)]'
)}
/>
</button>
</div>
)}
</div>
</div>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ export type {
MothershipErrorTagData,
OptionsTagData,
ParsedSpecialContent,
QuestionItem,
QuestionOption,
QuestionTagData,
QuestionType,
RuntimeSpecialTagName,
UsageUpgradeAction,
UsageUpgradeTagData,
Expand All @@ -17,9 +21,11 @@ export {
PendingTagIndicator,
parseFileTag,
parseJsonTagBody,
parseQuestionTagBody,
parseSpecialTags,
parseTagAttributes,
parseTextTagBody,
QUESTION_TYPES,
SpecialTags,
USAGE_UPGRADE_ACTIONS,
WORKSPACE_RESOURCE_TAG_TYPES,
Expand Down
Loading
Loading