mirror of
https://github.com/nexu-io/open-design.git
synced 2026-07-06 22:31:53 +08:00
* refactor(chat): unify ask-user flow on <question-form>, delete AskUserQuestion mechanism
There were two parallel "ask the user a clarifying question" mechanisms:
A. The AskUserQuestion SDK tool — an inline interactive card in the chat
stream (AskUserQuestionCard / StreamingAskUserQuestionCard), whose answer
was fed back into the still-open stream-json child via
POST /api/runs/:id/tool-result + submitChatRunToolResult.
B. The <question-form> markdown artifact — a QuestionsBanner entry point in
chat, the form rendered in the right-hand Questions tab
(QuestionsPanel / QuestionFormView), answers returned as the next user
message.
Two visual languages for the same job, and mechanism A's card was visually
broken (the text status "Awaiting your answer" was stuffed into the 24px
.op-status icon badge and overflowed onto the title). Mechanism A was pinned to
mid-conversation clarification while B was pinned to turn-1 discovery — an
artificial split.
This deletes mechanism A entirely and keeps only B, and widens <question-form>
from "turn-1 discovery only" to ANY turn so mid-conversation clarification flows
through the same banner → Questions tab → next user message loop.
Web:
- Delete apps/web/src/runtime/ask-user-question.ts (parser trio).
- ToolCard.tsx: drop the AUQ card/streaming-card/dispatch/imports and the
onAnswerToolUse / onSubmitForm props.
- AssistantMessage.tsx: drop the live AUQ render path,
suppressAskUserQuestionFallbackText, the AUQ name from SNAPSHOT_TOOL_NAMES,
and the onSubmitForm bridge. Keep suppressDuplicateQuestionForms.
- Delete the whole onSubmitForm / onAssistantFormSubmitStart return chain
across AssistantMessage / ChatPane / ProjectView / SideChatTab, and
submitChatRunToolResult in providers/daemon.ts.
Daemon (conservative — keep the generic stream-json input skeleton):
- applyClaudeStreamJsonRunBookkeeping: drop the AUQ detection branch and
run.pendingHostAnswers; stdin now closes on any non-tool_use clean terminal
turn.
- Delete run-tool-results.ts, submitToolResultToRun, and the
POST /api/runs/:id/tool-result endpoint.
- run-artifacts.ts:runAskedUserQuestion now scans streamed text for a
<question-form> marker (reassembled across text_delta chunks) instead of an
AUQ tool_use, preserving the run_finished.asked_user_question analytics
signal.
- Keep --input-format stream-json / promptInputFormat: 'stream-json' as generic
mid-turn input infra; refresh the comments that referenced AUQ.
System prompt / contracts:
- Replace the Claude-only AskUserQuestion clarification section with generic
"emit <question-form> for mid-conversation clarification" guidance; relax the
API-mode override wording; drop "Do not call AskUserQuestion" from the
skip-discovery override.
i18n / CSS:
- Remove the tool.askQuestion* keys from types.ts + all locales.
- Remove the op-ask-question* block and .op-status-awaiting from tools.css;
keep .op-status (the icon badge) and the qf-* classes that mechanism B uses.
Docs / tests:
- AGENTS.md: rewrite the runtime-conventions + chat-UI sections; add an
"Asking the user questions" section documenting the single mechanism.
- Delete the AUQ-only tests; rewrite the quiet-period bookkeeping, run-artifacts,
prompt, and plugin-folder tests to the new behavior.
Validation: pnpm guard (54), full typecheck, web tests, daemon suite
(4351 pass), contracts tests; replayed two Claude stream-json traces through the
mock CLI (incl. the [form answers — discovery] trace) — 0 parse failures, clean
end_turn termination.
* fix(chat): unify mid-conversation question-form across API/BYOK + analytics alias
Address review feedback on the ask-user unification:
- contracts composer (used by API/BYOK 'plain' mode) now mirrors the
daemon-side '## Clarifying questions mid-conversation' section and widens
the API_MODE_OVERRIDE allowed-output list from 'discovery on turn 1' to
'discovery (turn 1) and mid-conversation clarification', so BYOK/API runs
route follow-up choices through the unified Questions tab instead of
drifting back to plain markdown option lists. Makes the AGENTS.md
'mirrored through packages/contracts/src/prompts/system.ts' claim true.
- runAskedUserQuestion now matches the '<ask-question>' alias (already
whitelisted by the UI parser and daemon open-tag matcher), so a model that
drifts to the alias still records run_finished.asked_user_question instead
of being misclassified in the artifact funnel.
- Tests: contracts api-mode asserts mid-conversation forms are permitted in
both daemon and plain mode; run-artifacts asserts the alias is detected.
* fix(chat): make <ask-question> alias consistent across all paths + legacy AUQ renderer
Follow-ups from re-review of the ask-user unification:
- The <ask-question> alias is already accepted by the UI parser and the
daemon open-tag matcher, but two consumers still only recognized the
canonical <question-form>: db.listProjectsAwaitingInput's awaiting-input
filter and the next-turn transcript sanitizer. Both now match either tag,
so an alias-form turn (a) marks the project awaiting input and (b) gets
scrubbed from the prior assistant turn instead of replaying verbatim and
re-triggering the discovery-form loop.
- Re-add a read-only legacy AskUserQuestion renderer in ToolCard. The
interactive mechanism is gone, but persisted AUQ tool_use events survive
in upgraded chat history; without this they fell through to GenericCard and
surfaced the raw {"questions":[...]} JSON. The card now renders an inert
question summary (model-authored text, no new i18n keys, no submit path).
Tests: alias awaiting-input case (project-status), alias-form sanitizer
regression (sse), and legacy AUQ render + unparseable fallback (tool-renderers).
* fix(chat): correct asked_user_question detector + legacy AUQ answer rendering
Re-review follow-ups:
- runAskedUserQuestion read `data.text` for text_delta events, but the
persisted SSE shape carries the chunk on `delta` (packages/contracts/src/
sse/chat.ts), so production <question-form> runs appended nothing and
run_finished.asked_user_question was permanently false (the old tests passed
only because they built the wrong `text` field). Now reads `delta` (still
accepts `text` defensively).
- It also matched a bare open-tag, so a run that merely shows the literal
markup in a generated doc / code sample / HTML artifact could be
misclassified as a clarification turn and excluded from the artifact funnel.
Now requires a renderable closed block via a shared detector.
- Extracted the renderable-form detector + helpers out of server.ts into
apps/daemon/src/question-form-detect.ts so the missing-artifacts guard,
awaiting-input status, and run analytics share ONE check (the codebase
comment already anticipated promoting this once it had a third consumer).
The `<ask-question>` alias is covered by that shared matcher.
- LegacyAskUserQuestionCard now parses the persisted answer from
result.content and surfaces it ("question -> answer") so two historical AUQ
runs with different answers no longer render identically.
Tests: run-artifacts now uses the real `delta` shape + renderable bodies, adds
a literal-markup negative case; tool-renderers asserts the persisted answer is
surfaced.
---------
Co-authored-by: qiongyu1999 <2694684348@qq.com>
Co-authored-by: lefarcen <935902669@qq.com>
128 lines
5.9 KiB
TypeScript
128 lines
5.9 KiB
TypeScript
// Contract test for the prompts the plugin-folder card buttons send to the
|
|
// agent. `install` uses the simple shared template; `contribute` drives the
|
|
// `gh repo fork → branch → commit → gh pr create --web` flow against
|
|
// `nexu-io/open-design`; `publish` drives `gh repo create / push` against the
|
|
// author's own `plugin.repo` URL. The tests below lock the *shape* of each
|
|
// prompt (keywords + folder interpolation) without coupling to exact wording,
|
|
// so prose tweaks don't break the suite but accidental removal of a critical
|
|
// step would.
|
|
|
|
import { describe, expect, it } from 'vitest';
|
|
import { buildPluginFolderAgentActionPrompt } from '../../src/components/design-files/pluginFolderActions';
|
|
|
|
const FOLDER = 'generated-plugin';
|
|
|
|
describe('buildPluginFolderAgentActionPrompt', () => {
|
|
describe('install', () => {
|
|
it('mentions the folder path and the supported install CLI', () => {
|
|
const prompt = buildPluginFolderAgentActionPrompt(FOLDER, 'install');
|
|
expect(prompt).toContain(`Plugin folder: \`${FOLDER}\``);
|
|
expect(prompt).toContain('od plugin install --source');
|
|
});
|
|
});
|
|
|
|
describe('publish (repo-publish flow)', () => {
|
|
const prompt = buildPluginFolderAgentActionPrompt(FOLDER, 'publish');
|
|
|
|
it('delegates repo publishing to the deterministic plugin CLI helper', () => {
|
|
expect(prompt).toContain(`Plugin folder: \`${FOLDER}\``);
|
|
expect(prompt).toContain(`"$OD_NODE_BIN" "$OD_BIN" plugin publish-repo ${FOLDER}`);
|
|
expect(prompt).toMatch(/current gh login|target is not hard-coded/i);
|
|
});
|
|
|
|
it('states the CLI-owned responsibilities instead of re-listing shell steps', () => {
|
|
expect(prompt).toContain('gh api user --jq .login');
|
|
expect(prompt).toContain('--owner');
|
|
expect(prompt).toMatch(/last-resort fallback/i);
|
|
expect(prompt).toMatch(/placeholder owners are rejected/i);
|
|
expect(prompt).toMatch(/manifest repo normalization/i);
|
|
expect(prompt).toMatch(/repo existence check/i);
|
|
expect(prompt).toMatch(/repo create\/update/i);
|
|
expect(prompt).toMatch(/final verification/i);
|
|
});
|
|
|
|
it('bans the registry-submission CLI explicitly', () => {
|
|
// The legacy CLI is what shipped the bug — without an explicit ban
|
|
// the agent had been routing back to it. The mention must be in a
|
|
// negative imperative ("Do NOT call …"), not a recommendation.
|
|
expect(prompt).toMatch(
|
|
/Do NOT (call|route through) `?od plugin publish --to open-design`?/i,
|
|
);
|
|
expect(prompt).toMatch(
|
|
/registry[- ]submission|registry-submission flow|Open Design PR/i,
|
|
);
|
|
});
|
|
|
|
it('hard-bans mid-turn clarification + auto-install + force-push + retry', () => {
|
|
expect(prompt).toContain('<question-form>');
|
|
expect(prompt).toMatch(/fire-and-forget|clarification UI that waits/i);
|
|
expect(prompt).toMatch(/do not try to install/i);
|
|
expect(prompt).toMatch(/do not force-push|--force/i);
|
|
expect(prompt).toMatch(/do not retry/i);
|
|
});
|
|
|
|
it('interpolates the actual folder path into the CLI command', () => {
|
|
// Sanity check that template-string interpolation didn't regress into
|
|
// literal `${folderPath}` substrings.
|
|
expect(prompt).toContain(`plugin publish-repo ${FOLDER}`);
|
|
expect(prompt).not.toContain('${folderPath}');
|
|
});
|
|
|
|
it('ends by handing the repo URL back to chat', () => {
|
|
expect(prompt).toMatch(/final repo URL printed by the CLI/i);
|
|
});
|
|
});
|
|
|
|
describe('contribute (PR-based flow)', () => {
|
|
const prompt = buildPluginFolderAgentActionPrompt(FOLDER, 'contribute');
|
|
|
|
it('delegates Open Design PR creation to the deterministic plugin CLI helper', () => {
|
|
expect(prompt).toContain('nexu-io/open-design');
|
|
expect(prompt).toContain(`"$OD_NODE_BIN" "$OD_BIN" plugin open-design-pr ${FOLDER}`);
|
|
});
|
|
|
|
it('states the CLI-owned PR workflow instead of re-listing shell steps', () => {
|
|
expect(prompt).toContain('gh api user --jq .login');
|
|
expect(prompt).toContain('--owner');
|
|
expect(prompt).toMatch(/last-resort fallback/i);
|
|
expect(prompt).toMatch(/fork\/clone\/copy\/branch\/push/i);
|
|
expect(prompt).toContain('gh pr create --web');
|
|
// The legacy CLI is named in the prompt only as part of an explicit
|
|
// ban ("Do NOT call the legacy `od plugin publish --to open-design`")
|
|
// — verify the ban is in place, not the bare command.
|
|
expect(prompt).toMatch(/do not call the legacy `od plugin publish --to open-design`/i);
|
|
});
|
|
|
|
it('uses --web so the author confirms the PR in browser', () => {
|
|
// The "author keeps the final review click" invariant — preserved from
|
|
// 45f52d71's "We never POST anywhere" principle.
|
|
expect(prompt).toContain('--web');
|
|
expect(prompt).toMatch(/do not auto-submit/i);
|
|
});
|
|
|
|
it('hard-bans mid-turn clarification forms to avoid 600s stalls', () => {
|
|
// Regression guard for the stall we observed during e2e: agent paused
|
|
// mid-turn on a clarification waiting for a host answer the user never
|
|
// sent (they clicked the plugin-folder card instead).
|
|
expect(prompt).toContain('<question-form>');
|
|
expect(prompt).toMatch(/clarification UI that waits|fire-and-forget/i);
|
|
});
|
|
|
|
it('forbids the agent from installing tools or retrying failures', () => {
|
|
expect(prompt).toMatch(/do not try to install/i);
|
|
expect(prompt).toMatch(/do not retry/i);
|
|
});
|
|
|
|
it('interpolates the actual folder path into manifest and copy steps', () => {
|
|
// Sanity check that template-string interpolation didn't regress into
|
|
// literal `${folderPath}` substrings (we already shipped that bug once).
|
|
expect(prompt).toContain(`plugin open-design-pr ${FOLDER}`);
|
|
expect(prompt).not.toContain('${folderPath}');
|
|
});
|
|
|
|
it('ends by handing the PR URL back to chat', () => {
|
|
expect(prompt).toMatch(/PR URL|pull\/new|paste it into chat/);
|
|
});
|
|
});
|
|
});
|