From 7863ffeb492ae73da8fc8e3ca4817d26404e9563 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Boschi?= Date: Tue, 7 Apr 2026 09:32:24 +0200 Subject: [PATCH] fix(paperclip): address review fixes for paperclip integration (#900) - Add CI job for paperclip integration tests with change detection - Add paperclip to valid release integrations - Validate hindsightApiUrl is set in loadConfig() - Log warnings on recall/retain failures instead of silently swallowing - Remove hardcoded timeout from reflect call - Fix tsconfig module resolution to Node16 - Update tests to pass required hindsightApiUrl --- .github/workflows/test.yml | 35 +++++++++++++++++++ .../paperclip/src/client.ts | 2 +- .../paperclip/src/config.ts | 8 ++++- .../paperclip/src/middleware.ts | 4 +-- .../paperclip/src/recall.ts | 4 +-- .../paperclip/src/retain.ts | 4 +-- .../paperclip/tests/bank.test.ts | 16 +++++---- .../paperclip/tsconfig.json | 4 +-- scripts/release-integration.sh | 2 +- 9 files changed, 61 insertions(+), 18 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ffd9bf0f..933a9c07 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -45,6 +45,7 @@ jobs: integrations-ag2: ${{ steps.filter.outputs.integrations-ag2 }} integrations-hermes: ${{ steps.filter.outputs.integrations-hermes }} integrations-llamaindex: ${{ steps.filter.outputs.integrations-llamaindex }} + integrations-paperclip: ${{ steps.filter.outputs.integrations-paperclip }} dev: ${{ steps.filter.outputs.dev }} ci: ${{ steps.filter.outputs.ci }} # Secrets are available for internal PRs, pull_request_review, and workflow_dispatch. @@ -117,6 +118,8 @@ jobs: - 'hindsight-integrations/hermes/**' integrations-llamaindex: - 'hindsight-integrations/llamaindex/**' + integrations-paperclip: + - 'hindsight-integrations/paperclip/**' dev: - 'hindsight-dev/**' ci: @@ -357,6 +360,37 @@ jobs: working-directory: ./hindsight-integrations/chat run: npm run build + test-paperclip-integration: + needs: [detect-changes] + if: >- + github.event_name != 'pull_request_review' && + (github.event_name == 'workflow_dispatch' || + needs.detect-changes.outputs.integrations-paperclip == 'true' || + needs.detect-changes.outputs.ci == 'true') + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v6 + with: + ref: ${{ github.event.pull_request.head.sha || '' }} + + - name: Set up Node.js + uses: actions/setup-node@v6 + with: + node-version: '22' + + - name: Install dependencies + working-directory: ./hindsight-integrations/paperclip + run: npm ci + + - name: Build + working-directory: ./hindsight-integrations/paperclip + run: npm run build + + - name: Run tests + working-directory: ./hindsight-integrations/paperclip + run: npm test + build-control-plane: needs: [detect-changes] if: >- @@ -2428,6 +2462,7 @@ jobs: - build-ai-sdk-integration - test-ai-sdk-integration-deno - build-chat-integration + - test-paperclip-integration - build-control-plane - build-docs - test-rust-cli diff --git a/hindsight-integrations/paperclip/src/client.ts b/hindsight-integrations/paperclip/src/client.ts index e11bbc04..e752b121 100644 --- a/hindsight-integrations/paperclip/src/client.ts +++ b/hindsight-integrations/paperclip/src/client.ts @@ -78,7 +78,7 @@ export class HindsightClient { query, budget: options?.budget ?? 'mid', max_tokens: options?.maxTokens ?? 1024, - }, 12_000); + }); } async retain( diff --git a/hindsight-integrations/paperclip/src/config.ts b/hindsight-integrations/paperclip/src/config.ts index bc01ff8c..df10fbdb 100644 --- a/hindsight-integrations/paperclip/src/config.ts +++ b/hindsight-integrations/paperclip/src/config.ts @@ -29,7 +29,7 @@ export interface PaperclipMemoryConfig { } export function loadConfig(overrides?: Partial): PaperclipMemoryConfig { - return { + const config: PaperclipMemoryConfig = { hindsightApiUrl: process.env['HINDSIGHT_API_URL'] ?? '', hindsightApiToken: process.env['HINDSIGHT_API_TOKEN'], bankGranularity: ['company', 'agent'], @@ -40,4 +40,10 @@ export function loadConfig(overrides?: Partial): Papercli timeoutMs: 15_000, ...overrides, }; + if (!config.hindsightApiUrl) { + throw new Error( + 'hindsightApiUrl is required — set HINDSIGHT_API_URL or pass hindsightApiUrl to loadConfig()', + ); + } + return config; } diff --git a/hindsight-integrations/paperclip/src/middleware.ts b/hindsight-integrations/paperclip/src/middleware.ts index 8efaf74e..6a7ef4d1 100644 --- a/hindsight-integrations/paperclip/src/middleware.ts +++ b/hindsight-integrations/paperclip/src/middleware.ts @@ -84,8 +84,8 @@ export function createMemoryMiddleware(config: PaperclipMemoryConfig) { retain( { companyId, agentId, content: output, documentId: runId }, config, - ).catch(() => { - // Graceful degradation + ).catch((err) => { + console.warn('[hindsight-paperclip] retain failed:', (err as Error).message); }); } } diff --git a/hindsight-integrations/paperclip/src/recall.ts b/hindsight-integrations/paperclip/src/recall.ts index b3e8fa67..f37f8304 100644 --- a/hindsight-integrations/paperclip/src/recall.ts +++ b/hindsight-integrations/paperclip/src/recall.ts @@ -56,8 +56,8 @@ export async function recall( maxTokens: config.recallMaxTokens, }); results = response.results; - } catch { - // Graceful degradation — memory is enhancement, not requirement + } catch (err) { + console.warn('[hindsight-paperclip] recall failed:', (err as Error).message); return ''; } diff --git a/hindsight-integrations/paperclip/src/retain.ts b/hindsight-integrations/paperclip/src/retain.ts index c5e2492c..52e78f14 100644 --- a/hindsight-integrations/paperclip/src/retain.ts +++ b/hindsight-integrations/paperclip/src/retain.ts @@ -52,7 +52,7 @@ export async function retain( context: config.retainContext, metadata: { companyId, agentId, ...metadata }, }); - } catch { - // Graceful degradation — memory is enhancement, not requirement + } catch (err) { + console.warn('[hindsight-paperclip] retain failed:', (err as Error).message); } } diff --git a/hindsight-integrations/paperclip/tests/bank.test.ts b/hindsight-integrations/paperclip/tests/bank.test.ts index 3cc52d3d..a33d5430 100644 --- a/hindsight-integrations/paperclip/tests/bank.test.ts +++ b/hindsight-integrations/paperclip/tests/bank.test.ts @@ -5,38 +5,40 @@ import { loadConfig } from '../src/config.js'; describe('deriveBankId', () => { const ctx = { companyId: 'co-123', agentId: 'ag-456' }; + const baseUrl = 'http://fake:9077'; + it('default: paperclip::companyId::agentId', () => { - const config = loadConfig(); + const config = loadConfig({ hindsightApiUrl: baseUrl }); expect(deriveBankId(ctx, config)).toBe('paperclip::co-123::ag-456'); }); it('company-only granularity', () => { - const config = loadConfig({ bankGranularity: ['company'] }); + const config = loadConfig({ hindsightApiUrl: baseUrl, bankGranularity: ['company'] }); expect(deriveBankId(ctx, config)).toBe('paperclip::co-123'); }); it('agent-only granularity', () => { - const config = loadConfig({ bankGranularity: ['agent'] }); + const config = loadConfig({ hindsightApiUrl: baseUrl, bankGranularity: ['agent'] }); expect(deriveBankId(ctx, config)).toBe('paperclip::ag-456'); }); it('custom prefix', () => { - const config = loadConfig({ bankIdPrefix: 'myapp' }); + const config = loadConfig({ hindsightApiUrl: baseUrl, bankIdPrefix: 'myapp' }); expect(deriveBankId(ctx, config)).toBe('myapp::co-123::ag-456'); }); it('empty prefix with default granularity', () => { - const config = loadConfig({ bankIdPrefix: '' }); + const config = loadConfig({ hindsightApiUrl: baseUrl, bankIdPrefix: '' }); expect(deriveBankId(ctx, config)).toBe('co-123::ag-456'); }); it('throws when bank ID would be empty', () => { - const config = loadConfig({ bankIdPrefix: '', bankGranularity: [] }); + const config = loadConfig({ hindsightApiUrl: baseUrl, bankIdPrefix: '', bankGranularity: [] }); expect(() => deriveBankId(ctx, config)).toThrow('Bank ID cannot be empty'); }); it('reversed granularity order', () => { - const config = loadConfig({ bankGranularity: ['agent', 'company'] }); + const config = loadConfig({ hindsightApiUrl: baseUrl, bankGranularity: ['agent', 'company'] }); expect(deriveBankId(ctx, config)).toBe('paperclip::ag-456::co-123'); }); }); diff --git a/hindsight-integrations/paperclip/tsconfig.json b/hindsight-integrations/paperclip/tsconfig.json index 66ea425a..bf74166b 100644 --- a/hindsight-integrations/paperclip/tsconfig.json +++ b/hindsight-integrations/paperclip/tsconfig.json @@ -1,9 +1,9 @@ { "compilerOptions": { "target": "ES2022", - "module": "ES2022", + "module": "Node16", "lib": ["ES2022"], - "moduleResolution": "node", + "moduleResolution": "node16", "declaration": true, "outDir": "./dist", "rootDir": "./src", diff --git a/scripts/release-integration.sh b/scripts/release-integration.sh index d5251558..fdec0e59 100755 --- a/scripts/release-integration.sh +++ b/scripts/release-integration.sh @@ -13,7 +13,7 @@ print_info() { echo -e "${GREEN}[INFO]${NC} $1"; } print_warn() { echo -e "${YELLOW}[WARN]${NC} $1"; } print_error() { echo -e "${RED}[ERROR]${NC} $1"; } -VALID_INTEGRATIONS=("litellm" "pydantic-ai" "crewai" "ag2" "ai-sdk" "chat" "openclaw" "langgraph" "llamaindex" "nemoclaw" "strands" "claude-code" "codex" "hermes" "autogen") +VALID_INTEGRATIONS=("litellm" "pydantic-ai" "crewai" "ag2" "ai-sdk" "chat" "openclaw" "langgraph" "llamaindex" "nemoclaw" "strands" "claude-code" "codex" "hermes" "autogen" "paperclip") usage() { print_error "Usage: $0 "