diff --git a/.changeset/peek-connector-inslack-consent-text.md b/.changeset/peek-connector-inslack-consent-text.md new file mode 100644 index 00000000..edbbc289 --- /dev/null +++ b/.changeset/peek-connector-inslack-consent-text.md @@ -0,0 +1,11 @@ +--- +"@peekdev/mcp": patch +--- + +Make the per-action consent prompt human-readable. `buildElicitMessage` now +produces a masked, verb-specific sentence (e.g. *peek wants to Type "m•••m" into +`#email` on your live browser. Approve?*) instead of a generic +`run ""` string. Literal values (`type` text, `request_user_input` prompt) +are masked to the first and last character so no secret is rendered in the +connecting client's chat history. No MCP-contract change — the tool input schema +is unchanged; only the elicitation message text differs. diff --git a/packages/connector-core/src/runtime.test.ts b/packages/connector-core/src/runtime.test.ts index cd7928f4..74943ae6 100644 --- a/packages/connector-core/src/runtime.test.ts +++ b/packages/connector-core/src/runtime.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it, vi } from 'vitest'; import type { AgentOutcome, Brain, Session } from './brain.js'; import type { PeekMcp } from './mcp.js'; -import { ConnectorRuntime } from './runtime.js'; +import { ConnectorRuntime, classifyError } from './runtime.js'; import type { SecretStore } from './secret-store.js'; import { SessionStore } from './store.js'; import type { ConsentResponse, InboundMessage, SurfaceAdapter } from './surface.js'; @@ -430,6 +430,92 @@ describe('ConnectorRuntime turn serialization (concurrency clobber fix)', () => }); }); +describe('classifyError', () => { + // Real thrown-message grounding (verified against source before writing fixtures): + // - mcp connect: withTimeout label='mcp connect' → "mcp connect timed out after 10000ms" + // - mcp callTool: withTimeout label=`mcp callTool(${name})` → "mcp callTool(list_recent_sessions) timed out after 30000ms" + // - 401 auth: AuthenticationError.makeMessage → "401 {message from API}" (contains '401') + // - connection error: APIConnectionError → "Connection error." (contains 'connection error') + // - max-turns: SdkBrain → "SdkBrain exceeded 16 tool-use turns" (contains 'tool-use turns') + // NOTE: brief fixtures used 'Exceeded maxTurns (16) without a final answer' — WRONG. + // Real message does NOT contain 'maxturns', 'max turns', or 'max-turns'. Fixed here. + const cases: Array<[unknown, string]> = [ + [new Error('mcp connect timed out after 10000ms'), 'mcp-connection-lost'], + [new Error('mcp callTool(list_recent_sessions) timed out after 30000ms'), 'tool-error'], + [ + new Error('401 {"message":"invalid x-api-key","type":"authentication_error"}'), + 'llm-key-rejected', + ], + [new Error('Connection error.'), 'llm-endpoint-error'], + [new Error('No recording found for this browser session'), 'not-recording'], + [new Error('elicitInput deny reason: timeout'), 'consent-timeout'], + [new Error('SdkBrain exceeded 16 tool-use turns'), 'max-turns'], + ['a bare string with no signal', 'unknown'], + [{ weird: true }, 'unknown'], + ]; + for (const [err, kind] of cases) { + it(`classifies ${kind}`, () => { + const out = classifyError(err); + expect(out.kind).toBe(kind); + expect(out.headline.length).toBeGreaterThan(0); + expect(out.hint.length).toBeGreaterThan(0); + }); + } +}); + +describe('runLoop error legibility', () => { + it('calls postError with a classified kind when a turn throws', async () => { + const brain: Brain = { + newSession: (): Session => ({ history: [] }), + appendUserText: () => {}, + appendToolResult: () => {}, + runTurn: async () => { + throw new Error('401 {"message":"invalid x-api-key","type":"authentication_error"}'); + }, + }; + class ErrAdapter extends FakeAdapter { + errors: Array<[string, { kind: string; headline: string; hint: string }]> = []; + async postError(c: string, e: { kind: string; headline: string; hint: string }) { + this.errors.push([c, e]); + } + } + const adapter = new ErrAdapter(); + const store = new SessionStore(brain.newSession); + const mcp = { callTool: vi.fn(), onElicit: () => {} } as unknown as PeekMcp; + const runtime = new ConnectorRuntime({ adapter, brain, mcp, store }); + await runtime.start(); + adapter.msgHandler?.({ conversationId: 't1', userId: 'u', text: 'hi' }); + await vi.waitFor(() => expect(adapter.errors).toHaveLength(1)); + expect(adapter.errors[0]?.[1].kind).toBe('llm-key-rejected'); + expect(adapter.texts).toHaveLength(0); // used postError, not plain text + }); + + it('falls back to postText when the adapter has no postError', async () => { + const brain: Brain = { + newSession: (): Session => ({ history: [] }), + appendUserText: () => {}, + appendToolResult: () => {}, + runTurn: async () => { + throw new Error('boom'); + }, + }; + const adapter = new FakeAdapter(); // no postError + const store = new SessionStore(brain.newSession); + const mcp = { callTool: vi.fn(), onElicit: () => {} } as unknown as PeekMcp; + const runtime = new ConnectorRuntime({ adapter, brain, mcp, store }); + await runtime.start(); + adapter.msgHandler?.({ conversationId: 't1', userId: 'u', text: 'hi' }); + await vi.waitFor(() => expect(adapter.texts).toHaveLength(1)); + expect(adapter.texts[0]?.[0]).toBe('t1'); + // The composed text must contain the classified headline AND hint for the unknown kind. + // classifyError('boom') → kind:'unknown', headline:'Something went wrong reaching peek', + // hint:'Please try again. If it keeps happening, check the connector logs.' + const postedText = adapter.texts[0]?.[1] ?? ''; + expect(postedText).toContain('Something went wrong reaching peek'); + expect(postedText).toContain('Please try again'); + }); +}); + describe('ConnectorRuntime handler rejection', () => { it('does not produce an unhandled rejection when handleMessage rejects', async () => { // A brain whose runTurn always rejects diff --git a/packages/connector-core/src/runtime.ts b/packages/connector-core/src/runtime.ts index 7de43a7e..b3600712 100644 --- a/packages/connector-core/src/runtime.ts +++ b/packages/connector-core/src/runtime.ts @@ -4,7 +4,6 @@ import type { SecretStore } from './secret-store.js'; import type { SessionStore } from './store.js'; import type { ConsentResponse, InboundMessage, SurfaceAdapter } from './surface.js'; -const ERROR_TEXT = '⚠️ Something went wrong reaching peek. Please try again.'; const DENY_RESULT = 'The user denied this action. Do not retry it; explain or suggest an alternative.'; @@ -14,6 +13,90 @@ function mintCorrelationId(): string { return `pc-${Date.now()}-${correlationCounter}`; } +/** Defensively classify a caught turn error into a small, legible set. The default + * {kind:'unknown'} branch ensures a provider swap (whose error strings differ) + * can never break error handling — classification is provider-coupled, so it is + * best-effort and always falls through to a safe generic. Hints are SUGGESTIVE, + * not authoritative. + * + * Substring grounding (verified against mcp.ts + sdk-brain.ts + @anthropic-ai/sdk): + * - 'mcp connect' → withTimeout label 'mcp connect' → "mcp connect timed out after Nms" + * - 'mcp calltool' → withTimeout label `mcp callTool(${name})` → "mcp callTool(X) timed out after Nms" + * (checked BEFORE generic timeout branches so a callTool-timeout → tool-error, not consent-timeout) + * - '401' → AuthenticationError.makeMessage → "401 {error message from API}" + * - 'connection error' → APIConnectionError → "Connection error." (case-insensitive) + * - 'tool-use turns' → SdkBrain → "SdkBrain exceeded N tool-use turns" + * (brief used 'maxturns'/'max turns' which do NOT appear in the real message — fixed) + * - 'timeout' + ('elicit'|'consent') → peek-mcp elicitation deny/timeout text */ +export function classifyError(err: unknown): { kind: string; headline: string; hint: string } { + const msg = err instanceof Error ? err.message : typeof err === 'string' ? err : ''; + const m = msg.toLowerCase(); + if (m.includes('mcp connect')) { + return { + kind: 'mcp-connection-lost', + headline: 'Lost the connection to peek', + hint: 'The peek daemon may have stopped. Check that it is running, then try again.', + }; + } + if (m.includes('mcp calltool')) { + return { + kind: 'tool-error', + headline: 'A peek tool call failed', + hint: 'The action or query did not complete. Try rephrasing or ask again.', + }; + } + if ( + m.includes('401') || + m.includes('unauthorized') || + m.includes('x-api-key') || + m.includes('invalid api key') + ) { + return { + kind: 'llm-key-rejected', + headline: 'The AI provider rejected the API key', + hint: 'Check the model API key configured for the connector.', + }; + } + if ( + m.includes('econnrefused') || + m.includes('connection error') || + m.includes('fetch failed') || + m.includes('enotfound') + ) { + return { + kind: 'llm-endpoint-error', + headline: "Couldn't reach the AI provider", + hint: 'The model endpoint may be down or the base URL misconfigured. Try again shortly.', + }; + } + if (m.includes('no recording') || m.includes('not recording') || m.includes('no session')) { + return { + kind: 'not-recording', + headline: 'No recorded session to work with', + hint: 'Open the peek extension and record a browser session first.', + }; + } + if (m.includes('timeout') && (m.includes('elicit') || m.includes('consent'))) { + return { + kind: 'consent-timeout', + headline: 'The approval request timed out', + hint: 'No Approve/Deny was received in time. Send your request again.', + }; + } + if (m.includes('tool-use turns')) { + return { + kind: 'max-turns', + headline: 'The turn ran out of steps', + hint: 'peek reached its per-turn step limit. Narrow the request and try again.', + }; + } + return { + kind: 'unknown', + headline: 'Something went wrong reaching peek', + hint: 'Please try again. If it keeps happening, check the connector logs.', + }; +} + export interface RuntimeDeps { adapter: SurfaceAdapter; brain: Brain; @@ -161,7 +244,12 @@ export class ConnectorRuntime { } } catch (err) { console.error('connector loop error:', err); - await adapter.postText(conversationId, ERROR_TEXT); + const classified = classifyError(err); + if (adapter.postError) { + await adapter.postError(conversationId, classified); + } else { + await adapter.postText(conversationId, `${classified.headline}. ${classified.hint}`); + } } } diff --git a/packages/connector-core/src/surface.ts b/packages/connector-core/src/surface.ts index a868194e..294180b2 100644 --- a/packages/connector-core/src/surface.ts +++ b/packages/connector-core/src/surface.ts @@ -24,4 +24,10 @@ export interface SurfaceAdapter { postText(conversationId: string, text: string): Promise; postConsentRequest(conversationId: string, req: ConsentRequest): Promise; postConfirmation(conversationId: string, text: string): Promise; + /** Optional: post a classified, legible error. Runtime null-checks it, so an + * adapter that doesn't implement it degrades to postText. */ + postError?( + conversationId: string, + err: { kind: string; headline: string; hint: string }, + ): Promise; } diff --git a/packages/connector-slack/README.md b/packages/connector-slack/README.md new file mode 100644 index 00000000..dfc98cd0 --- /dev/null +++ b/packages/connector-slack/README.md @@ -0,0 +1,39 @@ +# @peekdev/connector-slack + +Slack surface adapter for the peek connector platform. Connects a peek agent +to Slack via Bolt's Socket Mode, routing Assistant thread messages and `/peek` +slash commands to the connector core. + +## Slack app setup + +### Required scopes + +| Scope | Why | +|---|---| +| `assistant:write` | Required to register the app as an AI assistant in Slack | +| `chat:write` | Required to post messages and set the "thinking…" status | + +### Slack app scope — `chat:write` + +The assistant "thinking…" status calls `assistant.threads.setStatus`. Slack is +migrating this capability from the `assistant:write` scope to `chat:write`. Add +**`chat:write`** to the bot token scopes in your Slack app manifest. Without it +the status is silently skipped (the turn still works); every other message uses +`chat.postMessage`, which also requires `chat:write`. + +## Usage + +```ts +import { SlackAdapter } from '@peekdev/connector-slack'; + +const adapter = new SlackAdapter({ + slackBotToken: process.env.SLACK_BOT_TOKEN, + slackAppToken: process.env.SLACK_APP_TOKEN, +}); + +adapter.onMessage(async (msg) => { + // Handle inbound messages from Slack +}); + +await adapter.start(); +``` diff --git a/packages/connector-slack/src/blockkit.test.ts b/packages/connector-slack/src/blockkit.test.ts index 8312daec..6b779898 100644 --- a/packages/connector-slack/src/blockkit.test.ts +++ b/packages/connector-slack/src/blockkit.test.ts @@ -1,5 +1,15 @@ import { describe, expect, it } from 'vitest'; -import { confirmation, consentCard, textBlocks } from './blockkit.js'; +import { + codeBlock, + confirmation, + consentCard, + errorBlock, + humanizeAction, + looksLikeCode, + maskValue, + resultBlocks, + textBlocks, +} from './blockkit.js'; describe('blockkit', () => { it('textBlocks wraps a mrkdwn section', () => { @@ -10,26 +20,177 @@ describe('blockkit', () => { { type: 'section', text: { type: 'mrkdwn', text: '✅ done' } }, ]); }); - it('consentCard encodes correlationId+conversationId in both button values', () => { - const { blocks } = consentCard('peek wants to act', { a: 1 }, 'c1', 't1'); +}); + +describe('maskValue (connector-slack copy)', () => { + it('matches the peek-mcp masking contract', () => { + expect(maskValue('mail@example.com')).toBe('m•••m'); + expect(maskValue('ab')).toBe('•••'); + expect(maskValue('')).toBe('•••'); + }); +}); + +describe('humanizeAction', () => { + it('click by selector', () => { + expect(humanizeAction({ type: 'click', selector: '#go' })).toContain('#go'); + expect(humanizeAction({ type: 'click', selector: '#go' })).toContain('Click'); + }); + it('type masks the text value', () => { + const s = humanizeAction({ type: 'type', selector: '#email', text: 'secret@x.io' }); + expect(s).toContain('s•••o'); + expect(s).not.toContain('secret@x.io'); + }); + it('navigate names the url', () => { + expect(humanizeAction({ type: 'navigate', url: 'https://x.test/a' })).toContain( + 'https://x.test/a', + ); + }); +}); + +describe('consentCard delegated path (summary only, empty details)', () => { + it('renders the summary in a clean card with header + context + buttons', () => { + const { blocks } = consentCard( + 'peek wants to Click `#go` on your live browser. Approve?', + {}, + 'c1', + 't1', + ); + const header = blocks.find((b) => b.type === 'header') as + | { text: { text: string } } + | undefined; + expect(header?.text.text).toContain('peek wants to act'); + const section = blocks.find((b) => b.type === 'section') as + | { text: { text: string } } + | undefined; + expect(section?.text.text).toContain('Click'); + const context = blocks.find((b) => b.type === 'context') as + | { elements: Array<{ text: string }> } + | undefined; + expect(context?.elements[0]?.text).toContain('c1'); + // Buttons still carry the encoded correlation payload. const actions = blocks.find((b) => b.type === 'actions') as { elements: Array<{ action_id: string; value: string }>; }; const approve = actions.elements.find((e) => e.action_id === 'peek_approve'); - const deny = actions.elements.find((e) => e.action_id === 'peek_deny'); - // biome-ignore lint/style/noNonNullAssertion: test asserts element exists; undefined would throw, not silently pass - expect(JSON.parse(approve!.value)).toEqual({ correlationId: 'c1', conversationId: 't1' }); - // biome-ignore lint/style/noNonNullAssertion: test asserts element exists; undefined would throw, not silently pass - expect(JSON.parse(deny!.value)).toEqual({ correlationId: 'c1', conversationId: 't1' }); + expect(JSON.parse(approve?.value ?? '{}')).toEqual({ + correlationId: 'c1', + conversationId: 't1', + }); + // No raw JSON code block on the delegated path. + expect(section?.text.text).not.toContain('```'); + }); +}); + +describe('errorBlock', () => { + it('renders a warning headline + context hint', () => { + const blocks = errorBlock('Lost the connection to peek', 'Is the peek daemon running?'); + const section = blocks.find((b) => b.type === 'section') as + | { text: { text: string } } + | undefined; + expect(section?.text.text).toContain(':warning:'); + expect(section?.text.text).toContain('Lost the connection to peek'); + const context = blocks.find((b) => b.type === 'context') as + | { elements: Array<{ text: string }> } + | undefined; + expect(context?.elements[0]?.text).toContain('Is the peek daemon running?'); + }); +}); + +describe('looksLikeCode', () => { + it('detects a fenced block', () => { + expect(looksLikeCode('here you go:\n```ts\nconst a = 1;\n```')).toBe(true); + }); + it('detects a Playwright test without a fence', () => { + expect( + looksLikeCode( + "import { test, expect } from '@playwright/test';\ntest('x', async ({ page }) => {})", + ), + ).toBe(true); + }); + it('treats plain forensic prose as non-code', () => { + expect(looksLikeCode('The button click failed because the network request 500ed.')).toBe(false); + }); +}); + +describe('codeBlock', () => { + it('wraps bare code in a single fence', () => { + const blocks = codeBlock("test('x', async ({ page }) => {})"); + const section = blocks[0] as { text: { text: string } }; + expect(section.text.text.startsWith('```')).toBe(true); + expect(section.text.text.endsWith('```')).toBe(true); + // not double-fenced + expect(section.text.text.split('```').length).toBe(3); + }); + it('does not double-fence already-fenced input', () => { + const blocks = codeBlock('```\nconst a = 1;\n```'); + const section = blocks[0] as { text: { text: string } }; + expect(section.text.text.split('```').length).toBe(3); + }); +}); + +describe('resultBlocks', () => { + it('routes code to a fenced block', () => { + const blocks = resultBlocks( + "import { test } from '@playwright/test';\ntest('x', async ({ page }) => {})", + ); + const section = blocks[0] as { text: { text: string } }; + expect(section.text.text).toContain('```'); + }); + it('routes prose to a plain mrkdwn section', () => { + const blocks = resultBlocks('Just some prose.'); + expect(blocks).toEqual([ + { type: 'section', text: { type: 'mrkdwn', text: 'Just some prose.' } }, + ]); + }); + it('passes mixed prose+fenced text through as mrkdwn without double-fencing', () => { + // Simulates an LLM reply like "Here's the repro:\n```ts\nconst a=1;\n```" + const mixed = "Here's the repro:\n```ts\nconst a = 1;\n```"; + const blocks = resultBlocks(mixed); + // Must route to textBlocks (a single mrkdwn section), not codeBlock + expect(blocks).toEqual([{ type: 'section', text: { type: 'mrkdwn', text: mixed } }]); + // Exactly one fence pair in the rendered text — not double-fenced + const rendered = (blocks[0] as { text: { text: string } }).text.text; + expect(rendered.split('```').length).toBe(3); + }); + it('routes bare Playwright code (no fence) through codeBlock — single fence wrap', () => { + const bare = + "import { test } from '@playwright/test';\ntest('x', async ({ page }) => { await page.goto('/'); })"; + const blocks = resultBlocks(bare); + const rendered = (blocks[0] as { text: { text: string } }).text.text; + // Wrapped in exactly one fence pair + expect(rendered.startsWith('```')).toBe(true); + expect(rendered.endsWith('```')).toBe(true); + expect(rendered.split('```').length).toBe(3); + }); +}); + +describe('consentCard suspend path (Action details → structured fields)', () => { + it('renders a humanized sentence + only present/non-default fields, masking text', () => { + const details = { type: 'type', selector: '#email', text: 'mail@example.com', delay: 40 }; + const { blocks } = consentCard('peek wants to act on your live browser', details, 'c9', 't9'); + const texts = blocks + .filter((b) => b.type === 'section') + .map((b) => (b as { text: { text: string } }).text.text) + .join('\n'); + expect(texts).toContain('Type'); + expect(texts).toContain('m•••m'); // masked + expect(texts).not.toContain('mail@example.com'); + expect(texts).toContain('#email'); // target field shown + expect(texts).not.toContain('delay'); // default value omitted + expect(texts).not.toContain('```'); // classified → no raw JSON + }); + it('falls back to a raw JSON code block for an unclassifiable details payload', () => { + const { blocks } = consentCard('peek wants to act', { not: 'an', action: true }, 'c1', 't1'); + const section = blocks.find((b) => b.type === 'section') as + | { text: { text: string } } + | undefined; + expect(section?.text.text).toContain('```'); }); - it('consentCard truncates the details payload when it exceeds the Slack section limit', () => { + it('truncates a huge raw fallback payload', () => { const { blocks } = consentCard('peek wants to act', { blob: 'x'.repeat(5000) }, 'c1', 't1'); const section = blocks.find((b) => b.type === 'section') as | { text: { text: string } } | undefined; - // biome-ignore lint/style/noNonNullAssertion: test asserts section exists - const text = section!.text.text; - expect(text.length).toBeLessThanOrEqual(3000); - expect(text).toContain('truncated'); + expect(section?.text.text).toContain('truncated'); }); }); diff --git a/packages/connector-slack/src/blockkit.ts b/packages/connector-slack/src/blockkit.ts index d2f01d4c..247cd108 100644 --- a/packages/connector-slack/src/blockkit.ts +++ b/packages/connector-slack/src/blockkit.ts @@ -1,7 +1,92 @@ import type { KnownBlock } from '@slack/types'; -export function textBlocks(text: string): KnownBlock[] { - return [{ type: 'section', text: { type: 'mrkdwn', text } }]; +/** Mask a value for a consent card: first + last char kept, middle → fixed + * 3-char bullet run; length ≤ 2 masks wholly. Byte-identical contract to + * peek-mcp's maskValue (the packages don't share a util). */ +export function maskValue(value: string): string { + if (value.length <= 2) return '•••'; + return `${value[0]}•••${value[value.length - 1]}`; +} + +const APPROVE_BUTTON_VALUE = (correlationId: string, conversationId: string): string => + JSON.stringify({ correlationId, conversationId }); + +function isActionDetails(details: unknown): details is Record & { type: string } { + return ( + typeof details === 'object' && + details !== null && + typeof (details as { type?: unknown }).type === 'string' + ); +} + +const asStr = (v: unknown): string | undefined => (typeof v === 'string' ? v : undefined); +const asNum = (v: unknown): number | undefined => (typeof v === 'number' ? v : undefined); +const targetOf = (d: Record): string => { + const base = asStr(d.ref) ?? asStr(d.selector) ?? '(active element)'; + const nth = asNum(d.nth); + return nth !== undefined ? `\`${base}\` #${nth}` : `\`${base}\``; +}; + +/** One-line human sentence for a suspend-path Action `details` payload. Masks + * any literal value that would persist in Slack history. */ +export function humanizeAction(action: Record): string { + const t = asStr(action.type); + switch (t) { + case 'click': + return `Click ${targetOf(action)}`; + case 'dblclick': + return `Double-click ${targetOf(action)}`; + case 'type': + return `Type "${maskValue(asStr(action.text) ?? '')}" into ${targetOf(action)}`; + case 'enter': + return `Press Enter on ${targetOf(action)}`; + case 'navigate': + return `Navigate to ${asStr(action.url) ?? '(url)'}`; + case 'back': + return 'Go back'; + case 'forward': + return 'Go forward'; + case 'reload': + return 'Reload the page'; + case 'scroll': + return action.ref !== undefined || action.selector !== undefined + ? `Scroll ${targetOf(action)} into view` + : 'Scroll the page'; + case 'screenshot': + return 'Take a screenshot'; + case 'waitFor': + return action.selector !== undefined ? `Wait for ${targetOf(action)}` : 'Wait'; + case 'highlight': + return `Highlight ${targetOf(action)}`; + case 'clear_highlight': + return 'Clear the highlight'; + case 'set_intent': + return 'Set the intent banner'; + case 'request_user_input': + return `Ask you: "${maskValue(asStr(action.prompt) ?? '')}"`; + default: + return `Run "${t ?? 'action'}"`; + } +} + +/** Structured fields for the suspend-path card — only keys present + non-default. + * Masks text/prompt values. */ +function actionFields(action: Record): string[] { + const fields: string[] = []; + const ref = asStr(action.ref); + const selector = asStr(action.selector); + const nth = asNum(action.nth); + if (ref !== undefined) fields.push(`*Target:* \`${ref}\``); + else if (selector !== undefined) fields.push(`*Target:* \`${selector}\``); + if (nth !== undefined) fields.push(`*Nth:* ${nth}`); + const url = asStr(action.url); + if (url !== undefined) fields.push(`*URL:* ${url}`); + const text = asStr(action.text); + if (text !== undefined) fields.push(`*Text:* "${maskValue(text)}"`); + const prompt = asStr(action.prompt); + if (prompt !== undefined) fields.push(`*Prompt:* "${maskValue(prompt)}"`); + if (action.observe === true) fields.push('*Observe:* yes'); + return fields; } export function consentCard( @@ -10,37 +95,114 @@ export function consentCard( correlationId: string, conversationId: string, ): { blocks: KnownBlock[] } { - const value = JSON.stringify({ correlationId, conversationId }); - const json = JSON.stringify(details, null, 2); - const MAX_DETAILS = 2800; - const shown = json.length > MAX_DETAILS ? `${json.slice(0, MAX_DETAILS)}\n… (truncated)` : json; - const body = `*${summary}*\n\`\`\`${shown}\`\`\``; - return { - blocks: [ - { type: 'section', text: { type: 'mrkdwn', text: body } }, + const value = APPROVE_BUTTON_VALUE(correlationId, conversationId); + const header: KnownBlock = { + type: 'header', + text: { type: 'plain_text', text: 'peek wants to act on your browser' }, + }; + const context: KnownBlock = { + type: 'context', + elements: [{ type: 'mrkdwn', text: `Action ${correlationId}` }], + }; + const buttons: KnownBlock = { + type: 'actions', + elements: [ { - type: 'actions', - elements: [ - { - type: 'button', - text: { type: 'plain_text', text: 'Approve' }, - style: 'primary', - action_id: 'peek_approve', - value, - }, - { - type: 'button', - text: { type: 'plain_text', text: 'Deny' }, - style: 'danger', - action_id: 'peek_deny', - value, - }, - ], + type: 'button', + text: { type: 'plain_text', text: 'Approve' }, + style: 'primary', + action_id: 'peek_approve', + value, + }, + { + type: 'button', + text: { type: 'plain_text', text: 'Deny' }, + style: 'danger', + action_id: 'peek_deny', + value, }, ], }; + + const bodyBlocks: KnownBlock[] = []; + if (isActionDetails(details)) { + // Suspend path: classified Action → humanized sentence + fields. + bodyBlocks.push({ + type: 'section', + text: { type: 'mrkdwn', text: `*${humanizeAction(details)}*` }, + }); + const fields = actionFields(details); + if (fields.length > 0) { + bodyBlocks.push({ type: 'section', text: { type: 'mrkdwn', text: fields.join('\n') } }); + } + } else if ( + details !== undefined && + details !== null && + Object.keys(details as object).length > 0 + ) { + // Unclassifiable non-empty details → raw JSON code-block fallback (truncated). + const json = JSON.stringify(details, null, 2); + const MAX_DETAILS = 2800; + const shown = json.length > MAX_DETAILS ? `${json.slice(0, MAX_DETAILS)}\n… (truncated)` : json; + bodyBlocks.push({ + type: 'section', + text: { type: 'mrkdwn', text: `*${summary}*\n\`\`\`${shown}\`\`\`` }, + }); + } else { + // Delegated path: details is {} — summary is already a masked human sentence. + bodyBlocks.push({ type: 'section', text: { type: 'mrkdwn', text: summary } }); + } + + return { blocks: [header, ...bodyBlocks, context, buttons] }; +} + +export function textBlocks(text: string): KnownBlock[] { + return [{ type: 'section', text: { type: 'mrkdwn', text } }]; +} + +const SECTION_LIMIT = 2900; // Slack section text hard limit is 3000; leave fence room. + +/** Heuristic: does this LLM narrative read as code the user would want fenced? + * True when it already carries a fenced block, or reads as a Playwright test. */ +export function looksLikeCode(text: string): boolean { + if (text.includes('```')) return true; + const hasPlaywright = text.includes('@playwright/test'); + const hasTestCall = /\btest\(|\bimport\s*\{\s*test\b/.test(text); + const hasPageApi = text.includes('page.'); + return hasPlaywright && (hasTestCall || hasPageApi); +} + +/** Wrap text in a single fenced mrkdwn block. Strips an existing outer fence so + * the output is never double-fenced; truncates to the Slack section limit. */ +export function codeBlock(text: string): KnownBlock[] { + let code = text.trim(); + if (code.startsWith('```') && code.endsWith('```')) { + code = code + .slice(3, -3) + .replace(/^[a-zA-Z]*\n/, '') + .trim(); // drop fence + optional lang tag + } + if (code.length > SECTION_LIMIT) code = `${code.slice(0, SECTION_LIMIT)}\n… (truncated)`; + return [{ type: 'section', text: { type: 'mrkdwn', text: `\`\`\`\n${code}\n\`\`\`` } }]; +} + +/** Route an LLM result to a code block or a prose section. + * Already-fenced text (mixed prose + code, or fully-fenced) is rendered as + * mrkdwn directly — Slack handles ``` natively and re-wrapping produces + * nested fences that close the outer fence early. Only bare Playwright code + * (no existing fence) is sent through codeBlock for a single fence wrap. */ +export function resultBlocks(text: string): KnownBlock[] { + if (text.includes('```')) return textBlocks(text); // already fenced → Slack mrkdwn renders it; never re-wrap + return looksLikeCode(text) ? codeBlock(text) : textBlocks(text); // bare Playwright code → fence once } export function confirmation(text: string): KnownBlock[] { return [{ type: 'section', text: { type: 'mrkdwn', text: `✅ ${text}` } }]; } + +export function errorBlock(headline: string, hint: string): KnownBlock[] { + return [ + { type: 'section', text: { type: 'mrkdwn', text: `:warning: *${headline}*` } }, + { type: 'context', elements: [{ type: 'mrkdwn', text: hint }] }, + ]; +} diff --git a/packages/connector-slack/src/index.ts b/packages/connector-slack/src/index.ts index a9e751e2..fbd90ae6 100644 --- a/packages/connector-slack/src/index.ts +++ b/packages/connector-slack/src/index.ts @@ -30,7 +30,16 @@ async function main(): Promise { const slackConfig = await buildSlackConfig(secretStore); const mcp = new PeekMcp(mcpConfig, 'peek-slack'); - await mcp.connect(); + try { + await mcp.connect(); + } catch (err) { + console.error( + '[peek-slack] Could not start the peek MCP server. ' + + 'Check that the peek daemon/CLI is installed and the spawn command is correct.', + ); + console.error(err); + process.exit(1); + } const tools = await mcp.listTools(); const anthropic = new Anthropic({ diff --git a/packages/connector-slack/src/slack-adapter.test.ts b/packages/connector-slack/src/slack-adapter.test.ts index 0b400004..76669fc9 100644 --- a/packages/connector-slack/src/slack-adapter.test.ts +++ b/packages/connector-slack/src/slack-adapter.test.ts @@ -1,5 +1,6 @@ -import { describe, expect, it } from 'vitest'; -import { parseConsentValue } from './slack-adapter.js'; +import { describe, expect, it, vi } from 'vitest'; +import { SlackAdapter } from './slack-adapter.js'; +import { parseConsentValue, suggestedPrompts } from './slack-adapter.js'; describe('parseConsentValue', () => { it('parses a well-formed correlation payload', () => { @@ -14,3 +15,89 @@ describe('parseConsentValue', () => { expect(parseConsentValue(JSON.stringify({ correlationId: 'c1' }))).toBeNull(); }); }); + +describe('suggestedPrompts', () => { + it('offers exactly four assistant prompts with title + message pairs', () => { + const { title, prompts } = suggestedPrompts(); + expect(title).toBe('Try asking peek:'); + expect(prompts).toHaveLength(4); + for (const p of prompts) { + expect(typeof p.title).toBe('string'); + expect(p.title.length).toBeGreaterThan(0); + expect(typeof p.message).toBe('string'); + expect(p.message.length).toBeGreaterThan(0); + } + expect(prompts.map((p) => p.title)).toEqual([ + 'What just failed?', + 'Show console errors', + 'What caused it?', + 'Make a Playwright repro', + ]); + }); +}); + +function makeAdapter(): { + adapter: SlackAdapter; + setStatus: ReturnType; + postMessage: ReturnType; +} { + const setStatus = vi.fn().mockResolvedValue({}); + const postMessage = vi.fn().mockResolvedValue({}); + const adapter = new SlackAdapter({ + slackBotToken: 'xoxb-test', + slackAppToken: 'xapp-test', + } as never); + // Test seam: swap the persisted Bolt client for a fake so post/status are observable. + (adapter as unknown as { app: { client: unknown } }).app.client = { + chat: { postMessage }, + assistant: { threads: { setStatus } }, + }; + return { adapter, setStatus, postMessage }; +} + +describe('SlackAdapter.postError', () => { + it('posts an errorBlock with the headline as the push fallback text', async () => { + const { adapter, postMessage } = makeAdapter(); + // Record a route so post() can resolve a channel. + ( + adapter as unknown as { routes: Map } + ).routes.set('t1', { channel: 'C1', threadTs: 'T1' }); + await adapter.postError('t1', { + kind: 'mcp-connection-lost', + headline: 'Lost peek', + hint: 'restart it', + }); + expect(postMessage).toHaveBeenCalledTimes(1); + const arg = postMessage.mock.calls[0]?.[0] as { text: string; blocks: unknown[] }; + expect(arg.text).toBe('Lost peek'); // meaningful mobile push + expect(Array.isArray(arg.blocks)).toBe(true); + }); +}); + +describe('SlackAdapter thinking status', () => { + it('sets a thread status at message receipt when a thread is present', async () => { + const { adapter, setStatus } = makeAdapter(); + ( + adapter as unknown as { + emit: (c: string, ch: string, t: string | undefined, u: string, x: string) => void; + } + ).emit('t1', 'C1', 'T1', 'u1', 'hello'); + await vi.waitFor(() => expect(setStatus).toHaveBeenCalledTimes(1)); + expect(setStatus).toHaveBeenCalledWith({ + channel_id: 'C1', + thread_ts: 'T1', + status: 'peek is thinking…', + }); + }); + + it('skips the status on the /peek slash path (no thread)', async () => { + const { adapter, setStatus } = makeAdapter(); + ( + adapter as unknown as { + emit: (c: string, ch: string, t: string | undefined, u: string, x: string) => void; + } + ).emit('cmd-C1-u1', 'C1', undefined, 'u1', 'hi'); + await new Promise((r) => setTimeout(r, 20)); + expect(setStatus).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/connector-slack/src/slack-adapter.ts b/packages/connector-slack/src/slack-adapter.ts index c8da8abe..4cb349ad 100644 --- a/packages/connector-slack/src/slack-adapter.ts +++ b/packages/connector-slack/src/slack-adapter.ts @@ -6,7 +6,7 @@ import type { } from '@peekdev/connector-core'; import { App, Assistant } from '@slack/bolt'; import type { BlockAction } from '@slack/bolt'; -import { confirmation, consentCard, textBlocks } from './blockkit.js'; +import { confirmation, consentCard, errorBlock, resultBlocks } from './blockkit.js'; import type { SlackConfig } from './config.js'; interface Route { @@ -14,6 +14,24 @@ interface Route { threadTs: string | undefined; } +export function suggestedPrompts(): { + title: string; + prompts: Array<{ title: string; message: string }>; +} { + return { + title: 'Try asking peek:', + prompts: [ + { title: 'What just failed?', message: 'What failed in my last browser session?' }, + { title: 'Show console errors', message: 'List the console errors from my last session' }, + { title: 'What caused it?', message: 'What did I do right before the last error?' }, + { + title: 'Make a Playwright repro', + message: 'Generate a Playwright test for my last session', + }, + ], + }; +} + export function parseConsentValue( raw: string | undefined, ): { correlationId: string; conversationId: string } | null { @@ -66,6 +84,22 @@ export class SlackAdapter implements SurfaceAdapter { return r; } + private async postStatus(conversationId: string, status: string): Promise { + const r = this.routes.get(conversationId); + // Status requires a thread; the /peek slash path has none — skip there. + if (!r || !r.threadTs) return; + try { + await this.app.client.assistant.threads.setStatus({ + channel_id: r.channel, + thread_ts: r.threadTs, + status, + }); + } catch { + // Status is a nicety; a failure (e.g. missing chat:write scope on an + // un-migrated app) must never break the turn. Swallow. + } + } + private async post(conversationId: string, blocks: unknown): Promise { const r = this.route(conversationId); await this.app.client.chat.postMessage({ @@ -78,13 +112,27 @@ export class SlackAdapter implements SurfaceAdapter { } async postText(conversationId: string, text: string): Promise { - await this.post(conversationId, textBlocks(text)); + await this.post(conversationId, resultBlocks(text)); } async postConfirmation(conversationId: string, text: string): Promise { await this.post(conversationId, confirmation(text)); } + async postError( + conversationId: string, + err: { kind: string; headline: string; hint: string }, + ): Promise { + const r = this.route(conversationId); + await this.app.client.chat.postMessage({ + channel: r.channel, + ...(r.threadTs ? { thread_ts: r.threadTs } : {}), + // biome-ignore lint/suspicious/noExplicitAny: Bolt's postMessage accepts KnownBlock[] but its typings require `any[]` here + blocks: errorBlock(err.headline, err.hint) as any, + text: err.headline, // meaningful mobile push + }); + } + async postConsentRequest(conversationId: string, req: ConsentRequest): Promise { const { blocks } = consentCard(req.summary, req.details, req.correlationId, conversationId); await this.post(conversationId, blocks); @@ -98,17 +146,23 @@ export class SlackAdapter implements SurfaceAdapter { text: string, ): void { this.routes.set(conversationId, { channel, threadTs }); + // Show a "thinking…" status immediately; Slack auto-clears it when the next + // message posts (postText/postConsentRequest). Fire-and-forget — never awaited, + // never allowed to block or throw into the message handler. + void this.postStatus(conversationId, 'peek is thinking…'); this.msgHandler?.({ conversationId, userId, text }); } private wire(): void { const assistant = new Assistant({ - threadStarted: async ({ say }) => { + threadStarted: async ({ say, setSuggestedPrompts }) => { await say( "Hi — I'm peek. Ask what failed in your last browser session, or tell me to act on the page you have open.", ); + const { title, prompts } = suggestedPrompts(); + await setSuggestedPrompts({ title, prompts }); }, - userMessage: async ({ message }) => { + userMessage: async ({ message, setTitle }) => { // Bolt types message as GenericMessageEvent but the actual payload has these fields const m = message as { thread_ts?: string; @@ -118,6 +172,12 @@ export class SlackAdapter implements SurfaceAdapter { channel?: string; }; if (!m.text || !m.channel) return; + // Thread title is a nicety; never block the message on it. + try { + await setTitle(m.text.slice(0, 75)); + } catch { + // Title failure must never drop the user's message. + } const cid = m.thread_ts ?? m.ts; this.emit(cid, m.channel, cid, m.user ?? 'unknown', m.text); }, diff --git a/packages/peek-mcp/src/mcp/elicitation.test.ts b/packages/peek-mcp/src/mcp/elicitation.test.ts index 4dad65c7..716030c9 100644 --- a/packages/peek-mcp/src/mcp/elicitation.test.ts +++ b/packages/peek-mcp/src/mcp/elicitation.test.ts @@ -1,5 +1,11 @@ import { describe, expect, it, vi } from 'vitest'; -import { type ElicitCapableServer, buildElicitMessage, elicitConsent } from './elicitation.js'; +import type { Action } from './action-schema.js'; +import { + type ElicitCapableServer, + buildElicitMessage, + elicitConsent, + maskValue, +} from './elicitation.js'; function server( caps: unknown, @@ -70,8 +76,67 @@ describe('elicitConsent', () => { }); }); +describe('maskValue', () => { + it('keeps first + last char, hides the middle with a fixed bullet run', () => { + expect(maskValue('mail@example.com')).toBe('m•••m'); + expect(maskValue('abcd')).toBe('a•••d'); + }); + it('never reveals a 1- or 2-char secret whole', () => { + expect(maskValue('')).toBe('•••'); + expect(maskValue('x')).toBe('•••'); + expect(maskValue('ab')).toBe('•••'); + }); +}); + describe('buildElicitMessage', () => { - it('names the action type', () => { - expect(buildElicitMessage({ type: 'click' })).toContain('click'); + const on = 'on your live browser'; + it('click by selector', () => { + const a: Action = { type: 'click', selector: '#submit', button: 'left' }; + const m = buildElicitMessage(a); + expect(m).toContain('Click'); + expect(m).toContain('#submit'); + expect(m).toContain(on); + }); + it('click by ref with nth', () => { + const a: Action = { type: 'click', ref: 'e12', nth: 2, button: 'left' }; + const m = buildElicitMessage(a); + expect(m).toContain('e12'); + expect(m).toContain('#2'); + }); + it('type masks the text value to first/last char', () => { + const a: Action = { type: 'type', selector: '#email', text: 'mail@example.com', delay: 40 }; + const m = buildElicitMessage(a); + expect(m).toContain('Type'); + expect(m).toContain('m•••m'); + expect(m).not.toContain('mail@example.com'); + expect(m).toContain('#email'); + }); + it('navigate names the url', () => { + const a: Action = { type: 'navigate', url: 'https://example.com/app' }; + expect(buildElicitMessage(a)).toContain('https://example.com/app'); + }); + it('request_user_input masks the prompt', () => { + const a: Action = { + type: 'request_user_input', + prompt: 'Enter your one-time code', + scope: 'field', + readBack: false, + timeoutMs: 120000, + }; + const m = buildElicitMessage(a); + expect(m).not.toContain('Enter your one-time code'); + expect(m).toContain('E•••e'); + }); + it('screenshot / reload / back / forward read cleanly', () => { + expect(buildElicitMessage({ type: 'screenshot' })).toContain('screenshot'); + expect(buildElicitMessage({ type: 'reload' })).toContain('Reload'); + expect(buildElicitMessage({ type: 'back' })).toContain('back'); + expect(buildElicitMessage({ type: 'forward' })).toContain('forward'); + }); + it('unknown verb falls back to the generic sentence', () => { + // Cast an unmodeled type to exercise the default branch defensively. + const m = buildElicitMessage({ type: 'page_view', maxElements: 200 } as Action); + expect(m).toContain('page_view'); + expect(m).toContain(on); }); }); diff --git a/packages/peek-mcp/src/mcp/elicitation.ts b/packages/peek-mcp/src/mcp/elicitation.ts index f531cb4a..4a4e8961 100644 --- a/packages/peek-mcp/src/mcp/elicitation.ts +++ b/packages/peek-mcp/src/mcp/elicitation.ts @@ -9,6 +9,8 @@ // destructive-override remain the backstop; elicitation is an ADDITIONAL delegated // prompt for the execute_action tool only. +import type { Action } from './action-schema.js'; + /** The subset of `McpServer.server` this module needs (structurally loose so the * SDK's richer types assign under exactOptionalPropertyTypes). */ export interface ElicitCapableServer { @@ -87,8 +89,68 @@ export async function elicitConsent( : { elicited: true, verdict: 'deny', reason: 'declined' }; } -/** Human-facing card text for an action. peek-mcp does not classify the action — - * it just names it. */ -export function buildElicitMessage(action: { type: string }): string { - return `peek wants to run "${action.type}" on your live browser. Approve?`; +/** Mask a sensitive value for a consent card: keep the first and last visible + * character, replace the middle with a fixed 3-char bullet run (never + * length-proportional — length must not leak). Values of length ≤ 2 mask + * wholly, so a 1- or 2-char secret is never shown. */ +export function maskValue(value: string): string { + if (value.length <= 2) return '•••'; + const first = value[0]; + const last = value[value.length - 1]; + // Both indices are defined: length > 2 guarantees [0] and [length-1] exist. + return `${first ?? ''}•••${last ?? ''}`; +} + +/** Human-facing consent-card text for an action. peek-mcp does NOT classify the + * action as destructive/act (that lives in the SW gate) — it only describes it, + * masking any literal value that would otherwise persist in the client's chat + * history. Widening the parameter to the Action union is peek-mcp-internal: the + * caller (server.ts dispatchActTool) already passes the full typed input.action, + * so the MCP contract is unchanged. */ +export function buildElicitMessage(action: Action): string { + const tail = 'on your live browser. Approve?'; + const target = (ref?: string, selector?: string, nth?: number): string => { + const base = ref ?? selector ?? '(active element)'; + return nth !== undefined ? `\`${base}\` #${nth}` : `\`${base}\``; + }; + switch (action.type) { + case 'click': + return `peek wants to Click ${target(action.ref, action.selector, action.nth)} ${tail}`; + case 'dblclick': + return `peek wants to Double-click ${target(action.ref, action.selector, action.nth)} ${tail}`; + case 'type': + return `peek wants to Type "${maskValue(action.text)}" into ${target(action.ref, action.selector)} ${tail}`; + case 'enter': + return `peek wants to press Enter on ${target(action.ref, action.selector)} ${tail}`; + case 'navigate': + return `peek wants to Navigate to ${action.url} ${tail}`; + case 'back': + return `peek wants to go back ${tail}`; + case 'forward': + return `peek wants to go forward ${tail}`; + case 'reload': + return `peek wants to Reload the page ${tail}`; + case 'scroll': + return action.ref !== undefined || action.selector !== undefined + ? `peek wants to Scroll ${target(action.ref, action.selector)} into view ${tail}` + : `peek wants to Scroll the page ${tail}`; + case 'screenshot': + return `peek wants to take a screenshot ${tail}`; + case 'waitFor': + return action.selector !== undefined + ? `peek wants to wait for ${target(undefined, action.selector)} ${tail}` + : `peek wants to wait ${tail}`; + case 'highlight': + return `peek wants to Highlight ${target(undefined, action.selector)} ${tail}`; + case 'clear_highlight': + return `peek wants to clear the highlight ${tail}`; + case 'set_intent': + return `peek wants to set its intent banner ${tail}`; + case 'request_user_input': + return `peek wants to ask you: "${maskValue(action.prompt)}" ${tail}`; + default: + // Unmodeled/read verbs (page_view, element_detail) or a future type — + // name it generically. `action` narrows to the remaining union members. + return `peek wants to run "${action.type}" ${tail}`; + } }