From 1110edc8955c2a7f3c593a4ff9ee6b51a04700ec Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Wed, 22 Jul 2026 06:31:38 -0700 Subject: [PATCH 1/5] carry prepare confirmation hash on result meta --- services/mcp/src/lib/build-tool-result.ts | 12 +++++ .../mcp/src/tools/confirmed-action-runtime.ts | 8 ++++ .../mcp/src/ui-apps/apps/loops-review.tsx | 14 ++++-- .../mcp/tests/unit/build-tool-result.test.ts | 45 +++++++++++++++++++ 4 files changed, 76 insertions(+), 3 deletions(-) diff --git a/services/mcp/src/lib/build-tool-result.ts b/services/mcp/src/lib/build-tool-result.ts index 026668198388..382260167a87 100644 --- a/services/mcp/src/lib/build-tool-result.ts +++ b/services/mcp/src/lib/build-tool-result.ts @@ -2,6 +2,7 @@ import { RESOURCE_URI_META_KEY } from '@modelcontextprotocol/ext-apps/server' import { estimateTokens } from '@/lib/estimate-tokens' import { formatResponse } from '@/lib/response' +import { isPrepareConfirmedActionResult } from '@/tools/confirmed-action-runtime' import { POSTHOG_FORMATTED_RESULTS_OVERRIDE_KEY, POSTHOG_META_KEY } from '@/tools/types' import { APP_DATA_META_KEY, type AnalyticsMetadata, type WithAnalytics } from '@/ui-apps/types' @@ -180,5 +181,16 @@ export function buildToolResultPayload(opts: BuildToolResultOptions): ToolResult payload._meta[APP_DATA_META_KEY] = structuredContent as Record } } + // `structuredContent` is only attached to UI-resource tools, so a UI app + // driving a confirmed action (e.g. the loops-review card's Create button) + // can't read the confirmation hash from the `-prepare` result — it only + // rides in the TOON text toward the model. Carry it on `_meta` too, the + // host/app-only channel apps already hydrate from. + if (isPrepareConfirmedActionResult(handlerResult)) { + payload._meta = { + ...payload._meta, + [APP_DATA_META_KEY]: rawResult as Record, + } + } return payload } diff --git a/services/mcp/src/tools/confirmed-action-runtime.ts b/services/mcp/src/tools/confirmed-action-runtime.ts index 6387d1628f49..5ed845fb0253 100644 --- a/services/mcp/src/tools/confirmed-action-runtime.ts +++ b/services/mcp/src/tools/confirmed-action-runtime.ts @@ -105,6 +105,14 @@ export interface PrepareConfirmedActionResult { next_steps: string } +export function isPrepareConfirmedActionResult(value: unknown): value is PrepareConfirmedActionResult { + if (value === null || typeof value !== 'object' || Array.isArray(value)) { + return false + } + const candidate = value as Partial + return typeof candidate.confirmation_hash === 'string' && candidate.confirmation_word === CONFIRMATION_WORD +} + /** * Run at the top of a `-prepare` tool's handler. Signs the args into a * hash and returns the payload the model relays to the user. diff --git a/services/mcp/src/ui-apps/apps/loops-review.tsx b/services/mcp/src/ui-apps/apps/loops-review.tsx index 8c6c2eabba06..096e749348b5 100644 --- a/services/mcp/src/ui-apps/apps/loops-review.tsx +++ b/services/mcp/src/ui-apps/apps/loops-review.tsx @@ -7,6 +7,7 @@ import { createRoot } from 'react-dom/client' import { LoopReviewView, type LoopReviewData, type LoopReviewState } from 'products/tasks/mcp/apps' import { AppWrapper } from '../components/AppWrapper' +import { APP_DATA_META_KEY } from '../types' function LoopReviewApp(): JSX.Element { return ( @@ -45,10 +46,17 @@ function LoopReviewContent({ data, app }: { data: LoopReviewData; app: App | nul setState({ loading: false, error: message, createdName: null }) return } - const confirmationHash = (prepared.structuredContent as { confirmation_hash?: string } | undefined) - ?.confirmation_hash + // The hash rides on `_meta` (app-only channel) — `structuredContent` is + // only attached to UI-resource tools, which `-prepare` tools are not. + const preparedData = ((prepared._meta as Record | undefined)?.[APP_DATA_META_KEY] ?? + prepared.structuredContent) as { confirmation_hash?: string } | undefined + const confirmationHash = preparedData?.confirmation_hash if (!confirmationHash) { - setState({ loading: false, error: 'Failed to create the loop.', createdName: null }) + setState({ + loading: false, + error: 'Failed to create the loop: the server did not return a confirmation hash.', + createdName: null, + }) return } const result = await app.callServerTool({ diff --git a/services/mcp/tests/unit/build-tool-result.test.ts b/services/mcp/tests/unit/build-tool-result.test.ts index 0575b17bcf0c..9d204c5032f3 100644 --- a/services/mcp/tests/unit/build-tool-result.test.ts +++ b/services/mcp/tests/unit/build-tool-result.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import { EXEC_BUILT_PAYLOAD, markExecPayload, buildToolResultPayload, isToolCallPayload } from '@/lib/build-tool-result' import { POSTHOG_FORMATTED_RESULTS_OVERRIDE_KEY, POSTHOG_META_KEY } from '@/tools/types' +import { APP_DATA_META_KEY } from '@/ui-apps/types' // Simulates a `query-trends` handler return value: a UI-resource tool that // carries both the raw `results` object and a pre-formatted pipe-delimited table @@ -270,3 +271,47 @@ describe('isToolCallPayload — nominal brand', () => { expect(isToolCallPayload(42)).toBe(false) }) }) + +describe('buildToolResultPayload — confirmed-action prepare results', () => { + const prepareResult = { + confirmation_hash: 'signed-token-abc', + confirmation_word: 'confirm', + action: 'create loop', + message: "About to create the loop 'Open PR Summary'. Reply 'confirm' to create it.", + next_steps: 'Surface the message above to the user.', + } + + it('carries the prepare payload on _meta so UI apps can read the hash', () => { + // `-prepare` tools have no UI resource, so structuredContent is never + // attached — without the _meta copy, a card driving the confirmed flow + // (loops-review Create button) has no machine-readable hash. + const payload = buildToolResultPayload({ + handlerResult: prepareResult, + toolMeta: undefined, + toolName: 'loops-create-prepare', + params: { name: 'Open PR Summary' }, + }) + + expect(payload._meta?.[APP_DATA_META_KEY]).toMatchObject({ + confirmation_hash: 'signed-token-abc', + confirmation_word: 'confirm', + }) + expect(payload).not.toHaveProperty('structuredContent') + expect(payload.content[0]!.text).toContain('signed-token-abc') + }) + + it.each([ + ['non-prepare object result', { results: [{ id: 1 }] }], + ['confirmation_hash without the confirm word', { confirmation_hash: 'abc', confirmation_word: 'yes' }], + ['non-string confirmation_hash', { confirmation_hash: 42, confirmation_word: 'confirm' }], + ])('does not attach _meta for %s', (_label, handlerResult) => { + const payload = buildToolResultPayload({ + handlerResult, + toolMeta: undefined, + toolName: 'whatever', + params: {}, + }) + + expect(payload).not.toHaveProperty('_meta') + }) +}) From 307ac99d1a257ae885271461662a9ed89294f82c Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Wed, 22 Jul 2026 09:19:19 -0700 Subject: [PATCH 2/5] send only reviewed fields from loop create card --- products/tasks/mcp/apps/LoopReviewView.tsx | 46 +++++++++++++++++-- services/mcp/src/lib/build-tool-result.ts | 7 +-- .../mcp/src/ui-apps/apps/loops-review.tsx | 29 +++++++++--- .../mcp/tests/unit/build-tool-result.test.ts | 3 -- 4 files changed, 66 insertions(+), 19 deletions(-) diff --git a/products/tasks/mcp/apps/LoopReviewView.tsx b/products/tasks/mcp/apps/LoopReviewView.tsx index f0c46c034da8..a27518065aec 100644 --- a/products/tasks/mcp/apps/LoopReviewView.tsx +++ b/products/tasks/mcp/apps/LoopReviewView.tsx @@ -37,6 +37,11 @@ export interface LoopReviewNotificationChannel { enabled?: boolean } +export interface LoopReviewConnectors { + mcp_installation_ids?: string[] + posthog_mcp_scopes?: 'read_only' | 'full' | string +} + /** The loop config the agent assembled — identical in shape to the `loops-create` tool * arguments, so the "Create loop" button can forward it unchanged. */ export interface LoopReviewData { @@ -49,7 +54,11 @@ export interface LoopReviewData { visibility?: string repositories?: LoopReviewRepository[] triggers?: LoopReviewTrigger[] + enabled?: boolean + overlap_policy?: 'skip' | 'allow' | 'cancel_previous' | string behaviors?: LoopReviewBehaviors + connectors?: LoopReviewConnectors + sandbox_environment?: string | null notifications?: Record context_target?: LoopReviewContextTarget | null _posthogUrl?: string @@ -91,11 +100,36 @@ function describeTrigger(trigger: LoopReviewTrigger): string { return 'API' } -function describeTriggers(triggers: LoopReviewTrigger[] | undefined): string { +const OVERLAP_LABELS: Record = { + skip: 'skips overlapping runs', + allow: 'allows overlapping runs', + cancel_previous: 'cancels the previous run', +} + +function describeTriggers(data: LoopReviewData): string { + const triggers = data.triggers + const parts: string[] = [] if (!triggers || triggers.length === 0) { - return 'Manual only' + parts.push('Manual only') + } else { + parts.push(triggers.map(describeTrigger).join(', ')) + } + if (data.enabled === false) { + parts.push('paused') } - return triggers.map(describeTrigger).join(', ') + if (data.overlap_policy) { + parts.push(OVERLAP_LABELS[data.overlap_policy] ?? data.overlap_policy) + } + return parts.join(' · ') +} + +function describePosthogAccess(connectors: LoopReviewConnectors | undefined): string { + return connectors?.posthog_mcp_scopes === 'full' ? 'Full (read-write)' : 'Read-only' +} + +function describeConnectors(connectors: LoopReviewConnectors | undefined): string { + const ids = connectors?.mcp_installation_ids ?? [] + return ids.length > 0 ? ids.join(', ') : 'None' } function describeRepository(repositories: LoopReviewRepository[] | undefined): string { @@ -165,17 +199,21 @@ export function LoopReviewView({ data, onCreate, state }: LoopReviewViewProps): const items: { label: string; value: ReactNode }[] = [ { label: 'Name', value: data.name?.trim() || 'Not set' }, + ...(data.description?.trim() ? [{ label: 'Description', value: data.description }] : []), { label: 'Visibility', value: data.visibility === 'team' ? 'Team' : 'Personal' }, { label: 'What it does', value: {data.instructions?.trim() || 'No prompt'}, }, - { label: 'Runs', value: describeTriggers(data.triggers) }, + { label: 'Runs', value: describeTriggers(data) }, { label: 'Context', value: describeContext(data.context_target) }, { label: 'Repository', value: describeRepository(data.repositories) }, { label: 'Model', value: describeModel(data) }, { label: 'Opens PRs', value: data.behaviors?.create_prs ? 'Yes' : 'No' }, { label: 'Auto-fix PRs', value: describeAutoFix(data.behaviors) }, + { label: 'PostHog access', value: describePosthogAccess(data.connectors) }, + { label: 'Connectors', value: describeConnectors(data.connectors) }, + { label: 'Sandbox', value: data.sandbox_environment || 'None' }, { label: 'Notifications', value: describeNotifications(data.notifications) }, ] diff --git a/services/mcp/src/lib/build-tool-result.ts b/services/mcp/src/lib/build-tool-result.ts index 382260167a87..870a6dba8204 100644 --- a/services/mcp/src/lib/build-tool-result.ts +++ b/services/mcp/src/lib/build-tool-result.ts @@ -181,11 +181,8 @@ export function buildToolResultPayload(opts: BuildToolResultOptions): ToolResult payload._meta[APP_DATA_META_KEY] = structuredContent as Record } } - // `structuredContent` is only attached to UI-resource tools, so a UI app - // driving a confirmed action (e.g. the loops-review card's Create button) - // can't read the confirmation hash from the `-prepare` result — it only - // rides in the TOON text toward the model. Carry it on `_meta` too, the - // host/app-only channel apps already hydrate from. + // `-prepare` tools have no UI resource, so UI apps driving a confirmed action + // read the hash from the app-only `_meta` channel instead of structuredContent. if (isPrepareConfirmedActionResult(handlerResult)) { payload._meta = { ...payload._meta, diff --git a/services/mcp/src/ui-apps/apps/loops-review.tsx b/services/mcp/src/ui-apps/apps/loops-review.tsx index 096e749348b5..173f21eb977e 100644 --- a/services/mcp/src/ui-apps/apps/loops-review.tsx +++ b/services/mcp/src/ui-apps/apps/loops-review.tsx @@ -31,13 +31,30 @@ function LoopReviewContent({ data, app }: { data: LoopReviewData; app: App | nul } setState({ loading: true, error: null, createdName: null }) try { - // `loops-create` is a confirmed action (prepare/execute), so an agent can't plant a - // persistent loop without an explicit human step. This button IS that step: prepare with - // the reviewed config unchanged (`loops-review`'s schema is the `loops-create` body), - // then execute with the returned hash — the click supplies the confirmation. + // `loops-create` is a confirmed action (prepare/execute) and this click is the human + // confirmation step, so only fields the card renders may travel: anything else in + // `data` would be created without ever being reviewed. + const reviewedConfig: Record = { + name: data.name, + description: data.description, + visibility: data.visibility, + instructions: data.instructions, + runtime_adapter: data.runtime_adapter, + model: data.model, + reasoning_effort: data.reasoning_effort, + repositories: data.repositories, + triggers: data.triggers, + enabled: data.enabled, + overlap_policy: data.overlap_policy, + behaviors: data.behaviors, + connectors: data.connectors, + sandbox_environment: data.sandbox_environment, + notifications: data.notifications, + context_target: data.context_target, + } const prepared = await app.callServerTool({ name: 'loops-create-prepare', - arguments: data as Record, + arguments: reviewedConfig, }) if (prepared.isError) { const message = @@ -46,8 +63,6 @@ function LoopReviewContent({ data, app }: { data: LoopReviewData; app: App | nul setState({ loading: false, error: message, createdName: null }) return } - // The hash rides on `_meta` (app-only channel) — `structuredContent` is - // only attached to UI-resource tools, which `-prepare` tools are not. const preparedData = ((prepared._meta as Record | undefined)?.[APP_DATA_META_KEY] ?? prepared.structuredContent) as { confirmation_hash?: string } | undefined const confirmationHash = preparedData?.confirmation_hash diff --git a/services/mcp/tests/unit/build-tool-result.test.ts b/services/mcp/tests/unit/build-tool-result.test.ts index 9d204c5032f3..0530c3d70379 100644 --- a/services/mcp/tests/unit/build-tool-result.test.ts +++ b/services/mcp/tests/unit/build-tool-result.test.ts @@ -282,9 +282,6 @@ describe('buildToolResultPayload — confirmed-action prepare results', () => { } it('carries the prepare payload on _meta so UI apps can read the hash', () => { - // `-prepare` tools have no UI resource, so structuredContent is never - // attached — without the _meta copy, a card driving the confirmed flow - // (loops-review Create button) has no machine-readable hash. const payload = buildToolResultPayload({ handlerResult: prepareResult, toolMeta: undefined, From e9041dda6d3dd364059d3384d4f7a0617a7117ed Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Wed, 22 Jul 2026 10:02:51 -0700 Subject: [PATCH 3/5] render each loop behavior flag on the review card --- .../tasks/mcp/apps/LoopReviewView.test.ts | 20 +++++++++++++++++++ products/tasks/mcp/apps/LoopReviewView.tsx | 12 ++++++++--- 2 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 products/tasks/mcp/apps/LoopReviewView.test.ts diff --git a/products/tasks/mcp/apps/LoopReviewView.test.ts b/products/tasks/mcp/apps/LoopReviewView.test.ts new file mode 100644 index 000000000000..7abb246b865c --- /dev/null +++ b/products/tasks/mcp/apps/LoopReviewView.test.ts @@ -0,0 +1,20 @@ +jest.mock('@posthog/mcp-ui', () => ({ DescriptionList: () => null }), { virtual: true }) +jest.mock('@posthog/quill', () => ({ Button: () => null, Card: () => null, CardContent: () => null }), { + virtual: true, +}) +jest.mock('lucide-react', () => ({ Check: () => null }), { virtual: true }) + +import { describeFixReviewComments, type LoopReviewBehaviors } from './LoopReviewView' + +describe('describeFixReviewComments', () => { + it.each<[string, LoopReviewBehaviors | undefined, string]>([ + ['fix_review_comments without watch_ci', { fix_review_comments: true, watch_ci: false }, 'Yes'], + ['fix_review_comments with watch_ci', { fix_review_comments: true, watch_ci: true }, 'Yes'], + ['watch_ci only', { fix_review_comments: false, watch_ci: true }, 'No'], + ['neither flag', { fix_review_comments: false, watch_ci: false }, 'No'], + ['missing behaviors', undefined, 'No'], + ['iteration cap', { fix_review_comments: true, max_fix_iterations: 5 }, 'Yes (up to 5 iterations)'], + ])('%s', (_label, behaviors, expected) => { + expect(describeFixReviewComments(behaviors)).toBe(expected) + }) +}) diff --git a/products/tasks/mcp/apps/LoopReviewView.tsx b/products/tasks/mcp/apps/LoopReviewView.tsx index a27518065aec..13ee0ad2200c 100644 --- a/products/tasks/mcp/apps/LoopReviewView.tsx +++ b/products/tasks/mcp/apps/LoopReviewView.tsx @@ -19,6 +19,7 @@ export interface LoopReviewBehaviors { create_prs?: boolean watch_ci?: boolean fix_review_comments?: boolean + max_fix_iterations?: number } export interface LoopReviewContextOutputs { @@ -171,8 +172,12 @@ function describeModel(data: LoopReviewData): string { return `${adapter} · ${model} · ${reasoning} reasoning` } -function describeAutoFix(behaviors: LoopReviewBehaviors | undefined): string { - return behaviors?.watch_ci && behaviors?.fix_review_comments ? 'On' : 'Off' +export function describeFixReviewComments(behaviors: LoopReviewBehaviors | undefined): string { + if (!behaviors?.fix_review_comments) { + return 'No' + } + const cap = behaviors.max_fix_iterations + return cap ? `Yes (up to ${cap} iterations)` : 'Yes' } export function LoopReviewView({ data, onCreate, state }: LoopReviewViewProps): ReactElement { @@ -210,7 +215,8 @@ export function LoopReviewView({ data, onCreate, state }: LoopReviewViewProps): { label: 'Repository', value: describeRepository(data.repositories) }, { label: 'Model', value: describeModel(data) }, { label: 'Opens PRs', value: data.behaviors?.create_prs ? 'Yes' : 'No' }, - { label: 'Auto-fix PRs', value: describeAutoFix(data.behaviors) }, + { label: 'Watches CI', value: data.behaviors?.watch_ci ? 'Yes' : 'No' }, + { label: 'Fixes review comments', value: describeFixReviewComments(data.behaviors) }, { label: 'PostHog access', value: describePosthogAccess(data.connectors) }, { label: 'Connectors', value: describeConnectors(data.connectors) }, { label: 'Sandbox', value: data.sandbox_environment || 'None' }, From 42526a41c921c56baa72d5ac8783d065e456b0b1 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Wed, 22 Jul 2026 10:23:57 -0700 Subject: [PATCH 4/5] show effective loop settings on review card --- .../tasks/mcp/apps/LoopReviewView.test.ts | 63 +++++++++++++++---- products/tasks/mcp/apps/LoopReviewView.tsx | 24 ++++--- products/tasks/mcp/apps/index.ts | 1 + 3 files changed, 64 insertions(+), 24 deletions(-) diff --git a/products/tasks/mcp/apps/LoopReviewView.test.ts b/products/tasks/mcp/apps/LoopReviewView.test.ts index 7abb246b865c..4e914e854e41 100644 --- a/products/tasks/mcp/apps/LoopReviewView.test.ts +++ b/products/tasks/mcp/apps/LoopReviewView.test.ts @@ -4,17 +4,58 @@ jest.mock('@posthog/quill', () => ({ Button: () => null, Card: () => null, CardC }) jest.mock('lucide-react', () => ({ Check: () => null }), { virtual: true }) -import { describeFixReviewComments, type LoopReviewBehaviors } from './LoopReviewView' +import { + describeFixReviewComments, + describePosthogAccess, + describeRunBehavior, + type LoopReviewBehaviors, + type LoopReviewConnectors, + type LoopReviewData, +} from './LoopReviewView' -describe('describeFixReviewComments', () => { - it.each<[string, LoopReviewBehaviors | undefined, string]>([ - ['fix_review_comments without watch_ci', { fix_review_comments: true, watch_ci: false }, 'Yes'], - ['fix_review_comments with watch_ci', { fix_review_comments: true, watch_ci: true }, 'Yes'], - ['watch_ci only', { fix_review_comments: false, watch_ci: true }, 'No'], - ['neither flag', { fix_review_comments: false, watch_ci: false }, 'No'], - ['missing behaviors', undefined, 'No'], - ['iteration cap', { fix_review_comments: true, max_fix_iterations: 5 }, 'Yes (up to 5 iterations)'], - ])('%s', (_label, behaviors, expected) => { - expect(describeFixReviewComments(behaviors)).toBe(expected) +describe('LoopReviewView', () => { + describe('describeFixReviewComments', () => { + it.each<[string, LoopReviewBehaviors | undefined, string]>([ + ['fix_review_comments without watch_ci', { fix_review_comments: true, watch_ci: false }, 'Yes'], + ['fix_review_comments with watch_ci', { fix_review_comments: true, watch_ci: true }, 'Yes'], + ['watch_ci only', { fix_review_comments: false, watch_ci: true }, 'No'], + ['neither flag', { fix_review_comments: false, watch_ci: false }, 'No'], + ['missing behaviors', undefined, 'No'], + ['iteration cap', { fix_review_comments: true, max_fix_iterations: 5 }, 'Yes (up to 5 iterations)'], + ['zero iteration cap', { fix_review_comments: true, max_fix_iterations: 0 }, 'Yes (up to 0 iterations)'], + ])('%s', (_label, behaviors, expected) => { + expect(describeFixReviewComments(behaviors)).toBe(expected) + }) + }) + + describe('describeRunBehavior', () => { + it.each<[string, Pick, string]>([ + ['no triggers shows the default overlap policy', {}, 'Manual only · skips overlapping runs'], + [ + 'cron schedule trigger', + { triggers: [{ type: 'schedule', config: { cron_expression: '0 9 * * 1' } }] }, + 'Schedule (0 9 * * 1) · skips overlapping runs', + ], + ['paused loop', { enabled: false }, 'Manual only · paused · skips overlapping runs'], + ['allow policy', { overlap_policy: 'allow' }, 'Manual only · allows overlapping runs'], + ['cancel_previous policy', { overlap_policy: 'cancel_previous' }, 'Manual only · cancels the previous run'], + [ + 'unknown policy falls back to the raw value', + { overlap_policy: 'defer' as unknown as LoopReviewData['overlap_policy'] }, + 'Manual only · defer', + ], + ])('%s', (_label, data, expected) => { + expect(describeRunBehavior(data)).toBe(expected) + }) + }) + + describe('describePosthogAccess', () => { + it.each<[string, LoopReviewConnectors | undefined, string]>([ + ['missing connectors', undefined, 'Read-only'], + ['read_only scope', { posthog_mcp_scopes: 'read_only' }, 'Read-only'], + ['full scope', { posthog_mcp_scopes: 'full' }, 'Full (read-write)'], + ])('%s', (_label, connectors, expected) => { + expect(describePosthogAccess(connectors)).toBe(expected) + }) }) }) diff --git a/products/tasks/mcp/apps/LoopReviewView.tsx b/products/tasks/mcp/apps/LoopReviewView.tsx index 13ee0ad2200c..a267175489e8 100644 --- a/products/tasks/mcp/apps/LoopReviewView.tsx +++ b/products/tasks/mcp/apps/LoopReviewView.tsx @@ -40,7 +40,7 @@ export interface LoopReviewNotificationChannel { export interface LoopReviewConnectors { mcp_installation_ids?: string[] - posthog_mcp_scopes?: 'read_only' | 'full' | string + posthog_mcp_scopes?: 'read_only' | 'full' } /** The loop config the agent assembled — identical in shape to the `loops-create` tool @@ -56,14 +56,13 @@ export interface LoopReviewData { repositories?: LoopReviewRepository[] triggers?: LoopReviewTrigger[] enabled?: boolean - overlap_policy?: 'skip' | 'allow' | 'cancel_previous' | string + overlap_policy?: 'skip' | 'allow' | 'cancel_previous' behaviors?: LoopReviewBehaviors connectors?: LoopReviewConnectors sandbox_environment?: string | null notifications?: Record context_target?: LoopReviewContextTarget | null _posthogUrl?: string - [key: string]: unknown } export interface LoopReviewState { @@ -107,24 +106,23 @@ const OVERLAP_LABELS: Record = { cancel_previous: 'cancels the previous run', } -function describeTriggers(data: LoopReviewData): string { - const triggers = data.triggers +export function describeRunBehavior(data: Pick): string { const parts: string[] = [] - if (!triggers || triggers.length === 0) { + if (!data.triggers || data.triggers.length === 0) { parts.push('Manual only') } else { - parts.push(triggers.map(describeTrigger).join(', ')) + parts.push(data.triggers.map(describeTrigger).join(', ')) } if (data.enabled === false) { parts.push('paused') } - if (data.overlap_policy) { - parts.push(OVERLAP_LABELS[data.overlap_policy] ?? data.overlap_policy) - } + // the server defaults overlap_policy to 'skip', so always show the effective policy + const overlapPolicy = data.overlap_policy ?? 'skip' + parts.push(OVERLAP_LABELS[overlapPolicy] ?? overlapPolicy) return parts.join(' · ') } -function describePosthogAccess(connectors: LoopReviewConnectors | undefined): string { +export function describePosthogAccess(connectors: LoopReviewConnectors | undefined): string { return connectors?.posthog_mcp_scopes === 'full' ? 'Full (read-write)' : 'Read-only' } @@ -177,7 +175,7 @@ export function describeFixReviewComments(behaviors: LoopReviewBehaviors | undef return 'No' } const cap = behaviors.max_fix_iterations - return cap ? `Yes (up to ${cap} iterations)` : 'Yes' + return cap != null ? `Yes (up to ${cap} iterations)` : 'Yes' } export function LoopReviewView({ data, onCreate, state }: LoopReviewViewProps): ReactElement { @@ -210,7 +208,7 @@ export function LoopReviewView({ data, onCreate, state }: LoopReviewViewProps): label: 'What it does', value: {data.instructions?.trim() || 'No prompt'}, }, - { label: 'Runs', value: describeTriggers(data) }, + { label: 'Runs', value: describeRunBehavior(data) }, { label: 'Context', value: describeContext(data.context_target) }, { label: 'Repository', value: describeRepository(data.repositories) }, { label: 'Model', value: describeModel(data) }, diff --git a/products/tasks/mcp/apps/index.ts b/products/tasks/mcp/apps/index.ts index 1d16d20b291e..c5a2656a67e6 100644 --- a/products/tasks/mcp/apps/index.ts +++ b/products/tasks/mcp/apps/index.ts @@ -1,6 +1,7 @@ export { LoopReviewView, type LoopReviewBehaviors, + type LoopReviewConnectors, type LoopReviewContextOutputs, type LoopReviewContextTarget, type LoopReviewData, From 3d2474eb3cb3416627db1fa4c51932ff574704a1 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Wed, 22 Jul 2026 10:25:03 -0700 Subject: [PATCH 5/5] extract loop review confirm helpers --- .../src/ui-apps/apps/loops-review-confirm.ts | 47 +++++++++++++++ .../mcp/src/ui-apps/apps/loops-review.tsx | 29 +-------- .../tests/unit/loops-review-confirm.test.ts | 59 +++++++++++++++++++ 3 files changed, 109 insertions(+), 26 deletions(-) create mode 100644 services/mcp/src/ui-apps/apps/loops-review-confirm.ts create mode 100644 services/mcp/tests/unit/loops-review-confirm.test.ts diff --git a/services/mcp/src/ui-apps/apps/loops-review-confirm.ts b/services/mcp/src/ui-apps/apps/loops-review-confirm.ts new file mode 100644 index 000000000000..efc2d82eb810 --- /dev/null +++ b/services/mcp/src/ui-apps/apps/loops-review-confirm.ts @@ -0,0 +1,47 @@ +import type { LoopReviewData } from 'products/tasks/mcp/apps' + +import { APP_DATA_META_KEY } from '../types' + +// `loops-create` is a confirmed action (prepare/execute) and the card's click is the human +// confirmation step, so only fields the card renders may travel: anything else in `data` +// would be created without ever being reviewed. Keying off `LoopReviewData` makes the +// compiler reject a card field that isn't listed here. +const REVIEWED_FIELDS: Record, true> = { + name: true, + description: true, + instructions: true, + runtime_adapter: true, + model: true, + reasoning_effort: true, + visibility: true, + repositories: true, + triggers: true, + enabled: true, + overlap_policy: true, + behaviors: true, + connectors: true, + sandbox_environment: true, + notifications: true, + context_target: true, +} + +export function buildReviewedConfig(data: LoopReviewData): Record { + const reviewed: Record = {} + for (const key of Object.keys(REVIEWED_FIELDS)) { + reviewed[key] = data[key as keyof LoopReviewData] + } + return reviewed +} + +// `-prepare` tools have no UI resource, so the server mirrors the result onto the app-only +// `_meta` channel; `structuredContent` stays as a fallback for hosts that forward it. +export function extractConfirmationHash(prepared: { + _meta?: Record + structuredContent?: unknown +}): string | undefined { + const preparedData = (prepared._meta?.[APP_DATA_META_KEY] ?? prepared.structuredContent) as + | { confirmation_hash?: unknown } + | undefined + const hash = preparedData?.confirmation_hash + return typeof hash === 'string' && hash.length > 0 ? hash : undefined +} diff --git a/services/mcp/src/ui-apps/apps/loops-review.tsx b/services/mcp/src/ui-apps/apps/loops-review.tsx index 173f21eb977e..f2fd4fb8a16e 100644 --- a/services/mcp/src/ui-apps/apps/loops-review.tsx +++ b/services/mcp/src/ui-apps/apps/loops-review.tsx @@ -7,7 +7,7 @@ import { createRoot } from 'react-dom/client' import { LoopReviewView, type LoopReviewData, type LoopReviewState } from 'products/tasks/mcp/apps' import { AppWrapper } from '../components/AppWrapper' -import { APP_DATA_META_KEY } from '../types' +import { buildReviewedConfig, extractConfirmationHash } from './loops-review-confirm' function LoopReviewApp(): JSX.Element { return ( @@ -31,30 +31,9 @@ function LoopReviewContent({ data, app }: { data: LoopReviewData; app: App | nul } setState({ loading: true, error: null, createdName: null }) try { - // `loops-create` is a confirmed action (prepare/execute) and this click is the human - // confirmation step, so only fields the card renders may travel: anything else in - // `data` would be created without ever being reviewed. - const reviewedConfig: Record = { - name: data.name, - description: data.description, - visibility: data.visibility, - instructions: data.instructions, - runtime_adapter: data.runtime_adapter, - model: data.model, - reasoning_effort: data.reasoning_effort, - repositories: data.repositories, - triggers: data.triggers, - enabled: data.enabled, - overlap_policy: data.overlap_policy, - behaviors: data.behaviors, - connectors: data.connectors, - sandbox_environment: data.sandbox_environment, - notifications: data.notifications, - context_target: data.context_target, - } const prepared = await app.callServerTool({ name: 'loops-create-prepare', - arguments: reviewedConfig, + arguments: buildReviewedConfig(data), }) if (prepared.isError) { const message = @@ -63,9 +42,7 @@ function LoopReviewContent({ data, app }: { data: LoopReviewData; app: App | nul setState({ loading: false, error: message, createdName: null }) return } - const preparedData = ((prepared._meta as Record | undefined)?.[APP_DATA_META_KEY] ?? - prepared.structuredContent) as { confirmation_hash?: string } | undefined - const confirmationHash = preparedData?.confirmation_hash + const confirmationHash = extractConfirmationHash(prepared) if (!confirmationHash) { setState({ loading: false, diff --git a/services/mcp/tests/unit/loops-review-confirm.test.ts b/services/mcp/tests/unit/loops-review-confirm.test.ts new file mode 100644 index 000000000000..ed60e520c76e --- /dev/null +++ b/services/mcp/tests/unit/loops-review-confirm.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from 'vitest' + +import { buildReviewedConfig, extractConfirmationHash } from '@/ui-apps/apps/loops-review-confirm' +import { APP_DATA_META_KEY } from '@/ui-apps/types' + +import type { LoopReviewData } from 'products/tasks/mcp/apps' + +describe('loops-review confirm helpers', () => { + describe('buildReviewedConfig', () => { + it('forwards reviewed fields and drops anything else', () => { + const data = { + name: 'Open PR summary', + instructions: 'Summarize open PRs', + visibility: 'team', + overlap_policy: 'allow', + _posthogUrl: 'https://us.posthog.com', + injected_field: 'must never travel', + } as unknown as LoopReviewData + + const config = buildReviewedConfig(data) + + expect(config).toMatchObject({ + name: 'Open PR summary', + instructions: 'Summarize open PRs', + visibility: 'team', + overlap_policy: 'allow', + }) + expect(config).not.toHaveProperty('_posthogUrl') + expect(config).not.toHaveProperty('injected_field') + }) + }) + + describe('extractConfirmationHash', () => { + it.each<[string, { _meta?: Record; structuredContent?: unknown }, string | undefined]>([ + [ + 'reads the hash from the _meta app channel', + { _meta: { [APP_DATA_META_KEY]: { confirmation_hash: 'hash-from-meta' } } }, + 'hash-from-meta', + ], + [ + 'prefers _meta over structuredContent', + { + _meta: { [APP_DATA_META_KEY]: { confirmation_hash: 'hash-from-meta' } }, + structuredContent: { confirmation_hash: 'hash-from-structured' }, + }, + 'hash-from-meta', + ], + [ + 'falls back to structuredContent', + { structuredContent: { confirmation_hash: 'hash-from-structured' } }, + 'hash-from-structured', + ], + ['returns undefined when neither channel has a hash', { _meta: {} }, undefined], + ['returns undefined for a non-string hash', { structuredContent: { confirmation_hash: 42 } }, undefined], + ])('%s', (_label, prepared, expected) => { + expect(extractConfirmationHash(prepared)).toBe(expected) + }) + }) +})