Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions products/tasks/mcp/apps/LoopReviewView.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
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,
describePosthogAccess,
describeRunBehavior,
type LoopReviewBehaviors,
type LoopReviewConnectors,
type LoopReviewData,
} from './LoopReviewView'

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<LoopReviewData, 'triggers' | 'enabled' | 'overlap_policy'>, 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)
})
})
})
60 changes: 51 additions & 9 deletions products/tasks/mcp/apps/LoopReviewView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export interface LoopReviewBehaviors {
create_prs?: boolean
watch_ci?: boolean
fix_review_comments?: boolean
max_fix_iterations?: number
}

export interface LoopReviewContextOutputs {
Expand All @@ -37,6 +38,11 @@ export interface LoopReviewNotificationChannel {
enabled?: boolean
}

export interface LoopReviewConnectors {
mcp_installation_ids?: string[]
posthog_mcp_scopes?: 'read_only' | 'full'
}

/** 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 {
Expand All @@ -49,11 +55,14 @@ export interface LoopReviewData {
visibility?: string
repositories?: LoopReviewRepository[]
triggers?: LoopReviewTrigger[]
enabled?: boolean
overlap_policy?: 'skip' | 'allow' | 'cancel_previous'
behaviors?: LoopReviewBehaviors
connectors?: LoopReviewConnectors
sandbox_environment?: string | null
notifications?: Record<string, LoopReviewNotificationChannel>
context_target?: LoopReviewContextTarget | null
_posthogUrl?: string
[key: string]: unknown
}

export interface LoopReviewState {
Expand Down Expand Up @@ -91,11 +100,35 @@ function describeTrigger(trigger: LoopReviewTrigger): string {
return 'API'
}

function describeTriggers(triggers: LoopReviewTrigger[] | undefined): string {
if (!triggers || triggers.length === 0) {
return 'Manual only'
const OVERLAP_LABELS: Record<string, string> = {
skip: 'skips overlapping runs',
allow: 'allows overlapping runs',
cancel_previous: 'cancels the previous run',
}

export function describeRunBehavior(data: Pick<LoopReviewData, 'triggers' | 'enabled' | 'overlap_policy'>): string {
const parts: string[] = []
if (!data.triggers || data.triggers.length === 0) {
parts.push('Manual only')
} else {
parts.push(data.triggers.map(describeTrigger).join(', '))
}
if (data.enabled === false) {
parts.push('paused')
}
return triggers.map(describeTrigger).join(', ')
// 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(' · ')
}

export 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 {
Expand Down Expand Up @@ -137,8 +170,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 != null ? `Yes (up to ${cap} iterations)` : 'Yes'
}

export function LoopReviewView({ data, onCreate, state }: LoopReviewViewProps): ReactElement {
Expand All @@ -165,17 +202,22 @@ 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: <span className="whitespace-pre-wrap">{data.instructions?.trim() || 'No prompt'}</span>,
},
{ label: 'Runs', value: describeTriggers(data.triggers) },
{ label: 'Runs', value: describeRunBehavior(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: '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' },
{ label: 'Notifications', value: describeNotifications(data.notifications) },
]

Expand Down
1 change: 1 addition & 0 deletions products/tasks/mcp/apps/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export {
LoopReviewView,
type LoopReviewBehaviors,
type LoopReviewConnectors,
type LoopReviewContextOutputs,
type LoopReviewContextTarget,
type LoopReviewData,
Expand Down
9 changes: 9 additions & 0 deletions services/mcp/src/lib/build-tool-result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -180,5 +181,13 @@ export function buildToolResultPayload(opts: BuildToolResultOptions): ToolResult
payload._meta[APP_DATA_META_KEY] = structuredContent as Record<string, unknown>
}
}
// `-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,
[APP_DATA_META_KEY]: rawResult as Record<string, unknown>,
}
}
return payload
}
8 changes: 8 additions & 0 deletions services/mcp/src/tools/confirmed-action-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<PrepareConfirmedActionResult>
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.
Expand Down
47 changes: 47 additions & 0 deletions services/mcp/src/ui-apps/apps/loops-review-confirm.ts
Original file line number Diff line number Diff line change
@@ -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<Exclude<keyof LoopReviewData, '_posthogUrl'>, 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<string, unknown> {
const reviewed: Record<string, unknown> = {}
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<string, unknown>
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
}
16 changes: 8 additions & 8 deletions services/mcp/src/ui-apps/apps/loops-review.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 { buildReviewedConfig, extractConfirmationHash } from './loops-review-confirm'

function LoopReviewApp(): JSX.Element {
return (
Expand All @@ -30,13 +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), 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.
const prepared = await app.callServerTool({
name: 'loops-create-prepare',
arguments: data as Record<string, unknown>,
arguments: buildReviewedConfig(data),
})
if (prepared.isError) {
const message =
Expand All @@ -45,10 +42,13 @@ 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
const confirmationHash = extractConfirmationHash(prepared)
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({
Expand Down
42 changes: 42 additions & 0 deletions services/mcp/tests/unit/build-tool-result.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -270,3 +271,44 @@ 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', () => {
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')
})
})
Loading
Loading