From c65f9ccf7a3ffc2bc98519f8f4260369921c860a Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 9 May 2026 15:30:45 +0000 Subject: [PATCH] feat(plugins): auto-derive choice surface for diff-review (Phase 8 entry slice) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plan P3 / spec §10.3.1 / §21.5. Mirrors the connector-gate's auto oauth-prompt pattern: when the EFFECTIVE pipeline (consumer-declared OR scenario-fallback) contains a stage whose atoms[] list `diff-review`, applyPlugin() now synthesises an implicit `choice` GenUI surface so the user can accept / reject / partial without the plugin author having to declare the surface by hand. apps/daemon/src/plugins/atoms/auto-surfaces.ts: deriveAutoAtomSurfaces({ pipeline }) → GenUISurfaceSpec[] For each stage that lists 'diff-review', emits one surface: id: '__auto_diff_review_' kind: 'choice' persist: 'run' (per-run; user sees one prompt per cycle through the stage) trigger: { stageId, atom: 'diff-review' } schema: { decision: ['accept' | 'reject' | 'partial'], accepted_files: string[], rejected_files: string[], reason: string } required: ['decision'] timeout: 24 h (a diff review may sit overnight) onTimeout: 'abort' apply.ts integration: - Auto-surface derivation moved AFTER the bundled-scenario fallback resolution (P3 / O1) so a consumer that inherits the bundled scenario's diff-review stage still gets the auto-surface. - Plugin-author-declared surface with the same id wins, via the existing mergeAutoOAuthPrompts dedupe (re-used so we don't duplicate dedupe logic). Daemon tests: 1608 → 1614 (+6 cases on plugins-auto-surfaces: multi-stage diff-review surface fan-out, no-pipeline returns empty, no-diff-review returns empty, plugin-declared pipeline integration, scenario-fallback pipeline integration, plugin- author-declared surface with same id wins). Co-authored-by: Tom Huang <1043269994@qq.com> --- apps/daemon/src/plugins/apply.ts | 23 ++- .../daemon/src/plugins/atoms/auto-surfaces.ts | 77 ++++++++++ .../tests/plugins-auto-surfaces.test.ts | 138 ++++++++++++++++++ 3 files changed, 232 insertions(+), 6 deletions(-) create mode 100644 apps/daemon/src/plugins/atoms/auto-surfaces.ts create mode 100644 apps/daemon/tests/plugins-auto-surfaces.test.ts diff --git a/apps/daemon/src/plugins/apply.ts b/apps/daemon/src/plugins/apply.ts index 8c678dd33..779e0192f 100644 --- a/apps/daemon/src/plugins/apply.ts +++ b/apps/daemon/src/plugins/apply.ts @@ -39,6 +39,7 @@ import { resolveConnectorBindings, type ConnectorProbe, } from './connector-gate.js'; +import { deriveAutoAtomSurfaces } from './atoms/auto-surfaces.js'; export class MissingInputError extends Error { readonly fields: string[]; @@ -111,12 +112,6 @@ export function applyPlugin(input: ApplyInput): ApplyComputed { resolveConnectorBindings(manifest, input.connectorProbe); const required = requiredCapabilities(manifest); const granted = resolveCapabilitiesGranted({ manifest, trust }); - const declaredSurfaces = manifest.od?.genui?.surfaces ?? []; - const autoSurfaces = input.connectorProbe - ? deriveAutoOAuthPrompts(connectorsResolved) - : []; - const genuiSurfaces = mergeAutoOAuthPrompts(declaredSurfaces, autoSurfaces); - const taskKind = (manifest.od?.taskKind ?? 'new-generation') as AppliedPluginSnapshot['taskKind']; // Spec §23.3.3: when the plugin omits `od.pipeline`, fall back to @@ -131,6 +126,22 @@ export function applyPlugin(input: ApplyInput): ApplyComputed { }); const appliedPipeline = pipelineResolution.pipeline; + const declaredSurfaces = manifest.od?.genui?.surfaces ?? []; + const autoOAuth = input.connectorProbe + ? deriveAutoOAuthPrompts(connectorsResolved) + : []; + // Spec §10.3.1 / §21.5: auto-derive surfaces for first-party atom + // stages (diff-review → choice surface). Plugin-author surfaces + // with the same id win; the merge helper handles the dedupe. + // We use the EFFECTIVE pipeline (appliedPipeline) so a plugin that + // inherits the bundled scenario's diff-review stage still gets + // the auto-surface. + const autoAtom = deriveAutoAtomSurfaces({ pipeline: appliedPipeline }); + const genuiSurfaces = mergeAutoOAuthPrompts( + mergeAutoOAuthPrompts(declaredSurfaces, autoOAuth), + autoAtom, + ); + const projectMetadata: PluginProjectMetadataPatch = { name: manifest.title ?? manifest.name, taskKind, diff --git a/apps/daemon/src/plugins/atoms/auto-surfaces.ts b/apps/daemon/src/plugins/atoms/auto-surfaces.ts new file mode 100644 index 000000000..b9dd350dd --- /dev/null +++ b/apps/daemon/src/plugins/atoms/auto-surfaces.ts @@ -0,0 +1,77 @@ +// Phase 8 entry slice / spec §10.3.1 / §21.5 — auto-derived GenUI surfaces +// for first-party atom stages. +// +// Mirrors the connector-gate's auto oauth-prompt pattern: when the +// pipeline contains a `diff-review` stage, the daemon synthesises a +// `choice` GenUI surface (`__auto_diff_review_`, +// persist='run') so the user can accept / reject / partial without +// the plugin author having to declare the surface by hand. +// Plugin-author-declared surfaces with the same id win — this helper +// returns the implicit list and `mergeAutoOAuthPrompts` (re-used) does +// the dedupe. +// +// Other atom stages that auto-derive surfaces in the future (e.g. +// `direction-picker` could auto-derive a `choice`) plug in here. + +import type { + GenUISurfaceSpec, + PluginPipeline, +} from '@open-design/contracts'; + +export interface AutoAtomSurfaceContext { + pipeline?: PluginPipeline | undefined; +} + +export function deriveAutoAtomSurfaces( + ctx: AutoAtomSurfaceContext, +): GenUISurfaceSpec[] { + const out: GenUISurfaceSpec[] = []; + if (!ctx.pipeline) return out; + for (const stage of ctx.pipeline.stages) { + const atoms = stage.atoms ?? []; + if (atoms.includes('diff-review')) { + out.push(buildDiffReviewSurface(stage.id)); + } + } + return out; +} + +function buildDiffReviewSurface(stageId: string): GenUISurfaceSpec { + return { + id: `__auto_diff_review_${stageId}`, + kind: 'choice', + persist: 'run', + trigger: { stageId, atom: 'diff-review' }, + prompt: 'Review the diff and choose how to proceed.', + schema: { + type: 'object', + title: 'Diff review', + description: 'Accept the patch, reject it, or pick which files to keep.', + properties: { + decision: { + type: 'string', + enum: ['accept', 'reject', 'partial'], + title: 'Decision', + }, + accepted_files: { + type: 'array', + items: { type: 'string' }, + title: 'Files to accept (only required when decision=partial)', + }, + rejected_files: { + type: 'array', + items: { type: 'string' }, + title: 'Files to reject (only required when decision=partial)', + }, + reason: { + type: 'string', + title: 'Notes for the patch author', + }, + }, + required: ['decision'], + }, + timeout: 24 * 60 * 60 * 1000, // 24h — diff review may sit overnight + onTimeout: 'abort', + capabilitiesRequired: [], + }; +} diff --git a/apps/daemon/tests/plugins-auto-surfaces.test.ts b/apps/daemon/tests/plugins-auto-surfaces.test.ts new file mode 100644 index 000000000..527712405 --- /dev/null +++ b/apps/daemon/tests/plugins-auto-surfaces.test.ts @@ -0,0 +1,138 @@ +// Phase 8 entry slice — auto-derived choice surface for diff-review. + +import { describe, expect, it } from 'vitest'; +import type { + InstalledPluginRecord, + PluginManifest, +} from '@open-design/contracts'; +import { applyPlugin } from '../src/plugins/apply.js'; +import { deriveAutoAtomSurfaces } from '../src/plugins/atoms/auto-surfaces.js'; + +const baseRegistry = (scenarios: any[] = []) => ({ + skills: [], + designSystems: [], + craft: [], + atoms: [], + scenarios, +}); + +const consumer = (od: NonNullable): InstalledPluginRecord => ({ + id: 'fixture', + title: 'Fixture', + version: '0.1.0', + sourceKind: 'local', + source: '/tmp/fixture', + fsPath: '/tmp/fixture', + trust: 'trusted', + capabilitiesGranted: ['prompt:inject'], + installedAt: Date.now(), + updatedAt: Date.now(), + manifest: { + $schema: 'https://open-design.ai/schemas/plugin.v1.json', + name: 'fixture', + title: 'Fixture', + version: '0.1.0', + od, + } as PluginManifest, +}); + +describe('deriveAutoAtomSurfaces — diff-review choice', () => { + it('emits one __auto_diff_review_ per stage that contains diff-review', () => { + const out = deriveAutoAtomSurfaces({ + pipeline: { + stages: [ + { id: 'review-final', atoms: ['diff-review'] }, + { id: 'review-touchups', atoms: ['critique-theater', 'diff-review'] }, + { id: 'plain', atoms: ['todo-write'] }, + ], + }, + }); + expect(out).toHaveLength(2); + expect(out.map((s) => s.id)).toEqual([ + '__auto_diff_review_review-final', + '__auto_diff_review_review-touchups', + ]); + for (const s of out) { + expect(s.kind).toBe('choice'); + expect(s.persist).toBe('run'); + expect((s.schema as { properties: { decision: { enum: string[] } } }).properties.decision.enum).toEqual(['accept', 'reject', 'partial']); + expect(s.trigger?.atom).toBe('diff-review'); + } + }); + + it('returns empty when no pipeline is supplied', () => { + expect(deriveAutoAtomSurfaces({})).toEqual([]); + }); + + it("returns empty when no stage carries 'diff-review'", () => { + expect(deriveAutoAtomSurfaces({ + pipeline: { stages: [{ id: 'plan', atoms: ['todo-write'] }] }, + })).toEqual([]); + }); +}); + +describe('applyPlugin — diff-review auto-surface integration', () => { + it('lands the auto-surface on AppliedPluginSnapshot.genuiSurfaces when the plugin pipeline declares diff-review', () => { + const out = applyPlugin({ + plugin: consumer({ + taskKind: 'code-migration', + pipeline: { + stages: [ + { id: 'review', atoms: ['diff-review'] }, + ], + }, + }), + inputs: {}, + registry: baseRegistry(), + }); + const ids = out.result.genuiSurfaces?.map((s) => s.id) ?? []; + expect(ids).toContain('__auto_diff_review_review'); + const surface = out.result.genuiSurfaces?.find((s) => s.id === '__auto_diff_review_review'); + expect(surface?.kind).toBe('choice'); + }); + + it('lands the auto-surface when the diff-review stage comes from the bundled scenario fallback', () => { + const out = applyPlugin({ + plugin: consumer({ taskKind: 'code-migration' }), // no pipeline + inputs: {}, + registry: baseRegistry([ + { + id: 'od-code-migration', + taskKind: 'code-migration', + pipeline: { + stages: [ + { id: 'verify', atoms: ['patch-edit', 'build-test'] }, + { id: 'review', atoms: ['diff-review'] }, + { id: 'handoff', atoms: ['handoff'] }, + ], + }, + }, + ]), + }); + const ids = out.result.genuiSurfaces?.map((s) => s.id) ?? []; + expect(ids).toContain('__auto_diff_review_review'); + }); + + it('a plugin-declared surface with the same id wins over the auto-derived one', () => { + const declaredSurface = { + id: '__auto_diff_review_review', + kind: 'choice' as const, + persist: 'project' as const, + prompt: 'CUSTOM PROMPT', + schema: { type: 'object', properties: { decision: { type: 'string', enum: ['accept', 'reject', 'partial'] } }, required: ['decision'] }, + }; + const out = applyPlugin({ + plugin: consumer({ + taskKind: 'code-migration', + pipeline: { stages: [{ id: 'review', atoms: ['diff-review'] }] }, + genui: { surfaces: [declaredSurface] }, + }), + inputs: {}, + registry: baseRegistry(), + }); + const matches = (out.result.genuiSurfaces ?? []).filter((s) => s.id === '__auto_diff_review_review'); + expect(matches).toHaveLength(1); + expect(matches[0]?.persist).toBe('project'); + expect(matches[0]?.prompt).toBe('CUSTOM PROMPT'); + }); +});