Repository navigation
feat(mothership): let a question card pick from the workspace's resources - #8735
waleedlatif1 wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
|
There was a problem hiding this comment.
1 issue found across 10 files
Confidence score: 3/5
resource-question-rows.tsxcan show candidates from the previous workspace during a workspace switch, and users can select one in the current chat. Don’t keep previous data across workspace changes, or disable selection until the new workspace’s data loads.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/question/resource-question-rows.tsx">
<violation number="1" location="apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/question/resource-question-rows.tsx:39">
P2: These workspace-scoped list hooks keep previous data when their keys change, so this renders the prior workspace’s candidates during a workspace switch and allows selecting one in the current chat. Ignore placeholder data and treat it as loading until the current workspace’s list resolves.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Turn on auto-fix | Re-trigger cubic
| type | ||
| ] | ||
| const candidates = useMemo( | ||
| () => (query.data ?? []).map((item) => ({ id: item.id, name: item.name })), |
There was a problem hiding this comment.
P2: These workspace-scoped list hooks keep previous data when their keys change, so this renders the prior workspace’s candidates during a workspace switch and allows selecting one in the current chat. Ignore placeholder data and treat it as loading until the current workspace’s list resolves.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/question/resource-question-rows.tsx, line 39:
<comment>These workspace-scoped list hooks keep previous data when their keys change, so this renders the prior workspace’s candidates during a workspace switch and allows selecting one in the current chat. Ignore placeholder data and treat it as loading until the current workspace’s list resolves.</comment>
<file context>
@@ -0,0 +1,124 @@
+ type
+ ]
+ const candidates = useMemo(
+ () => (query.data ?? []).map((item) => ({ id: item.id, name: item.name })),
+ [query.data]
+ )
</file context>
|
| resourceType={question.resourceType} | ||
| query={freeText} | ||
| disabled={disabled} | ||
| onPick={(resource) => handleSingleSelect(resource.title, resource)} |
There was a problem hiding this comment.
Multiline names lose the recap
Picking a workflow whose name contains a newline breaks the answered recap after reload. Workflow names allow internal newlines, but this passes the name unchanged into an answer format that requires one line per question. parseQuestionAnswerMessage returns null, so the chat leaves the question unpaired and shows a separate answer bubble.
Normalize line breaks in the picked label before formatting the answer. Keep the resource context unchanged.
| } | ||
|
|
||
| return ( | ||
| <div className='max-h-[180px] overflow-y-auto'> |
There was a problem hiding this comment.
The new scroll box hides resource rows without the required edge treatment. The styling guide requires useScrollEdges, scrollFadeClass, and scrollFadeAttributes for regions that hide rows beyond an edge.
Apply those helpers so users can see when more resources are offscreen. This repository requirement must be satisfied before merging.
Context Used: Tailwind CSS and styling conventions (source)
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!
| expect(rowLabels()).toEqual(WORKFLOWS.map((workflow) => workflow.name)) | ||
| expect(mockUseWorkflows).toHaveBeenCalledWith('ws-1', { enabled: true }) | ||
| expect(mockUseTablesList).toHaveBeenCalledWith('ws-1', 'active', { enabled: false }) |
There was a problem hiding this comment.
Tests use forbidden assertions
This test checks rendered labels and calls to mocked hooks. CLAUDE.md explicitly forbids mock-call and rendered-text assertions. The preview and recap tests also check rendered text.
Replace these checks with end-to-end proof that a picked resource reaches the chat request and survives transcript reload. This testing requirement must be satisfied before merging.
Context Used: CLAUDE.md (source)
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!
Summary
<question>kind:{"type": "resource_select", "prompt": string, "resourceType": "workflow" | "table" | "file" | "knowledgebase"}. It has nooptions. The card fetches that family's live list for the current workspace and renders each row with the add-resource menu's row (renderDropdownItem,resourceFromItem). It has no 4-option cap.prompt — answerline. The chosen resource is attached as a chat context (mapResourceToContext), the same way a typed @-mention is, so the agent gets the id and not just the name. To support this,onOptionSelectis nowInteractionAnswerHandler (message, contexts?)and it is threaded through toonSubmit.prompt — answertext.resource_selectbody fails validation and falls back to showing the prompt as plain text, which is today's behaviour. Organization chats have no workspace to list, so they get the free-text row only, and the worker does not offer the kind there.Companion: https://github.com/simstudioai/mothership/pull/618
Type of Change
Testing
question/resource-question.test.tsx(new, jsdom), 6 tests:{kind: 'workflow', workflowId, label}context, andparseQuestionAnswerMessageround-trips itspecial-tags.test.ts:resource_selectparses with no options and drops stray fields. An unknown or missingresourceTypeis rejected.tsc(apps/sim),bun run lintandbun run check:audits(58 audits) all pass. The unused-exports baseline shrank by the 4 retiredQUESTION_TYPES/QuestionTypeentries.bun run test: 35,207 pass and 1 fails. The failure isremark-plain-text.test.ts, a 10 s timing test that timed out because the machine was under load. It passes alone, both on this branch and on the base.Checklist