diff --git a/packages/core/src/__tests__/context.test.ts b/packages/core/src/__tests__/context.test.ts index 6b4f96e..908a270 100644 --- a/packages/core/src/__tests__/context.test.ts +++ b/packages/core/src/__tests__/context.test.ts @@ -790,6 +790,19 @@ describe('createAskableContext', () => { revenue.remove(); }); + it('invalid CSS selector in data-askable-parent does not throw and falls back to DOM ancestry', () => { + const el = makeEl({ widget: 'orphan' }, 'Orphan'); + el.setAttribute('data-askable-parent', ':invalid-selector('); + const ctx = createAskableContext(); + ctx.observe(document); + + expect(() => el.click()).not.toThrow(); + expect(ctx.getFocus()?.meta).toEqual({ widget: 'orphan' }); + + ctx.destroy(); + el.remove(); + }); + it('serializes DOM hierarchy in JSON output and respects scope filtering for ancestors', () => { const dashboard = makeEl({ view: 'dashboard' }, 'Dashboard'); dashboard.setAttribute('data-askable-scope', 'analytics'); diff --git a/packages/core/src/__tests__/observer.test.ts b/packages/core/src/__tests__/observer.test.ts index 16695f6..16d3664 100644 --- a/packages/core/src/__tests__/observer.test.ts +++ b/packages/core/src/__tests__/observer.test.ts @@ -660,4 +660,60 @@ describe('Observer', () => { obs.unobserve(); }); }); + + describe('data-askable-parent and parseMeta safety', () => { + it('invalid CSS selector in data-askable-parent does not throw', () => { + const el = attach(makeEl({ id: 'orphan' }, 'Orphan')); + el.setAttribute('data-askable-parent', ':invalid-selector('); + + const onFocus = vi.fn(); + const obs = new Observer(onFocus); + obs.observe(document); + + expect(() => el.click()).not.toThrow(); + expect(onFocus).toHaveBeenCalledOnce(); + expect(onFocus.mock.calls[0][0].meta).toEqual({ id: 'orphan' }); + expect(onFocus.mock.calls[0][0].ancestors).toBeUndefined(); + + obs.unobserve(); + }); + + it('non-object JSON in data-askable falls back to raw string', () => { + const el = document.createElement('div'); + el.setAttribute('data-askable', '42'); + el.textContent = 'Number meta'; + document.body.appendChild(el); + elements.push(el); + + const onFocus = vi.fn(); + const obs = new Observer(onFocus); + obs.observe(document); + + el.click(); + + expect(onFocus).toHaveBeenCalledOnce(); + expect(onFocus.mock.calls[0][0].meta).toBe('42'); + + obs.unobserve(); + }); + + it('array JSON in data-askable falls back to raw string', () => { + const el = document.createElement('div'); + el.setAttribute('data-askable', '[1,2,3]'); + el.textContent = 'Array meta'; + document.body.appendChild(el); + elements.push(el); + + const onFocus = vi.fn(); + const obs = new Observer(onFocus); + obs.observe(document); + + el.click(); + + expect(onFocus).toHaveBeenCalledOnce(); + expect(onFocus.mock.calls[0][0].meta).toBe('[1,2,3]'); + + obs.unobserve(); + }); + }); }); diff --git a/packages/core/src/context.ts b/packages/core/src/context.ts index 22dfdc3..5b05e5e 100644 --- a/packages/core/src/context.ts +++ b/packages/core/src/context.ts @@ -159,10 +159,14 @@ export class AskableContextImpl implements AskableContext { const queryRoot = typeof (rootNode as ParentNode).querySelector === 'function' ? rootNode as ParentNode : document; - const candidate = queryRoot.querySelector(selector); - return candidate instanceof HTMLElement && candidate !== el && candidate.hasAttribute('data-askable') - ? candidate - : null; + try { + const candidate = queryRoot.querySelector(selector); + return candidate instanceof HTMLElement && candidate !== el && candidate.hasAttribute('data-askable') + ? candidate + : null; + } catch { + return null; + } } private limitHierarchyDepth(elements: HTMLElement[], depth?: number): HTMLElement[] { diff --git a/packages/core/src/observer.ts b/packages/core/src/observer.ts index 2b486d1..65a27ee 100644 --- a/packages/core/src/observer.ts +++ b/packages/core/src/observer.ts @@ -53,7 +53,11 @@ function isBrowser(): boolean { function parseMeta(raw: string): Record | string { try { - return JSON.parse(raw) as Record; + const parsed: unknown = JSON.parse(raw); + if (parsed !== null && typeof parsed === 'object' && !Array.isArray(parsed)) { + return parsed as Record; + } + return raw; } catch { return raw; } @@ -70,10 +74,14 @@ function resolveExplicitParent(el: HTMLElement): HTMLElement | null { const queryRoot = typeof (rootNode as ParentNode).querySelector === 'function' ? rootNode as ParentNode : document; - const candidate = queryRoot.querySelector(selector); - return candidate instanceof HTMLElement && candidate !== el && candidate.hasAttribute('data-askable') - ? candidate - : null; + try { + const candidate = queryRoot.querySelector(selector); + return candidate instanceof HTMLElement && candidate !== el && candidate.hasAttribute('data-askable') + ? candidate + : null; + } catch { + return null; + } } type MetaCacheEntry = { diff --git a/packages/create-askable-app/src/scaffold.js b/packages/create-askable-app/src/scaffold.js index 4414083..a61cde3 100644 --- a/packages/create-askable-app/src/scaffold.js +++ b/packages/create-askable-app/src/scaffold.js @@ -63,6 +63,10 @@ export async function runCli(args) { const projectName = projectArg.trim(); const targetDir = path.resolve(process.cwd(), projectName); + if (!targetDir.startsWith(process.cwd() + path.sep) && targetDir !== process.cwd()) { + throw new Error(`Target directory must be inside the current working directory: ${targetDir}`); + } + if (!isDirectoryEmpty(targetDir)) { throw new Error(`Target directory is not empty: ${targetDir}`); } diff --git a/packages/create-askable-app/test/scaffold.test.js b/packages/create-askable-app/test/scaffold.test.js index 6b1a1b2..fb536a8 100644 --- a/packages/create-askable-app/test/scaffold.test.js +++ b/packages/create-askable-app/test/scaffold.test.js @@ -1,6 +1,6 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import { isDirectoryEmpty, toPackageName } from '../src/scaffold.js'; +import { isDirectoryEmpty, toPackageName, runCli } from '../src/scaffold.js'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; @@ -21,3 +21,10 @@ test('isDirectoryEmpty ignores .DS_Store', () => { fs.writeFileSync(path.join(target, '.DS_Store'), ''); assert.equal(isDirectoryEmpty(target), true); }); + +test('runCli rejects path traversal outside cwd', async () => { + await assert.rejects( + () => runCli(['../../malicious']), + /Target directory must be inside the current working directory/, + ); +}); diff --git a/packages/mcp/src/__tests__/index.test.ts b/packages/mcp/src/__tests__/index.test.ts index 2b26b0c..234e842 100644 --- a/packages/mcp/src/__tests__/index.test.ts +++ b/packages/mcp/src/__tests__/index.test.ts @@ -167,7 +167,7 @@ describe('createAskableMcpServer', () => { const handler = getToolHandler(provider, 'get_current_context'); const result = await handler({}); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('provider crashed'); + expect(result.content[0].text).toContain('Failed to get context'); }); it('returns isError when format_context_for_prompt provider throws', async () => { @@ -177,7 +177,7 @@ describe('createAskableMcpServer', () => { const handler = getToolHandler(provider, 'format_context_for_prompt'); const result = await handler({}); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('context unavailable'); + expect(result.content[0].text).toContain('Failed to format context'); }); it('returns isError when formatContextForPrompt throws after getContext succeeds', async () => { @@ -189,7 +189,7 @@ describe('createAskableMcpServer', () => { const handler = getToolHandler(provider, 'format_context_for_prompt'); const result = await handler({}); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('format failed'); + expect(result.content[0].text).toContain('Failed to format context'); }); it('passes recognized context options to the provider', async () => { diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index 0ed16a8..fcf5ccf 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -135,9 +135,10 @@ export function createAskableMcpServer(options: AskableMcpServerOptions): McpSer ], }; } catch (err) { + console.error('[askable-mcp] get_current_context failed:', err); return { isError: true, - content: [{ type: 'text', text: `Failed to get context: ${err instanceof Error ? err.message : String(err)}` }], + content: [{ type: 'text', text: 'Failed to get context. Check server logs for details.' }], }; } }, @@ -184,9 +185,10 @@ export function createAskableMcpServer(options: AskableMcpServerOptions): McpSer ], }; } catch (err) { + console.error('[askable-mcp] format_context_for_prompt failed:', err); return { isError: true, - content: [{ type: 'text', text: `Failed to format context: ${err instanceof Error ? err.message : String(err)}` }], + content: [{ type: 'text', text: 'Failed to format context. Check server logs for details.' }], }; } }, diff --git a/packages/react/src/__tests__/useAskableTextSelectionCapture.test.tsx b/packages/react/src/__tests__/useAskableTextSelectionCapture.test.tsx index c7e6966..b067206 100644 --- a/packages/react/src/__tests__/useAskableTextSelectionCapture.test.tsx +++ b/packages/react/src/__tests__/useAskableTextSelectionCapture.test.tsx @@ -1,4 +1,5 @@ import { render, screen, fireEvent, waitFor, act } from '@testing-library/react'; +import { vi } from 'vitest'; import { useAskableTextSelectionCapture } from '../useAskableTextSelectionCapture.js'; function selectText(text: string): HTMLElement { @@ -50,6 +51,29 @@ describe('useAskableTextSelectionCapture', () => { expect((captured[0] as { target?: { text?: string } }).target?.text).toBe('React selectionchange fires'); }); + it('invokes the latest onCapture after prop changes mid-session', async () => { + const first = vi.fn(); + const second = vi.fn(); + + function Consumer({ cb }: { cb: (p: unknown) => void }) { + const capture = useAskableTextSelectionCapture({ debounce: 0, onCapture: cb }); + return ( + + ); + } + + const { rerender } = render(); + act(() => { fireEvent.click(screen.getByText('Start')); }); + + rerender(); + + selectText('Changed callback'); + document.dispatchEvent(new Event('selectionchange')); + + await waitFor(() => expect(second).toHaveBeenCalledTimes(1)); + expect(first).not.toHaveBeenCalled(); + }); + it('captures the current browser selection', async () => { function Consumer() { const capture = useAskableTextSelectionCapture({ diff --git a/packages/react/src/useAskableTextSelectionCapture.ts b/packages/react/src/useAskableTextSelectionCapture.ts index 4d89fdf..b76f6ab 100644 --- a/packages/react/src/useAskableTextSelectionCapture.ts +++ b/packages/react/src/useAskableTextSelectionCapture.ts @@ -67,12 +67,12 @@ export function useAskableTextSelectionCapture( handleRef.current = null; setActive(false); } - currentOptions.onCapture?.(packet, selection); + optionsRef.current.onCapture?.(packet, selection); }, onCancel() { handleRef.current = null; setActive(false); - currentOptions.onCancel?.(); + optionsRef.current.onCancel?.(); }, });