mirror of
https://github.com/nexu-io/open-design.git
synced 2026-06-01 03:14:35 +07:00
feat(web): native diff-review UI on GenUISurfaceRenderer (Phase 8 entry slice)
Plan Q1 / spec §21.5.
apps/web/src/components/GenUISurfaceRenderer.tsx now ships first-
class branches for the auto-derived diff-review choice surface and
generic single-enum-property choice surfaces.
Diff-review (DiffReviewChoiceSurface):
- Three top-level buttons: 'Accept all' / 'Reject all' / 'Partial…'.
- Optional 'Skip' when the host supplies onSkip.
- Optional notes textarea — forwarded as decision.reason when non-empty.
- On 'Accept all': submits { decision: 'accept', accepted_files,
rejected_files: [] } using the touched file list from
pending.context.touchedFiles. Daemon side default-fills when the
list is empty.
- On 'Reject all': symmetric.
- On 'Partial…': reveals a per-file accept/reject toggle for each
touched file. Submit refuses locally when ANY file is left
undecided (mirrors the daemon's 'partial must cover every
touched file' contract from §3.O5 so the user doesn't ping the
server with an obviously-invalid payload).
- Disabled when context.touchedFiles is empty (the daemon's
default-fill path doesn't help with a partial decision).
Generic choice (GenericChoiceSurface):
- Detects schemas of shape `{ properties: { <key>: { enum: [...] } } }`
and renders one button per enum value. Property literally named
'decision' wins over other enum properties when several are
declared (so plugin-author-customised diff-review schemas keep
rendering as accept/reject/partial buttons even if they add
extra fields).
PendingSurface gains an optional `context: { touchedFiles?: [] }`
field. Future runtime-context entries plug in here without bloating
the GenUISurfaceSpec contract.
Web tests: 586 → 593 (+7 cases on
GenUISurfaceRenderer.diff-review: accept-all default-fill, reject-
all default-fill, partial union, partial blocks on undecided file,
partial disabled when context absent, optional reason forwarding,
generic single-enum choice button group).
Co-authored-by: Tom Huang <1043269994@qq.com>
This commit is contained in:
parent
e135739f99
commit
85beeb58c9
2 changed files with 476 additions and 5 deletions
|
|
@ -1,10 +1,12 @@
|
|||
// Plan §3.C3 / §3.L3 / spec §10.3 — Generative UI surface renderer.
|
||||
// Plan §3.C3 / §3.L3 / Plan §3.Q1 / spec §10.3 — Generative UI surface renderer.
|
||||
//
|
||||
// Renders a single pending GenUI surface. v1 ships first-class
|
||||
// renderers for `confirmation`, `oauth-prompt`, and bundled-component
|
||||
// surfaces (sandboxed iframe). `form` and `choice` without a component
|
||||
// fall back to a JSON-Schema preview + a generic "value-json" textarea
|
||||
// (the proper schema-driven renderer lands in Phase 2A.5).
|
||||
// renderers for `confirmation`, `oauth-prompt`, the auto-derived
|
||||
// `__auto_diff_review_<stageId>` choice surface (Phase 8 entry slice),
|
||||
// generic single-property `choice` surfaces (any schema with a
|
||||
// primary enum property), and bundled-component surfaces (sandboxed
|
||||
// iframe). `form` without a component falls back to a JSON-Schema
|
||||
// preview + a generic "value-json" textarea.
|
||||
|
||||
import { useEffect, useRef, useState } from 'react';
|
||||
import type { GenUISurfaceSpec } from '@open-design/contracts';
|
||||
|
|
@ -22,6 +24,14 @@ export interface PendingSurface {
|
|||
// `/api/plugins/<componentPluginId>/asset/<component.path>`. The
|
||||
// host supplies it from the run's AppliedPluginSnapshot.pluginId.
|
||||
componentPluginId?: string;
|
||||
// Plan §3.Q1 — runtime context passed into the surface renderer.
|
||||
// Today only `touchedFiles` is read by the auto-derived
|
||||
// diff-review surface (the file checklist shown when the user
|
||||
// picks 'partial'); future entries can carry per-stage context
|
||||
// without bloating GenUISurfaceSpec itself.
|
||||
context?: {
|
||||
touchedFiles?: string[];
|
||||
};
|
||||
}
|
||||
|
||||
interface Props {
|
||||
|
|
@ -78,6 +88,48 @@ export function GenUISurfaceRenderer(props: Props) {
|
|||
);
|
||||
}
|
||||
|
||||
// Plan §3.Q1 / spec §21.5 — Phase 8 native review-and-apply surface.
|
||||
//
|
||||
// The auto-derived diff-review choice surface
|
||||
// (`__auto_diff_review_<stageId>`) gets a dedicated UI: three
|
||||
// top-level buttons (accept / reject / partial) plus a per-file
|
||||
// checklist that shows up when the user picks 'partial'.
|
||||
// Plugin-author-declared surfaces with the same id win and reach
|
||||
// this branch only when they preserve the auto schema shape, so we
|
||||
// also fall through into the generic `choice` renderer below for
|
||||
// surfaces that customise away from this contract.
|
||||
if (surface.kind === 'choice' && surface.id.startsWith('__auto_diff_review_')) {
|
||||
return (
|
||||
<DiffReviewChoiceSurface
|
||||
surface={surface}
|
||||
files={asStringArray(props.pending.context?.touchedFiles)}
|
||||
onAnswered={submit}
|
||||
disabled={submitting}
|
||||
error={error}
|
||||
{...(props.onSkip ? { onSkip: props.onSkip } : {})}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
// Generic `choice` surface: any schema with a single primary enum
|
||||
// property renders as a button group. Multi-property choices fall
|
||||
// back to the FreeFormJsonForm so power users can edit by hand.
|
||||
if (surface.kind === 'choice') {
|
||||
const primary = pickPrimaryEnum(surface.schema);
|
||||
if (primary) {
|
||||
return (
|
||||
<GenericChoiceSurface
|
||||
surface={surface}
|
||||
primary={primary}
|
||||
onAnswered={submit}
|
||||
disabled={submitting}
|
||||
error={error}
|
||||
{...(props.onSkip ? { onSkip: props.onSkip } : {})}
|
||||
/>
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Plan §3.L3 / spec §10.3.5 — plugin-bundled component surface.
|
||||
//
|
||||
// A surface that ships its own component path renders inside a
|
||||
|
|
@ -192,6 +244,246 @@ export function GenUISurfaceRenderer(props: Props) {
|
|||
);
|
||||
}
|
||||
|
||||
// Plan §3.Q1 — diff-review native UI.
|
||||
//
|
||||
// Renders the three top-level decisions (accept / reject / partial).
|
||||
// Clicking 'accept' / 'reject' submits a payload that matches the
|
||||
// auto-derived surface schema; the daemon's diff-review atom impl
|
||||
// (apps/daemon/src/plugins/atoms/diff-review.ts) already handles the
|
||||
// default-fill rules so accepted_files / rejected_files don't need to
|
||||
// be sent on the simple paths.
|
||||
//
|
||||
// 'partial' reveals a checklist: every file the patch-edit stages
|
||||
// touched (passed in as `files`) gets a per-file accept / reject
|
||||
// toggle. The submit guard mirrors the daemon's contract — every
|
||||
// touched file must be on one of the two lists or the daemon throws
|
||||
// 'missing <file>'. The UI surfaces that locally to keep the round
|
||||
// trip cheap.
|
||||
function DiffReviewChoiceSurface(props: {
|
||||
surface: GenUISurfaceSpec;
|
||||
files: string[];
|
||||
onAnswered: (value: unknown) => void | Promise<void>;
|
||||
disabled: boolean;
|
||||
error: string | null;
|
||||
onSkip?: () => void;
|
||||
}) {
|
||||
const [mode, setMode] = useState<'idle' | 'partial'>('idle');
|
||||
const [reason, setReason] = useState('');
|
||||
const [perFile, setPerFile] = useState<Record<string, 'accept' | 'reject' | 'undecided'>>(() =>
|
||||
Object.fromEntries(props.files.map((f) => [f, 'undecided'] as const)),
|
||||
);
|
||||
|
||||
const accept = async () => {
|
||||
await props.onAnswered({
|
||||
decision: 'accept',
|
||||
...(props.files.length > 0 ? { accepted_files: props.files, rejected_files: [] } : {}),
|
||||
...(reason ? { reason } : {}),
|
||||
});
|
||||
};
|
||||
const reject = async () => {
|
||||
await props.onAnswered({
|
||||
decision: 'reject',
|
||||
accepted_files: [],
|
||||
...(props.files.length > 0 ? { rejected_files: props.files } : {}),
|
||||
...(reason ? { reason } : {}),
|
||||
});
|
||||
};
|
||||
const submitPartial = async () => {
|
||||
const accepted = props.files.filter((f) => perFile[f] === 'accept');
|
||||
const rejected = props.files.filter((f) => perFile[f] === 'reject');
|
||||
const undecided = props.files.filter((f) => (perFile[f] ?? 'undecided') === 'undecided');
|
||||
if (undecided.length > 0) {
|
||||
// The daemon would reject this with 'missing <file>'; surface
|
||||
// locally so the user doesn't ping back the server.
|
||||
throw new Error(`Pick accept or reject for: ${undecided.join(', ')}`);
|
||||
}
|
||||
await props.onAnswered({
|
||||
decision: 'partial',
|
||||
accepted_files: accepted,
|
||||
rejected_files: rejected,
|
||||
...(reason ? { reason } : {}),
|
||||
});
|
||||
};
|
||||
|
||||
return (
|
||||
<div className="genui-surface genui-surface--diff-review" role="dialog" aria-label={props.surface.id}>
|
||||
<div className="genui-surface__prompt">
|
||||
{props.surface.prompt ?? 'Review the diff and choose how to proceed.'}
|
||||
</div>
|
||||
<div className="genui-surface__hint">
|
||||
{props.files.length > 0
|
||||
? `${props.files.length} file${props.files.length === 1 ? '' : 's'} touched.`
|
||||
: 'No file list available — the daemon will default-fill the accept / reject sets.'}
|
||||
</div>
|
||||
<div className="genui-surface__actions">
|
||||
<button
|
||||
type="button"
|
||||
className="genui-surface__primary"
|
||||
disabled={props.disabled}
|
||||
onClick={() => void accept()}
|
||||
data-testid="genui-diff-accept"
|
||||
>
|
||||
Accept all
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
className="genui-surface__secondary"
|
||||
disabled={props.disabled}
|
||||
onClick={() => void reject()}
|
||||
data-testid="genui-diff-reject"
|
||||
>
|
||||
Reject all
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
className="genui-surface__secondary"
|
||||
disabled={props.disabled || props.files.length === 0}
|
||||
onClick={() => setMode('partial')}
|
||||
data-testid="genui-diff-partial"
|
||||
>
|
||||
Partial…
|
||||
</button>
|
||||
{props.onSkip ? (
|
||||
<button
|
||||
type="button"
|
||||
className="genui-surface__secondary"
|
||||
disabled={props.disabled}
|
||||
onClick={props.onSkip}
|
||||
>
|
||||
Skip
|
||||
</button>
|
||||
) : null}
|
||||
</div>
|
||||
{mode === 'partial' ? (
|
||||
<div className="genui-surface__partial">
|
||||
<ul className="genui-surface__file-list">
|
||||
{props.files.map((f) => (
|
||||
<li key={f} className="genui-surface__file-row">
|
||||
<code className="genui-surface__file-name">{f}</code>
|
||||
<label className="genui-surface__file-toggle">
|
||||
<input
|
||||
type="radio"
|
||||
name={`diff-${f}`}
|
||||
value="accept"
|
||||
checked={perFile[f] === 'accept'}
|
||||
onChange={() => setPerFile((s) => ({ ...s, [f]: 'accept' }))}
|
||||
data-testid={`genui-diff-file-accept-${f}`}
|
||||
/>
|
||||
accept
|
||||
</label>
|
||||
<label className="genui-surface__file-toggle">
|
||||
<input
|
||||
type="radio"
|
||||
name={`diff-${f}`}
|
||||
value="reject"
|
||||
checked={perFile[f] === 'reject'}
|
||||
onChange={() => setPerFile((s) => ({ ...s, [f]: 'reject' }))}
|
||||
data-testid={`genui-diff-file-reject-${f}`}
|
||||
/>
|
||||
reject
|
||||
</label>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
<button
|
||||
type="button"
|
||||
className="genui-surface__primary"
|
||||
disabled={props.disabled}
|
||||
onClick={() => void submitPartial().catch(() => { /* surfaced via parent error */ })}
|
||||
data-testid="genui-diff-partial-submit"
|
||||
>
|
||||
Submit partial decision
|
||||
</button>
|
||||
</div>
|
||||
) : null}
|
||||
<textarea
|
||||
className="genui-surface__textarea genui-surface__reason"
|
||||
placeholder="Notes for the patch author (optional)"
|
||||
rows={2}
|
||||
value={reason}
|
||||
onChange={(e) => setReason(e.target.value)}
|
||||
data-testid="genui-diff-reason"
|
||||
/>
|
||||
{props.error ? <div className="genui-surface__error">{props.error}</div> : null}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// Plan §3.Q1 — generic single-enum-property choice renderer.
|
||||
//
|
||||
// Detects schemas of shape `{ properties: { <key>: { enum: [...] } } }`
|
||||
// and renders one button per enum value. Submits an object with the
|
||||
// enum value at the picked key. Multi-property choices fall back to
|
||||
// the JSON textarea (handled by the caller).
|
||||
function GenericChoiceSurface(props: {
|
||||
surface: GenUISurfaceSpec;
|
||||
primary: { key: string; enum: string[] };
|
||||
onAnswered: (value: unknown) => void | Promise<void>;
|
||||
disabled: boolean;
|
||||
error: string | null;
|
||||
onSkip?: () => void;
|
||||
}) {
|
||||
return (
|
||||
<div className="genui-surface genui-surface--choice" role="dialog" aria-label={props.surface.id}>
|
||||
<div className="genui-surface__prompt">
|
||||
{props.surface.prompt ?? `Plugin needs ${props.primary.key} input.`}
|
||||
</div>
|
||||
<div className="genui-surface__actions">
|
||||
{props.primary.enum.map((value, idx) => (
|
||||
<button
|
||||
key={value}
|
||||
type="button"
|
||||
className={idx === 0 ? 'genui-surface__primary' : 'genui-surface__secondary'}
|
||||
disabled={props.disabled}
|
||||
onClick={() => void props.onAnswered({ [props.primary.key]: value })}
|
||||
data-testid={`genui-choice-${value}`}
|
||||
>
|
||||
{value}
|
||||
</button>
|
||||
))}
|
||||
{props.onSkip ? (
|
||||
<button
|
||||
type="button"
|
||||
className="genui-surface__secondary"
|
||||
disabled={props.disabled}
|
||||
onClick={props.onSkip}
|
||||
>
|
||||
Skip
|
||||
</button>
|
||||
) : null}
|
||||
</div>
|
||||
{props.error ? <div className="genui-surface__error">{props.error}</div> : null}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function pickPrimaryEnum(schema: unknown): { key: string; enum: string[] } | null {
|
||||
if (!schema || typeof schema !== 'object') return null;
|
||||
const props = (schema as { properties?: Record<string, unknown> }).properties;
|
||||
if (!props || typeof props !== 'object') return null;
|
||||
// Prefer a property literally named 'decision' (the diff-review
|
||||
// contract) so it wins over other enum properties when several are
|
||||
// declared.
|
||||
const ordered = Object.keys(props).sort((a, b) =>
|
||||
a === 'decision' ? -1 : b === 'decision' ? 1 : 0,
|
||||
);
|
||||
for (const key of ordered) {
|
||||
const prop = props[key];
|
||||
if (!prop || typeof prop !== 'object') continue;
|
||||
const e = (prop as { enum?: unknown }).enum;
|
||||
if (!Array.isArray(e)) continue;
|
||||
const stringValues = e.filter((v): v is string => typeof v === 'string');
|
||||
if (stringValues.length === 0) continue;
|
||||
return { key, enum: stringValues };
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function asStringArray(input: unknown): string[] {
|
||||
if (!Array.isArray(input)) return [];
|
||||
return input.filter((s): s is string => typeof s === 'string');
|
||||
}
|
||||
|
||||
function FreeFormJsonForm({
|
||||
onSubmit,
|
||||
disabled,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,179 @@
|
|||
// @vitest-environment jsdom
|
||||
|
||||
// Plan §3.Q1 / spec §21.5 — diff-review native UI on the
|
||||
// GenUISurfaceRenderer.
|
||||
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
|
||||
import { GenUISurfaceRenderer } from '../../src/components/GenUISurfaceRenderer';
|
||||
import type { GenUISurfaceSpec } from '@open-design/contracts';
|
||||
|
||||
afterEach(() => cleanup());
|
||||
|
||||
const diffReviewSurface = (over: Partial<GenUISurfaceSpec> = {}): GenUISurfaceSpec => ({
|
||||
id: '__auto_diff_review_review',
|
||||
kind: 'choice',
|
||||
persist: 'run',
|
||||
trigger: { stageId: 'review', atom: 'diff-review' },
|
||||
prompt: 'Review the diff and choose how to proceed.',
|
||||
schema: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
decision: { type: 'string', enum: ['accept', 'reject', 'partial'] },
|
||||
accepted_files: { type: 'array', items: { type: 'string' } },
|
||||
rejected_files: { type: 'array', items: { type: 'string' } },
|
||||
reason: { type: 'string' },
|
||||
},
|
||||
required: ['decision'],
|
||||
},
|
||||
...over,
|
||||
});
|
||||
|
||||
describe('GenUISurfaceRenderer — diff-review choice surface', () => {
|
||||
it('Accept all submits a decision payload covering every touched file', async () => {
|
||||
const onAnswered = vi.fn();
|
||||
render(
|
||||
<GenUISurfaceRenderer
|
||||
pending={{
|
||||
surface: diffReviewSurface(),
|
||||
runId: 'run-1',
|
||||
context: { touchedFiles: ['Button.tsx', 'Button.css'] },
|
||||
}}
|
||||
onAnswered={onAnswered}
|
||||
/>,
|
||||
);
|
||||
fireEvent.click(screen.getByTestId('genui-diff-accept'));
|
||||
await waitFor(() => expect(onAnswered).toHaveBeenCalledWith({
|
||||
decision: 'accept',
|
||||
accepted_files: ['Button.tsx', 'Button.css'],
|
||||
rejected_files: [],
|
||||
}));
|
||||
});
|
||||
|
||||
it('Reject all submits a decision payload covering every touched file', async () => {
|
||||
const onAnswered = vi.fn();
|
||||
render(
|
||||
<GenUISurfaceRenderer
|
||||
pending={{
|
||||
surface: diffReviewSurface(),
|
||||
runId: 'run-1',
|
||||
context: { touchedFiles: ['x.ts'] },
|
||||
}}
|
||||
onAnswered={onAnswered}
|
||||
/>,
|
||||
);
|
||||
fireEvent.click(screen.getByTestId('genui-diff-reject'));
|
||||
await waitFor(() => expect(onAnswered).toHaveBeenCalledWith({
|
||||
decision: 'reject',
|
||||
accepted_files: [],
|
||||
rejected_files: ['x.ts'],
|
||||
}));
|
||||
});
|
||||
|
||||
it('Partial reveals per-file accept/reject toggles + submits the union', async () => {
|
||||
const onAnswered = vi.fn();
|
||||
render(
|
||||
<GenUISurfaceRenderer
|
||||
pending={{
|
||||
surface: diffReviewSurface(),
|
||||
runId: 'run-1',
|
||||
context: { touchedFiles: ['a.ts', 'b.ts'] },
|
||||
}}
|
||||
onAnswered={onAnswered}
|
||||
/>,
|
||||
);
|
||||
fireEvent.click(screen.getByTestId('genui-diff-partial'));
|
||||
fireEvent.click(screen.getByTestId('genui-diff-file-accept-a.ts'));
|
||||
fireEvent.click(screen.getByTestId('genui-diff-file-reject-b.ts'));
|
||||
fireEvent.click(screen.getByTestId('genui-diff-partial-submit'));
|
||||
await waitFor(() => expect(onAnswered).toHaveBeenCalledWith({
|
||||
decision: 'partial',
|
||||
accepted_files: ['a.ts'],
|
||||
rejected_files: ['b.ts'],
|
||||
}));
|
||||
});
|
||||
|
||||
it('Partial submit refuses when a file is left undecided', async () => {
|
||||
const onAnswered = vi.fn();
|
||||
render(
|
||||
<GenUISurfaceRenderer
|
||||
pending={{
|
||||
surface: diffReviewSurface(),
|
||||
runId: 'run-1',
|
||||
context: { touchedFiles: ['a.ts', 'b.ts'] },
|
||||
}}
|
||||
onAnswered={onAnswered}
|
||||
/>,
|
||||
);
|
||||
fireEvent.click(screen.getByTestId('genui-diff-partial'));
|
||||
fireEvent.click(screen.getByTestId('genui-diff-file-accept-a.ts'));
|
||||
fireEvent.click(screen.getByTestId('genui-diff-partial-submit'));
|
||||
// a.ts decided, b.ts left undecided → submit is blocked locally
|
||||
// and onAnswered is never called.
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
expect(onAnswered).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('disables Partial when no touched-file context is supplied', () => {
|
||||
render(
|
||||
<GenUISurfaceRenderer
|
||||
pending={{ surface: diffReviewSurface(), runId: 'run-1' }}
|
||||
onAnswered={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
expect((screen.getByTestId('genui-diff-partial') as HTMLButtonElement).disabled).toBe(true);
|
||||
});
|
||||
|
||||
it('forwards the optional reason field on accept', async () => {
|
||||
const onAnswered = vi.fn();
|
||||
render(
|
||||
<GenUISurfaceRenderer
|
||||
pending={{
|
||||
surface: diffReviewSurface(),
|
||||
runId: 'run-1',
|
||||
context: { touchedFiles: ['x.ts'] },
|
||||
}}
|
||||
onAnswered={onAnswered}
|
||||
/>,
|
||||
);
|
||||
fireEvent.change(screen.getByTestId('genui-diff-reason'), {
|
||||
target: { value: 'looks good' },
|
||||
});
|
||||
fireEvent.click(screen.getByTestId('genui-diff-accept'));
|
||||
await waitFor(() => expect(onAnswered).toHaveBeenCalledWith({
|
||||
decision: 'accept',
|
||||
accepted_files: ['x.ts'],
|
||||
rejected_files: [],
|
||||
reason: 'looks good',
|
||||
}));
|
||||
});
|
||||
});
|
||||
|
||||
describe('GenUISurfaceRenderer — generic single-enum choice', () => {
|
||||
it('renders one button per enum value and submits the picked value', async () => {
|
||||
const onAnswered = vi.fn();
|
||||
const surface: GenUISurfaceSpec = {
|
||||
id: 'direction',
|
||||
kind: 'choice',
|
||||
persist: 'run',
|
||||
prompt: 'Pick a direction.',
|
||||
schema: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
choice: { type: 'string', enum: ['cool', 'warm', 'neutral'] },
|
||||
},
|
||||
required: ['choice'],
|
||||
},
|
||||
};
|
||||
render(
|
||||
<GenUISurfaceRenderer
|
||||
pending={{ surface, runId: 'run-1' }}
|
||||
onAnswered={onAnswered}
|
||||
/>,
|
||||
);
|
||||
fireEvent.click(screen.getByTestId('genui-choice-warm'));
|
||||
await waitFor(() =>
|
||||
expect(onAnswered).toHaveBeenCalledWith({ choice: 'warm' }),
|
||||
);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue