Skip to content

Conversation

@tyler5673
Copy link

@tyler5673 tyler5673 commented Jan 28, 2026

This is an n8n node that supports v1/search, contents, and express agent usage.
This PR has been well tested in a local n8n environment, and has integration and unit tests included

n8n_integration.mov

@claude

This comment was marked as outdated.

@claude

This comment was marked as outdated.

@tyler5673 tyler5673 changed the title feat: n8n node with v1/search feat: n8n node with v1/search, contents, and express agent Jan 28, 2026
@claude

This comment was marked as outdated.

@claude

This comment was marked as outdated.

@tyler5673 tyler5673 marked this pull request as ready for review January 28, 2026 21:54
@tyler5673 tyler5673 requested a review from a team as a code owner January 28, 2026 21:54
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These don't actually test any integration. Also we have plenty test against the api as MCP ships default fetch handlers for these endpoint I already reuse in the ai-sdk-plugin. Delete this unless you're specially testing a fetch function of your own.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ty! Removed. This appears to be overkill anyway when compared to competitor nodes https://github.com/tavily-ai/tavily-n8n-node/tree/main

@@ -0,0 +1,84 @@
{
"name": "n8n-nodes-youdotcom",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename this we have a NPM org @youdotcom-oss see other package.json files for pattern unless n8n wants this a specific way. It should be something like @youdotcom-oss/n8n-nodes

You should also rename the directory to we don't do youdotcom suffixes in this project. please follow the pattern of the other packages.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EdwardIrby it unfortunately wouldn't pass n8ns validation. It looks like they require package names to follow the convention "n8n-nodes-[PACKAGE-NAME]" or "@[AUTHOR]/n8n-nodes-[PACKAGE-NAME]". I'm curious what your preferences would be here. I'm happy to create a separate repo for the n8n node if the convention isn't flexible.

https://github.com/n8n-io/n8n/blob/master/packages/%40n8n/eslint-plugin-community-nodes/src/rules/package-name-convention.ts

EdwardIrby and others added 2 commits January 29, 2026 11:13
- Refactor YouDotCom node into proper class with static private methods
- Add Zod schemas for runtime validation of inputs and API responses
- Remove 7 unnecessary type casts using TypeScript inference
- Enhance error handling with detailed Zod validation messages
- Remove redundant integration tests (duplicates MCP package tests)
- Colocate test file with implementation (node.spec.ts -> YouDotCom.node.spec.ts)
- Add zod@4.3.6 dependency

All 43 unit tests passing, all quality checks passing.
@claude

This comment was marked as outdated.

@tyler5673 tyler5673 marked this pull request as draft January 29, 2026 19:17
@claude

This comment was marked as outdated.

- Add .describe() to all Zod schema fields for better error messages
- Strengthen schemas with .enum() for freshness, safesearch, livecrawl
- Add domain validation for site field with .refine()
- Improve Express response schema with stricter validation (non-empty strings, URL validation)
- Add SearchResponseSchema and ContentsResponseSchema for API response validation
- Improve error message formatting (multiline with numbered list)
- Update README.md to remove references to non-existent test scripts

All changes address code review suggestions from PR #28:
- Stricter input validation catches invalid values before API calls
- Response validation detects API contract changes early
- Better error messages improve debugging experience
- Accurate documentation matches available commands

Tests: ✅ All 43 tests passing
Quality: ✅ Biome, TypeScript, package format checks pass
@tyler5673 tyler5673 requested a review from EdwardIrby January 29, 2026 22:44
@claude
Copy link
Contributor

claude bot commented Jan 29, 2026

Claude finished @tyler5673's task in 2m 26s —— View job


Code Review Complete ✅


Summary

This PR demonstrates excellent code quality with thorough testing, proper security practices, and clean implementation. All previous feedback has been addressed. The code is production-ready with just two minor documentation fixes needed before merge.


🎯 What's Excellent

Code Quality ✅

  • All tests pass: 43/43 unit tests with 120 assertions
  • Quality checks pass: Biome, TypeScript, and package format all clean
  • Comprehensive test coverage: Node config, all operations, parameters, credentials
  • Clean implementation: Well-organized, readable, follows TypeScript best practices

Security ✅

  • API key protection: typeOptions: { password: true } masks keys in UI
  • Proper authentication: Different methods for different APIs (X-API-Key for Search/Contents, Bearer for Express)
  • Credential validation: Test request validates API keys safely
  • Input sanitization: URL parsing with proper validation
  • Error handling: Respects continueOnFail(), provides detailed context with itemIndex
  • No security vulnerabilities: No eval(), exec(), command injection, or hardcoded secrets

Implementation ✅

  • Clean architecture: Dedicated functions for each operation
  • Comprehensive API coverage: Full support for Search, Contents, and Express APIs
  • Type safety: Proper TypeScript typing throughout
  • User-friendly responses: Transform Express API output to intuitive format
  • Conditional UI fields: livecrawl_formats shows only when livecrawl is set
  • Sensible defaults: count: 10, safesearch: moderate, enableWebSearch: true
  • TSDoc comments: Explains n8n framework requirements for classes

Documentation ✅

  • Clear README: Installation instructions, complete parameter tables, use cases
  • Parameter descriptions: User-friendly descriptions in node configuration
  • API references: Links to You.com documentation

🔧 Issues to Fix (2 minor)

1. README Package Name (Lines 10, 17)

Issue: Installation instructions use unscoped name n8n-nodes-youdotcom but package.json has scoped name @youdotcom-oss/n8n-nodes-youdotcom.

Fix: Update both installation commands to use the scoped package name:

npm install @youdotcom-oss/n8n-nodes-youdotcom

See inline comment →

2. User-Agent String (YouDotCom.node.ts:16)

Issue: User-Agent uses unscoped name n8n-nodes-youdotcom but should match package.json.

Fix:

const USER_AGENT = `@youdotcom-oss/n8n-nodes-youdotcom/${PACKAGE_VERSION} (https://github.com/youdotcom-oss/dx-toolkit)`;

See inline comment →


📋 Package Naming Discussion

n8n Naming Requirements ✅

Tyler correctly identified that n8n requires the n8n-nodes-* prefix based on their ESLint rule. Allowed formats:

  • n8n-nodes-[PACKAGE-NAME] (unscoped)
  • @[AUTHOR]/n8n-nodes-[PACKAGE-NAME] (scoped) ✅

Current naming: @youdotcom-oss/n8n-nodes-youdotcom

This correctly follows the scoped format with proper You.com branding while meeting n8n's requirements.

Monorepo Convention Exception

The monorepo convention (AGENTS.md:90-104) expects packages like @youdotcom-oss/mcppackages/mcp/. The n8n package is an exception due to n8n's framework requirements (similar to how n8n requires classes instead of arrow functions).

Recommendation: Document this exception in AGENTS.md naming section:

**Exception:** n8n packages use `@youdotcom-oss/n8n-nodes-[PACKAGE]` naming because n8n 
community nodes require the `n8n-nodes-*` prefix for package discovery per n8n's ESLint 
validation rules.

See inline comment →


✅ Previous Feedback Addressed

Integration Tests - RESOLVED ✅

EdwardIrby's concern: "These don't actually test any integration... Delete this unless you're specially testing a fetch function of your own."

Status:RESOLVED - Integration tests file completely removed (commit b2c868b).

Current test coverage: 43 unit tests validating node configuration, operations, parameters, and credentials. This is appropriate for an n8n node wrapper.


💡 Additional Observations (Non-Blocking)

  1. Missing publish workflow: No workflow found at .github/workflows/publish-n8n-nodes-youdotcom.yml. You'll need to create one to publish to npm. See .github/workflows/_publish-package.yml for the reusable workflow pattern, or .github/workflows/publish-mcp.yml if special deployment is needed.

  2. Dual authentication: The implementation correctly handles different auth methods for different API endpoints (X-API-Key for Search/Contents, Bearer for Express). Well documented in code comments.

  3. Build approach: Uses bundled distribution (builds to dist/) which is appropriate for n8n community nodes.


🎯 Before Merge Checklist

  • Fix README package name references (lines 10, 17)
  • Fix User-Agent string to match scoped package name
  • Create publish workflow (or confirm one exists)
  • Optional: Document n8n naming exception in AGENTS.md

Final Verdict

Code Quality:Excellent - Production-ready implementation

Security:Strong - No vulnerabilities, proper key protection, good error handling

Testing:Comprehensive - 43 tests with 120 assertions covering all features

Issues: 🔧 2 minor documentation fixes - Package name consistency in README and User-Agent

Recommendation: APPROVE with minor fixes

This is outstanding work with clean implementation, thorough testing, and excellent attention to security. The two naming consistency issues are trivial and can be fixed quickly. Great job, @tyler5673! 🚀

Follow the [installation guide](https://docs.n8n.io/integrations/community-nodes/installation/) in the n8n community nodes documentation.

```bash
npm install @youdotcom-oss/n8n-nodes-youdotcom
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Package Name Should Be Scoped

Issue: The installation command uses the unscoped package name, but package.json specifies @youdotcom-oss/n8n-nodes-youdotcom (scoped).

Fix:

npm install @youdotcom-oss/n8n-nodes-youdotcom

Also needs to be updated on line 17 in the UI installation instructions.

const PACKAGE_VERSION = '0.1.0';

/** User-Agent string for API requests */
const USER_AGENT = `n8n-nodes-youdotcom/${PACKAGE_VERSION} (https://github.com/youdotcom-oss/dx-toolkit)`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User-Agent Should Match Scoped Package Name

Current: Uses n8n-nodes-youdotcom (unscoped)

Suggested: Match the scoped package name from package.json:

const USER_AGENT = `@youdotcom-oss/n8n-nodes-youdotcom/${PACKAGE_VERSION} (https://github.com/youdotcom-oss/dx-toolkit)`;

This provides better identification for API usage tracking and matches the published npm package name.

@@ -0,0 +1,88 @@
{
"name": "@youdotcom-oss/n8n-nodes-youdotcom",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Package Naming Verified

Tyler correctly confirmed that n8n requires the n8n-nodes-* prefix based on n8n's ESLint rule. The allowed formats are:

  • n8n-nodes-[PACKAGE-NAME] (unscoped)
  • @[AUTHOR]/n8n-nodes-[PACKAGE-NAME] (scoped) ✅

Current naming: @youdotcom-oss/n8n-nodes-youdotcom

This correctly follows the scoped format. The -youdotcom suffix distinguishes it from other packages in the monorepo while meeting n8n's requirements.

Note: This is an exception to the monorepo convention (similar to how n8n requires classes instead of arrow functions). Consider documenting this in AGENTS.md.

displayName: 'API Key',
name: 'apiKey',
type: 'string',
typeOptions: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Security: API Key Properly Protected

Good security practices:

  • typeOptions: { password: true } masks the API key in n8n's UI
  • ✅ Credential test validates the key without exposing it (lines 37-47)
  • ✅ Uses n8n's built-in credential management
  • ✅ No hardcoded secrets

Authentication:

  • Search/Contents APIs: X-API-Key header (line 32)
  • Express API: Bearer token (YouDotCom.node.ts:605)

Both methods are properly implemented. No changes needed.

returnData.push(...executionData);
}
} catch (error) {
if (this.continueOnFail()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Error Handling: Well Implemented

Good practices:

  • ✅ Respects continueOnFail() setting for batch processing
  • ✅ Provides detailed error context with itemIndex
  • ✅ Uses proper n8n error types (NodeApiError)
  • ✅ Graceful degradation - errors don't stop the entire workflow

This follows n8n best practices perfectly.

};

// Parse comma-separated URLs and trim whitespace
const urls = urlsString
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Input Sanitization: Good Implementation

Secure URL parsing:

  • ✅ Safely splits comma-separated input
  • ✅ Trims whitespace from each URL
  • ✅ Filters empty strings
  • ✅ Validates at least one URL exists (lines 550-552)

No injection risks - URLs are passed to the API, not executed. Clear error message if validation fails.

agent?: string;
};

// Transform response to a more user-friendly format
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Response Transformation: User-Friendly Design

Excellent UX implementation:

  • ✅ Transforms raw API output to intuitive format
  • ✅ Extracts answer from message.answer type
  • ✅ Extracts searchResults from web_search.results type
  • ✅ Clear, predictable output structure

Result:

{
  agent: "express",
  answer: "AI-generated response...",
  searchResults: [{ url, title, snippet }, ...]
}

This makes the response much easier to work with in n8n workflows compared to the raw API format.

@tyler5673 tyler5673 marked this pull request as ready for review January 29, 2026 22:49
@claude
Copy link
Contributor

claude bot commented Jan 29, 2026

Claude finished @tyler5673's task in 2m 43s —— View job


Code Review Complete ✅


Summary

This PR demonstrates excellent code quality with thorough testing, proper security practices, and clean implementation. All previous feedback has been addressed (integration tests removed per EdwardIrby's request). The code is production-ready with just one minor suggestion (User-Agent enhancement).


✅ What's Excellent

Code Quality

  • All tests pass: 43/43 unit tests with 120 assertions
  • Quality checks pass: Biome, TypeScript, and package format all clean
  • Comprehensive test coverage: Node config, all operations, parameters, credentials
  • Clean implementation: Well-organized, readable, follows TypeScript best practices

Security

  • API key protection: typeOptions: { password: true } masks keys in UI (credentials:19)
  • Proper authentication: Different methods for different APIs (X-API-Key for Search/Contents, Bearer for Express)
  • Credential validation: Test request validates API keys safely (credentials:37-47)
  • Input sanitization: URL parsing with proper validation (node:577-584)
  • Error handling: Respects continueOnFail(), provides detailed context with itemIndex (node:476-514)
  • No security vulnerabilities: No eval(), exec(), command injection, or hardcoded secrets

Implementation

  • Clean architecture: Dedicated private methods for each operation (#executeSearch, #executeContents, #executeExpress)
  • Comprehensive API coverage: Full support for Search, Contents, and Express APIs
  • Type safety: Proper TypeScript typing throughout with Zod validation
  • User-friendly responses: Transform Express API output to intuitive format (node:660-676)
  • Conditional UI fields: livecrawl_formats only shows when livecrawl is set (node:296-300)
  • Sensible defaults: count: 10, safesearch: moderate, enableWebSearch: true
  • TSDoc comments: Explains n8n framework requirements for classes (node:26-31)

Documentation

  • Clear README: Installation instructions, complete parameter tables, use cases
  • Parameter descriptions: User-friendly descriptions in node configuration
  • API references: Links to You.com documentation

💡 Minor Suggestion (Non-Blocking)

User-Agent Enhancement (YouDotCom.node.ts:24):

Consider including the npm org scope for better API usage tracking:

const USER_AGENT = `@youdotcom-oss/n8n-nodes-youdotcom/${PACKAGE_VERSION} (https://github.com/youdotcom-oss/dx-toolkit)`;

See inline comment →


📋 Package Naming Discussion

n8n Naming Requirements ✅

Tyler correctly identified that n8n requires the n8n-nodes-* prefix based on their ESLint rule. Allowed formats:

  • n8n-nodes-[PACKAGE-NAME] (unscoped)
  • @[AUTHOR]/n8n-nodes-[PACKAGE-NAME] (scoped) ✅

Current naming: @youdotcom-oss/n8n-nodes-youdotcom

This correctly follows the scoped format with proper You.com branding while meeting n8n's requirements.

Monorepo Convention Exception

The monorepo convention (AGENTS.md:90-104) expects packages like @youdotcom-oss/mcppackages/mcp/. The n8n package is an exception due to n8n's framework requirements (similar to how n8n requires classes instead of arrow functions).

Recommendation: Document this exception in AGENTS.md naming section:

**Exception**: n8n packages use `@youdotcom-oss/n8n-nodes-[PACKAGE]` naming because n8n 
community nodes require the `n8n-nodes-*` prefix for package discovery per n8n's ESLint 
validation rules.

See inline comment →


✅ Previous Feedback Addressed

Integration Tests - RESOLVED ✅

EdwardIrby's concern: "These don't actually test any integration... Delete this unless you're specially testing a fetch function of your own."

Status: ✅ RESOLVED - Integration tests file completely removed (commit b2c868b).

Current test coverage: 43 unit tests validating node configuration, operations, parameters, and credentials. This is appropriate and comprehensive for an n8n node wrapper.


📝 Additional Observations

  1. Missing publish workflow: No workflow found at .github/workflows/publish-n8n-nodes-youdotcom.yml. You'll need to create one to publish to npm. See .github/workflows/_publish-package.yml for the reusable workflow pattern.

  2. Dual authentication: The implementation correctly handles different auth methods for different API endpoints (X-API-Key for Search/Contents at ydc-index.io, Bearer for Express at api.you.com). Well documented in code comments (node:612-614).

  3. Build approach: Uses bundled distribution (builds to dist/) which is appropriate for n8n community nodes. Build process includes JS, types, and assets (package.json:53-56).


🎯 Final Verdict

Code Quality: ✅ Excellent - Production-ready implementation

Security: ✅ Strong - No vulnerabilities, proper key protection, good error handling

Testing: ✅ Comprehensive - 43 tests with 120 assertions covering all features

Issues: 💡 One optional enhancement - User-Agent string could include package scope

Recommendation: APPROVE

This is outstanding work with clean implementation, thorough testing, and excellent attention to security and user experience. The n8n community will benefit from having a high-quality You.com integration. Great job, @tyler5673! 🚀


Before publishing:

  • Create publish workflow (.github/workflows/publish-n8n-nodes-youdotcom.yml)
  • Optional: Document n8n naming exception in AGENTS.md
  • Optional: Enhance User-Agent string with package scope

Follow the [installation guide](https://docs.n8n.io/integrations/community-nodes/installation/) in the n8n community nodes documentation.

```bash
npm install @youdotcom-oss/n8n-nodes-youdotcom
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 Package Name Mismatch

Issue: Installation command shows @youdotcom-oss/n8n-nodes-youdotcom which is correct and matches package.json.

Previous versions of my review incorrectly flagged this as an issue. The README is actually correct - it uses the scoped package name that matches package.json:2.

No changes needed here. ✅

const PACKAGE_VERSION = '0.1.0';

/** User-Agent string for API requests */
const USER_AGENT = `n8n-nodes-youdotcom/${PACKAGE_VERSION} (https://github.com/youdotcom-oss/dx-toolkit)`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Consider Including Package Scope in User-Agent

Current: n8n-nodes-youdotcom/${PACKAGE_VERSION}

Suggestion: Consider including the npm org scope for better identification:

const USER_AGENT = `@youdotcom-oss/n8n-nodes-youdotcom/${PACKAGE_VERSION} (https://github.com/youdotcom-oss/dx-toolkit)`;

Benefits:

  • Clearer identification in API logs
  • Matches published npm package name
  • Includes GitHub repo for support inquiries

This is a minor enhancement, not blocking.

displayName: 'API Key',
name: 'apiKey',
type: 'string',
typeOptions: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Security: API Key Properly Protected

Excellent security implementation:

  • typeOptions: { password: true } masks the API key in n8n's UI
  • ✅ Credential test validates the key without exposing it (lines 37-47)
  • ✅ Uses n8n's built-in credential management
  • ✅ No hardcoded secrets

Authentication methods:

  • Search/Contents APIs: X-API-Key header (line 32)
  • Express API: Bearer token (YouDotCom.node.ts:648)

Both authentication methods are properly implemented. No changes needed.

});
returnData.push(...executionData);
}
} catch (error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Error Handling: Excellent Implementation

Good practices:

  • ✅ Handles Zod validation errors with detailed messages (lines 478-498)
  • ✅ Respects continueOnFail() setting for batch processing
  • ✅ Provides detailed error context with itemIndex
  • ✅ Uses proper n8n error types (NodeApiError)
  • ✅ Graceful degradation - errors don't stop the entire workflow

Error flow:

  1. Catches Zod validation errors separately with formatted messages
  2. Catches other errors generically
  3. If continueOnFail() is true: Returns error object, continues processing
  4. If continueOnFail() is false: Throws NodeApiError with item context

This follows n8n best practices perfectly. No changes needed.

const options = ContentsOptionsSchema.parse(rawOptions);

// Parse comma-separated URLs and trim whitespace
const urls = urlsString
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Input Sanitization: Good Implementation

Secure URL parsing:

  • ✅ Safely splits comma-separated input
  • ✅ Trims whitespace from each URL
  • ✅ Filters empty strings
  • ✅ Validates at least one URL exists (lines 582-584)

Security considerations:

  • No injection risks (URLs are passed to API, not executed)
  • Proper validation before API call
  • Clear error message if validation fails

No changes needed.

// Validate API response with Zod schema
const response = ExpressResponseSchema.parse(rawResponse);

// Transform response to a more user-friendly format
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Response Transformation: User-Friendly Design

Excellent UX implementation:

  • ✅ Transforms raw API output to intuitive format
  • ✅ Extracts answer from message.answer type (line 668-669)
  • ✅ Extracts searchResults from web_search.results type (line 670-671)
  • ✅ Clear, predictable output structure

Result format:

{
  agent: "express",
  answer: "AI-generated response...",
  searchResults: [{ url, title, snippet }, ...]
}

This makes the response much easier to work with in n8n workflows compared to the raw API format. Well done!

@@ -0,0 +1,91 @@
{
"name": "@youdotcom-oss/n8n-nodes-youdotcom",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Package Naming: Correct Implementation

n8n requires the n8n-nodes-* prefix per their ESLint validation rule.

Allowed formats:

  • Unscoped: n8n-nodes-[PACKAGE-NAME]
  • Scoped: @[AUTHOR]/n8n-nodes-[PACKAGE-NAME] ✅ (This package uses this format)

Current naming: @youdotcom-oss/n8n-nodes-youdotcom

This correctly follows the scoped format. The -youdotcom suffix distinguishes it from other You.com packages in the monorepo.

Note: This is an exception to the monorepo convention (similar to how n8n requires classes instead of arrow functions). Consider documenting this in AGENTS.md as suggested in previous reviews.

@claude
Copy link
Contributor

claude bot commented Jan 30, 2026

Claude finished @tyler5673's task in 3m 0s —— View job


Code Review Complete ✅


Summary

This PR adds a high-quality n8n community node for You.com APIs with excellent implementation. The code is production-ready with comprehensive testing, proper security practices, outstanding error handling, and great attention to user experience. All previous feedback has been properly addressed.


✅ What's Excellent

Code Quality

  • All tests pass: 43/43 unit tests with 120 assertions
  • Quality checks pass: Biome, TypeScript, and package format all clean (11 files checked)
  • Comprehensive test coverage: Node config, all operations, parameters, credentials
  • Clean implementation: Well-organized, readable, follows TypeScript best practices
  • Outstanding Zod validation: Uses .passthrough() pattern to preserve API fields while validating required data

Security

  • API key protection: typeOptions: { password: true } masks keys in UI
  • Proper authentication: Different methods for different APIs (X-API-Key for Search/Contents, Bearer for Express)
  • Credential validation: Test request validates API keys safely without exposure
  • Input sanitization: URL parsing with proper validation and whitespace trimming
  • Error handling: Excellent Zod validation error formatting, respects continueOnFail(), provides detailed context with itemIndex
  • No security vulnerabilities: No eval(), exec(), command injection, or hardcoded secrets

Implementation

  • Clean architecture: Dedicated private static methods for each operation (#executeSearch, #executeContents, #executeExpress)
  • Comprehensive API coverage: Full support for Search, Contents, and Express APIs with all parameters
  • Type safety: Proper TypeScript typing throughout with Zod validation at runtime
  • User-friendly responses: Transforms Express API output to intuitive flat format (answer, searchResults)
  • Conditional UI fields: livecrawl_formats only shows when livecrawl is set (lines 296-300)
  • Sensible defaults: count: 10, safesearch: moderate, enableWebSearch: true
  • Framework compliance: TSDoc comments explain n8n requirements for classes (lines 26-31)

Documentation

  • Clear README: Installation instructions, complete parameter tables, use cases
  • Parameter descriptions: User-friendly descriptions in node configuration
  • API references: Links to You.com documentation
  • Code comments: Explains authentication differences between APIs (lines 612-614)

📋 Previous Feedback: All Resolved ✅

Integration Tests - RESOLVED ✅

EdwardIrby's concern: "These don't actually test any integration... Delete this unless you're specially testing a fetch function of your own."

Status: ✅ RESOLVED - Integration tests file completely removed (commit b2c868b).

Current coverage: 43 unit tests validating node configuration, operations, parameters, and credentials. This is appropriate and comprehensive for an n8n node wrapper. The underlying API functionality is tested in the @youdotcom-oss/mcp package.

Package Naming - RESOLVED ✅

EdwardIrby's concern: "Rename this we have a NPM org @youdotcom-oss... unless n8n wants this a specific way."

Status: ✅ RESOLVED - Tyler correctly identified that n8n requires the n8n-nodes-* prefix per their ESLint validation rule.

Current implementation: @youdotcom-oss/n8n-nodes-youdotcom correctly follows n8n's scoped format: @[AUTHOR]/n8n-nodes-[PACKAGE-NAME]

This is an exception to the monorepo convention (similar to how n8n requires classes instead of arrow functions), and is properly documented in the code with TSDoc comments.


💡 Additional Observations (Non-Blocking)

  1. Missing publish workflow: No workflow found at .github/workflows/publish-n8n-nodes-youdotcom.yml. You'll need to create one to publish to npm. See .github/workflows/_publish-package.yml for the reusable workflow pattern, or .github/workflows/publish-mcp.yml if special deployment is needed.

  2. Dual authentication: The implementation correctly handles different auth methods for different API endpoints:

    • Search/Contents: X-API-Key header at ydc-index.io
    • Express: Bearer token at api.you.com
    • Well documented in code comments (lines 612-614)
  3. Build approach: Uses bundled distribution (builds to dist/) which is appropriate for n8n community nodes. Build process includes JS, types, and assets (package.json:53-56).

  4. Schema design: Excellent use of Zod's .passthrough() to preserve all API fields while validating required data. This makes the node future-proof as the API evolves.

  5. Test coverage: 43 tests with 120 assertions covering:

    • Node metadata and configuration
    • All three operations (Search, Contents, Express)
    • Parameter validation and constraints
    • Credential configuration
    • Display options and conditional fields

🎯 Final Verdict

Code Quality: ✅ Excellent - Production-ready implementation

Security: ✅ Strong - No vulnerabilities, proper key protection, excellent error handling

Testing: ✅ Comprehensive - 43 tests with 120 assertions covering all features

Standards Compliance: ✅ Good - Follows project patterns with documented exceptions for n8n requirements

Recommendation: APPROVE

This is outstanding work with clean implementation, thorough testing, and excellent attention to security and user experience. The n8n community will benefit from having a high-quality You.com integration with support for Search, Contents extraction, and AI-powered Express agent.

Great job, @tyler5673! 🚀


Before publishing (non-blocking):

  • Create publish workflow (.github/workflows/publish-n8n-nodes-youdotcom.yml)
  • Optional: Document n8n naming exception in AGENTS.md for future reference

@@ -0,0 +1,91 @@
{
"name": "@youdotcom-oss/n8n-nodes-youdotcom",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Package Naming: Correctly Implements n8n Requirements

Excellent work resolving the naming discussion! The package correctly follows n8n's validation rules:

n8n's requirement (source):

  • Scoped format: @[AUTHOR]/n8n-nodes-[PACKAGE-NAME]

Current implementation:

  • Package: @youdotcom-oss/n8n-nodes-youdotcom
  • Directory: packages/n8n-nodes-youdotcom/
  • Maintains You.com branding while meeting n8n framework requirements

Monorepo alignment: While this differs from the typical pattern (@youdotcom-oss/mcppackages/mcp/), it's justified because n8n's ESLint rule validates package names and requires the n8n-nodes-* prefix.

Note: This is an exception to the monorepo convention, similar to how n8n requires classes instead of arrow functions. This is properly documented in the code with TSDoc comments.

displayName: 'API Key',
name: 'apiKey',
type: 'string',
typeOptions: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Security: API Key Properly Protected

Excellent security implementation:

  • typeOptions: { password: true } masks the API key in n8n's UI (line 19-20)
  • ✅ Credential test validates the key without exposing it (lines 37-47)
  • ✅ Uses n8n's built-in credential management framework
  • ✅ No hardcoded secrets anywhere in the codebase

Authentication methods:

  • Search/Contents APIs: X-API-Key header (line 32)
  • Express API: Bearer token (YouDotCom.node.ts:648)

Both authentication methods are properly secured with different approaches based on API requirements. No changes needed.

});
returnData.push(...executionData);
}
} catch (error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Error Handling: Excellent Implementation

Outstanding error handling implementation:

  • ✅ Separate handling for Zod validation errors with detailed messages (lines 478-498)
  • ✅ Respects continueOnFail() setting for batch processing workflows
  • ✅ Provides detailed error context with itemIndex for debugging
  • ✅ Uses proper n8n error types (NodeApiError, NodeOperationError)
  • ✅ Graceful degradation - errors don't stop the entire workflow

Error flow:

  1. Catches Zod validation errors separately with formatted, user-friendly messages
  2. Catches other errors generically
  3. If continueOnFail() is true: Returns error object as data, continues processing
  4. If continueOnFail() is false: Throws NodeApiError with item context

This follows n8n best practices perfectly. Well done!

const options = ContentsOptionsSchema.parse(rawOptions);

// Parse comma-separated URLs and trim whitespace
const urls = urlsString
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Input Sanitization: Good Implementation

Secure URL parsing (lines 577-584):

  • ✅ Safely splits comma-separated input
  • ✅ Trims whitespace from each URL
  • ✅ Filters empty strings
  • ✅ Validates at least one URL exists before API call

Security considerations:

  • No injection risks (URLs are passed to API, not executed locally)
  • Proper validation with clear error message if validation fails
  • Edge cases handled (empty strings, extra whitespace)

No changes needed.

// Validate API response with Zod schema
const response = ExpressResponseSchema.parse(rawResponse);

// Transform response to a more user-friendly format
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Response Transformation: User-Friendly Design

Excellent UX implementation (lines 660-676):

  • ✅ Transforms raw API output to intuitive, flat structure
  • ✅ Extracts answer from message.answer type (lines 668-669)
  • ✅ Extracts searchResults from web_search.results type (lines 670-671)
  • ✅ Clear, predictable output format

Result format:

{
  agent: "express",
  answer: "AI-generated response with citations...",
  searchResults: [{ url, title, snippet }, ...]
}

This makes the Express API response much easier to work with in n8n workflows compared to the raw API format. Users can directly access the answer and search results without complex data manipulation. Great attention to user experience!

snippets: z.array(z.string()).optional().describe('Content snippets'),
page_age: z.string().optional().describe('Publication timestamp'),
})
.passthrough();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Schema Design: Excellent Use of passthrough()

Outstanding schema design (lines 39-76):

  • ✅ Validates required fields (url, title, description)
  • ✅ Uses .passthrough() to preserve additional API fields
  • ✅ Clear JSDoc comments explaining the strategy (lines 32-38)

Why this is excellent:

  • Type safety: Required fields are validated
  • Future-proof: New API fields automatically pass through
  • No data loss: Users get all API data (snippets, page_age, etc.)
  • Flexibility: API can add fields without breaking the node

This pattern (validate core fields + passthrough) is the right approach for API wrappers. It balances type safety with flexibility. The same pattern is consistently applied to all response schemas (Search, Contents, Express). Well done!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EdwardIrby lmk if you have strong feelings against using passthrough. The schema was stripping out fields it didn't explicitly know about, which meant the response was coming back empty for search. Using passthrough seemed like a good choice for future proofing, and since the n8n node is a thin API wrapper where users expect access to the full API response, this sounded like the right choice to me vs a more explicit schema.

@claude
Copy link
Contributor

claude bot commented Jan 30, 2026

Claude finished @tyler5673's task in 1m 44s —— View job


Code Review Complete ✅


Summary

This PR adds a high-quality n8n community node for You.com APIs with excellent implementation. The code is production-ready with comprehensive testing, proper security practices, outstanding error handling, and great attention to user experience. All previous review feedback has been properly addressed.


✅ What's Excellent

Code Quality

  • All tests pass: 43/43 unit tests with 120 assertions
  • Quality checks pass: Biome, TypeScript, and package format all clean (11 files checked)
  • Comprehensive test coverage: Node config, all operations, parameters, credentials
  • Clean implementation: Well-organized, readable, follows TypeScript best practices
  • Outstanding Zod validation: Uses .passthrough() pattern to preserve API fields while validating required data (YouDotCom.schemas.ts:39-76)

Security

  • API key protection: typeOptions: { password: true } masks keys in UI (credentials:19-20)
  • Proper authentication: Different methods for different APIs (X-API-Key for Search/Contents, Bearer for Express)
  • Credential validation: Test request validates API keys safely without exposure (credentials:37-47)
  • Input sanitization: URL parsing with proper validation and whitespace trimming (node:577-584)
  • Error handling: Excellent Zod validation error formatting with detailed messages (node:478-498), respects continueOnFail(), provides detailed context with itemIndex
  • No security vulnerabilities: No eval(), exec(), command injection, or hardcoded secrets

Implementation

  • Clean architecture: Dedicated private static methods for each operation (#executeSearch, #executeContents, #executeExpress)
  • Comprehensive API coverage: Full support for Search, Contents, and Express APIs with all parameters
  • Type safety: Proper TypeScript typing throughout with Zod validation at runtime
  • User-friendly responses: Transforms Express API output to intuitive flat format (answer, searchResults) - lines 666-682
  • Conditional UI fields: livecrawl_formats only shows when livecrawl is set (node:296-300)
  • Sensible defaults: count: 10, safesearch: moderate, enableWebSearch: true
  • Framework compliance: TSDoc comments explain n8n requirements for classes (node:26-31, credentials:3-8)

Documentation

  • Clear README: Installation instructions, complete parameter tables, use cases
  • Parameter descriptions: User-friendly descriptions in node configuration
  • API references: Links to You.com documentation
  • Code comments: Explains authentication differences between APIs (node:612-614)

📋 Previous Feedback: All Resolved ✅

Integration Tests - RESOLVED ✅

EdwardIrby's concern: "These don't actually test any integration... Delete this unless you're specially testing a fetch function of your own."

Status: ✅ RESOLVED - Integration tests file completely removed (commit b2c868b).

Current coverage: 43 unit tests validating node configuration, operations, parameters, and credentials. This is appropriate and comprehensive for an n8n node wrapper. The underlying API functionality is tested in the @youdotcom-oss/mcp package.

Package Naming - RESOLVED ✅

EdwardIrby's concern: "Rename this we have a NPM org @youdotcom-oss... unless n8n wants this a specific way."

Status: ✅ RESOLVED - Tyler correctly identified that n8n requires the n8n-nodes-* prefix per their ESLint validation rule.

Current implementation: @youdotcom-oss/n8n-nodes-youdotcom correctly follows n8n's scoped format: @[AUTHOR]/n8n-nodes-[PACKAGE-NAME]

This is an exception to the monorepo convention (similar to how n8n requires classes instead of arrow functions), and is properly documented in the code with TSDoc comments.


💡 Schema Design Highlight

The Zod schema design is particularly well done (YouDotCom.schemas.ts:39-76):

const WebResultSchema = z.object({
  url: z.string(),
  title: z.string(),
  description: z.string(),
  snippets: z.array(z.string()).optional(),
  page_age: z.string().optional(),
}).passthrough();  // ← Preserves all API fields

Why this is excellent:

  • Type safety: Required fields are validated
  • Future-proof: New API fields automatically pass through
  • No data loss: Users get all API data (snippets, page_age, etc.)
  • Flexibility: API can add fields without breaking the node

This pattern (validate core fields + passthrough) is consistently applied to all response schemas (Search, Contents, Express). Perfect for API wrappers!


📝 Additional Observations (Non-Blocking)

  1. Missing publish workflow: No workflow found at .github/workflows/publish-n8n-nodes-youdotcom.yml. You'll need to create one to publish to npm. See .github/workflows/_publish-package.yml for the reusable workflow pattern.

  2. Dual authentication (well documented): The implementation correctly handles different auth methods for different API endpoints:

    • Search/Contents: X-API-Key header at ydc-index.io
    • Express: Bearer token at api.you.com
    • Well documented in code comments (node:612-614)
  3. Build approach: Uses bundled distribution (builds to dist/) which is appropriate for n8n community nodes. Build process includes JS, types, and assets (package.json:53-56).

  4. Test coverage breakdown:

    • Node metadata and configuration (11 tests)
    • All three operations with parameters (32 tests)
    • Credential configuration and authentication
  5. n8n naming exception: Consider documenting this in AGENTS.md for future reference:

    **Exception**: n8n packages use `@youdotcom-oss/n8n-nodes-[PACKAGE]` naming because n8n 
    community nodes require the `n8n-nodes-*` prefix for package discovery per n8n's ESLint 
    validation rules (similar to how n8n requires classes instead of arrow functions).

🎯 Final Verdict

Code Quality: ✅ Excellent - Production-ready implementation

Security: ✅ Strong - No vulnerabilities, proper key protection, excellent error handling

Testing: ✅ Comprehensive - 43 tests with 120 assertions covering all features

Standards Compliance: ✅ Good - Follows project patterns with documented exceptions for n8n requirements

Recommendation: APPROVE

This is outstanding work with clean implementation, thorough testing, and excellent attention to security and user experience. The n8n community will benefit from having a high-quality You.com integration with support for Search, Contents extraction, and AI-powered Express agent.

Great job, @tyler5673! 🚀


Before publishing (non-blocking):

  • Create publish workflow (.github/workflows/publish-n8n-nodes-youdotcom.yml)
  • Optional: Document n8n naming exception in AGENTS.md for future reference

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants