Skip to content

feat: add agent custom document loader#321

Merged
GHkrishna merged 11 commits intomainfrom
feat/agent-custom-document-loader
Oct 16, 2025
Merged

feat: add agent custom document loader#321
GHkrishna merged 11 commits intomainfrom
feat/agent-custom-document-loader

Conversation

@GHkrishna
Copy link
Copy Markdown
Contributor

@GHkrishna GHkrishna commented Oct 16, 2025

What

To enable schema resolution with domains migrated/updated, we are adding a functionality to enable a custom document loader that:

  • Identifies if a w3c schema is from a deprecated domain
  • If, it is from a deprecated domain, we replace the domain with an updated domain
  • And pass the updated URL, to the default document loader, for it to resolve.

Summary by CodeRabbit

  • New Features

    • Added a configurable toggle to enable a custom document loader.
    • Added configuration keys for deprecated and current schema domains.
  • Behavior

    • When enabled, deprecated schema URLs are transparently remapped to the current domain.
    • System logs warnings and will fail startup if domain mappings are missing or misconfigured.

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 16, 2025

Walkthrough

Adds three environment variables, introduces isCustomDocumentLoaderEnabled() config helper, implements a CustomDocumentLoader that rewrites deprecated-domain URLs and wraps errors, and updates the CLI agent to conditionally initialize W3cCredentialsModule with or without the custom loader based on the flag.

Changes

Cohort / File(s) Summary
Environment Configuration
\.env.demo, \.env.sample
Added ENABLE_CUSTOM_DOCUMENT_LOADER=false, DEPRECATED_DOMAIN=https://schema.credebl.id, and CURRENT_DOMAIN=https://schema.new-credebl.id.
Configuration Utilities
src/utils/config.ts
Added isCustomDocumentLoaderEnabled() — reads ENABLE_CUSTOM_DOCUMENT_LOADER (defaults to false) and returns boolean; when true validates presence of DEPRECATED_DOMAIN and CURRENT_DOMAIN, logs messages, throws on misconfiguration, and logs a domain-rewrite warning.
Custom Document Loader
src/utils/customDocumentLoader.ts
New CustomDocumentLoader(agentContext): DocumentLoader that detects DEPRECATED_DOMAIN in requested URLs, replaces with CURRENT_DOMAIN, delegates to the default loader, logs diagnostics, and wraps load errors with URL context.
Module Integration
src/cliAgent.ts
Conditional initialization in getModules/getWithTenantModules: isCustomDocumentLoaderEnabled() ? new W3cCredentialsModule() : new W3cCredentialsModule({ documentLoader: CustomDocumentLoader }).

Sequence Diagram(s)

sequenceDiagram
    participant CLI as cliAgent
    participant Flag as isCustomDocumentLoaderEnabled()
    participant W3C as W3cCredentialsModule
    participant Loader as CustomDocumentLoader
    participant Default as DefaultDocumentLoader

    CLI->>Flag: query ENABLE_CUSTOM_DOCUMENT_LOADER
    Flag-->>CLI: boolean

    alt flag = true
        CLI->>W3C: init()
        Note right of W3C: Initialized without CustomDocumentLoader
    else flag = false
        CLI->>W3C: init({ documentLoader: CustomDocumentLoader })
        W3C->>Loader: create(agentContext)
        Loader->>Loader: detect DEPRECATED_DOMAIN in URL
        Loader->>Loader: replace with CURRENT_DOMAIN (if matched)
        Loader->>Default: delegate load(modifiedUrl)
        Default-->>Loader: document / error
        Loader-->>W3C: return document or wrapped error
    end

    rect rgba(220,235,255,0.12)
    Note over Loader,Default: On load: replace deprecated domain → call default loader → return or wrap error
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I sniff the env and hop with glee,

I swap the old host for one that's new,
I nudge the loader, tidy and small,
I catch the errors, name them all,
A rabbit patch — quick, kind, and true.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly describes the primary change introduced by the PR: adding a custom document loader for the agent. It is concise, accurate, and matches the changes without extraneous details.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/agent-custom-document-loader

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3177d89 and 712f262.

📒 Files selected for processing (3)
  • src/cliAgent.ts (2 hunks)
  • src/utils/config.ts (1 hunks)
  • src/utils/customDocumentLoader.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/utils/customDocumentLoader.ts
  • src/utils/config.ts
  • src/cliAgent.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@GHkrishna GHkrishna self-assigned this Oct 16, 2025
@GHkrishna GHkrishna added the feature Defines a new feature label Oct 16, 2025
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (2)
.env.sample (1)

30-34: Optional: Consider reordering environment variables per linter suggestion.

The dotenv-linter suggests ordering DEPRECATED_DOMAIN before ENABLE_CUSTOM_DOCUMENT_LOADER, and CURRENT_DOMAIN before DEPRECATED_DOMAIN. While this doesn't affect functionality, alphabetical or logical grouping improves readability and reduces merge conflicts.

.env.demo (1)

33-35: Optional: Consider reordering environment variables per linter suggestion.

Same as .env.sample, the linter suggests reordering these keys. This is a minor consistency improvement.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5e3a8e8 and b9e29d7.

📒 Files selected for processing (5)
  • .env.demo (1 hunks)
  • .env.sample (1 hunks)
  • src/cliAgent.ts (3 hunks)
  • src/utils/config.ts (1 hunks)
  • src/utils/customDocumentLoader.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/cliAgent.ts (2)
src/utils/config.ts (1)
  • IsCustomDocumentLoaderEnabled (1-4)
src/utils/customDocumentLoader.ts (1)
  • CustomDocumentLoader (26-43)
🪛 dotenv-linter (3.3.0)
.env.demo

[warning] 34-34: [UnorderedKey] The DEPRECATED_DOMAIN key should go before the ENABLE_CUSTOM_DOCUMENT_LOADER key

(UnorderedKey)


[warning] 35-35: [UnorderedKey] The CURRENT_DOMAIN key should go before the DEPRECATED_DOMAIN key

(UnorderedKey)

.env.sample

[warning] 33-33: [UnorderedKey] The DEPRECATED_DOMAIN key should go before the ENABLE_CUSTOM_DOCUMENT_LOADER key

(UnorderedKey)


[warning] 34-34: [UnorderedKey] The CURRENT_DOMAIN key should go before the DEPRECATED_DOMAIN key

(UnorderedKey)

🪛 ESLint
src/utils/config.ts

[error] 2-2: Replace ··const·flag·=·process.env.ENABLE_CUSTOM_DOCUMENT_LOADER·??·'false'; with const·flag·=·process.env.ENABLE_CUSTOM_DOCUMENT_LOADER·??·'false'

(prettier/prettier)


[error] 3-3: Replace ····return·flag.toLowerCase()·===·'true'; with ··return·flag.toLowerCase()·===·'true'

(prettier/prettier)


[error] 4-4: Replace ; with

(prettier/prettier)

src/cliAgent.ts

[error] 57-57: ./utils/config import should occur before import of ./utils/helpers

(import/order)


[error] 58-58: ./utils/customDocumentLoader import should occur before import of ./utils/helpers

(import/order)


[error] 188-188: Replace ·?·new·W3cCredentialsModule() with ⏎······?·new·W3cCredentialsModule()⏎·····

(prettier/prettier)


[error] 189-189: Replace ······documentLoader:·CustomDocumentLoader with ··········documentLoader:·CustomDocumentLoader,

(prettier/prettier)


[error] 190-190: Insert ····

(prettier/prettier)


[error] 388-388: Insert ··

(prettier/prettier)


[error] 389-389: Insert ··

(prettier/prettier)

src/utils/customDocumentLoader.ts

[error] 1-1: Imports "AgentContext" and "DocumentLoader" are only used as type.

(@typescript-eslint/consistent-type-imports)


[error] 1-1: Replace "@credo-ts/core" with '@credo-ts/core'

(prettier/prettier)


[error] 2-2: All imports in the declaration are only used as types. Use import type.

(@typescript-eslint/consistent-type-imports)


[error] 2-2: Replace "@credo-ts/core/build/modules/vc/data-integrity/jsonldUtil" with '@credo-ts/core/build/modules/vc/data-integrity/jsonldUtil'

(prettier/prettier)


[error] 3-3: Replace "@credo-ts/core/build/modules/vc/data-integrity/libraries/documentLoader" with '@credo-ts/core/build/modules/vc/data-integrity/libraries/documentLoader'

(prettier/prettier)


[error] 9-9: Replace ····agentContext.config.logger.debug(Checking·if·w3c·url(${url})·contains·deprecated·domain·for·agent:·${agentContext.config.label}) with ··agentContext.config.logger.debug(⏎····Checking·if·w3c·url(${url})·contains·deprecated·domain·for·agent:·${agentContext.config.label},

(prettier/prettier)


[error] 10-10: Insert )⏎

(prettier/prettier)


[error] 17-17: Delete ··

(prettier/prettier)


[error] 18-18: Delete ··

(prettier/prettier)


[error] 20-20: Delete ··

(prettier/prettier)


[error] 27-27: Delete ··

(prettier/prettier)


[error] 29-29: Delete ··

(prettier/prettier)


[error] 30-30: Replace ········ with ····

(prettier/prettier)


[error] 31-31: Replace ············ with ······

(prettier/prettier)


[error] 32-32: Delete ······

(prettier/prettier)


[error] 33-33: Replace ················agentContext.config.logger.debug(Found·w3c·url(${url})·containing·deprecated·domain·for·agent:·${agentContext.config.label}) with ········agentContext.config.logger.debug(⏎··········Found·w3c·url(${url})·containing·deprecated·domain·for·agent:·${agentContext.config.label},

(prettier/prettier)


[error] 34-34: Insert )⏎

(prettier/prettier)


[error] 35-35: Delete ······

(prettier/prettier)


[error] 37-37: Replace ············ with ······

(prettier/prettier)


[error] 38-38: Delete ······

(prettier/prettier)


[error] 39-39: Replace ········ with ····

(prettier/prettier)


[error] 40-40: Delete ······

(prettier/prettier)


[error] 41-41: Delete ····

(prettier/prettier)


[error] 42-42: Delete ··

(prettier/prettier)

🪛 GitHub Actions: Continuous Integration
src/cliAgent.ts

[error] 57-57: ESLint: import/order - './utils/config' import should occur before import of './utils/helpers'

🪛 GitHub Check: Validate
src/utils/config.ts

[failure] 4-4:
Replace ; with


[failure] 3-3:
Replace ····return·flag.toLowerCase()·===·'true'; with ··return·flag.toLowerCase()·===·'true'


[failure] 2-2:
Replace ··const·flag·=·process.env.ENABLE_CUSTOM_DOCUMENT_LOADER·??·'false'; with const·flag·=·process.env.ENABLE_CUSTOM_DOCUMENT_LOADER·??·'false'

src/cliAgent.ts

[failure] 58-58:
./utils/customDocumentLoader import should occur before import of ./utils/helpers


[failure] 57-57:
./utils/config import should occur before import of ./utils/helpers


[failure] 190-190:
Insert ····


[failure] 189-189:
Replace ······documentLoader:·CustomDocumentLoader with ··········documentLoader:·CustomDocumentLoader,


[failure] 188-188:
Replace ·?·new·W3cCredentialsModule() with ⏎······?·new·W3cCredentialsModule()⏎·····


[failure] 389-389:
Insert ··


[failure] 388-388:
Insert ··

Comment thread src/cliAgent.ts Outdated
Comment thread src/cliAgent.ts Outdated
Comment thread src/utils/config.ts Outdated
Comment thread src/utils/customDocumentLoader.ts Outdated
Comment thread src/utils/customDocumentLoader.ts Outdated
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (5)
src/cliAgent.ts (2)

57-58: Fix import order per ESLint rules.

The imports from ./utils/config and ./utils/customDocumentLoader must appear before ./utils/helpers to maintain alphabetical ordering and pass CI checks.

Apply this diff to fix import order:

 import { IndicioAcceptanceMechanism, IndicioTransactionAuthorAgreement, Network, NetworkName } from './enums'
 import { setupServer } from './server'
+import { IsCustomDocumentLoaderEnabled } from './utils/config'
+import { CustomDocumentLoader } from './utils/customDocumentLoader'
 import { generateSecretKey } from './utils/helpers'
 import { TsLogger } from './utils/logger'
-import { IsCustomDocumentLoaderEnabled } from './utils/config'
-import { CustomDocumentLoader } from './utils/customDocumentLoader'

188-192: Logic inversion: Flag behavior contradicts its name.

When ENABLE_CUSTOM_DOCUMENT_LOADER=true, the code uses the default loader (no custom loader). When false, it uses CustomDocumentLoader. This is backwards.

Expected behavior: ENABLE_CUSTOM_DOCUMENT_LOADER=true should enable the custom loader, not disable it.

Apply this diff to fix the logic inversion and formatting:

-    w3cCredentials: IsCustomDocumentLoaderEnabled()
-      ? new W3cCredentialsModule()
-      : new W3cCredentialsModule({
-          documentLoader: CustomDocumentLoader,
-        }),
+    w3cCredentials: IsCustomDocumentLoaderEnabled()
+      ? new W3cCredentialsModule({
+          documentLoader: CustomDocumentLoader,
+        })
+      : new W3cCredentialsModule(),
src/utils/customDocumentLoader.ts (3)

8-13: Add validation for required environment variables.

Line 12 uses a non-null assertion (!) on process.env.DEPRECATED_DOMAIN without validating it's set. If this environment variable is undefined, it will cause runtime errors.

Add validation at the top of the file:

+const DEPRECATED_DOMAIN = process.env.DEPRECATED_DOMAIN
+const CURRENT_DOMAIN = process.env.CURRENT_DOMAIN
+
+if (!DEPRECATED_DOMAIN || !CURRENT_DOMAIN) {
+  throw new Error(
+    'DEPRECATED_DOMAIN and CURRENT_DOMAIN environment variables must be set when using CustomDocumentLoader'
+  )
+}
+
 /**
  * Check if URL belongs to CREDEBL schema domain
  */
 function isW3CDeprecatedCredeblSchema(url: string, agentContext: AgentContext): boolean {
   agentContext.config.logger.debug(
     `Checking if w3c url(${url}) contains deprecated domain for agent: ${agentContext.config.label}`,
   )
-  return url.startsWith(process.env.DEPRECATED_DOMAIN!)
+  return url.startsWith(DEPRECATED_DOMAIN)
 }

1-3: Use import type for type-only imports.

AgentContext, DocumentLoader, and DocumentLoaderResult are only used as types. Use import type syntax to improve build optimization and clarify intent.

Apply this diff:

-import { CredoError, AgentContext, DocumentLoader } from '@credo-ts/core'
-import { DocumentLoaderResult } from '@credo-ts/core/build/modules/vc/data-integrity/jsonldUtil'
+import { CredoError } from '@credo-ts/core'
+import type { AgentContext, DocumentLoader } from '@credo-ts/core'
+import type { DocumentLoaderResult } from '@credo-ts/core/build/modules/vc/data-integrity/jsonldUtil'
 import { defaultDocumentLoader } from '@credo-ts/core/build/modules/vc/data-integrity/libraries/documentLoader'

18-23: Use validated constants instead of non-null assertions.

Lines 20 uses non-null assertions on DEPRECATED_DOMAIN and CURRENT_DOMAIN. After adding validation at the file top (see previous comment), use the validated constants.

Update the function to use validated constants:

 function resolvableCredeblSchemaUrl(url: string, agent: AgentContext): string {
   agent.config.logger.debug(`Replacing deprecated domain with updated domain`)
-  const updatedUrl = url.replace(process.env.DEPRECATED_DOMAIN!, process.env.CURRENT_DOMAIN!)
-
-  return updatedUrl
+  return url.replace(DEPRECATED_DOMAIN, CURRENT_DOMAIN)
 }
🧹 Nitpick comments (1)
src/utils/customDocumentLoader.ts (1)

43-45: Preserve original error details in error wrapping.

The error wrapping loses the original error's stack trace and details, making debugging harder.

Apply this diff to preserve error context:

     } catch (error) {
-      throw new CredoError(`Failed to load document for ${url}: ${error}`)
+      const errorMessage = error instanceof Error ? error.message : String(error)
+      throw new CredoError(`Failed to load document for ${url}: ${errorMessage}`, { cause: error })
     }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b9e29d7 and b597c95.

📒 Files selected for processing (3)
  • src/cliAgent.ts (2 hunks)
  • src/utils/config.ts (1 hunks)
  • src/utils/customDocumentLoader.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/cliAgent.ts (2)
src/utils/config.ts (1)
  • IsCustomDocumentLoaderEnabled (1-4)
src/utils/customDocumentLoader.ts (1)
  • CustomDocumentLoader (28-47)
🪛 ESLint
src/utils/customDocumentLoader.ts

[error] 1-1: Imports "AgentContext" and "DocumentLoader" are only used as type.

(@typescript-eslint/consistent-type-imports)


[error] 2-2: All imports in the declaration are only used as types. Use import type.

(@typescript-eslint/consistent-type-imports)

src/cliAgent.ts

[error] 57-57: ./utils/config import should occur before import of ./utils/helpers

(import/order)


[error] 58-58: ./utils/customDocumentLoader import should occur before import of ./utils/helpers

(import/order)

🪛 GitHub Actions: Continuous Integration
src/cliAgent.ts

[error] 57-57: ESLint: Import order violation. './utils/config' import should occur before import of './utils/helpers'. (import/order)

🪛 GitHub Check: Validate
src/utils/customDocumentLoader.ts

[failure] 2-2:
All imports in the declaration are only used as types. Use import type


[failure] 1-1:
Imports "AgentContext" and "DocumentLoader" are only used as type

src/cliAgent.ts

[failure] 58-58:
./utils/customDocumentLoader import should occur before import of ./utils/helpers


[failure] 57-57:
./utils/config import should occur before import of ./utils/helpers

🔇 Additional comments (1)
src/utils/config.ts (1)

1-4: LGTM!

The function correctly reads the environment variable, provides a safe default, and normalizes the value before comparison.

…itiation

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
src/utils/config.ts (1)

5-14: Refactor to separate validation from flag retrieval.

This function performs side effects (console logging) every time it's called. If invoked multiple times during the application lifecycle, it will log duplicate warnings and errors. Consider refactoring to:

  1. Call validation once at startup
  2. Separate the flag retrieval from validation logic

Apply this diff to separate concerns:

+// Validate configuration once at module load time
+const validateCustomLoaderConfig = (): void => {
+  const flag = process.env.ENABLE_CUSTOM_DOCUMENT_LOADER ?? 'false'
+  const isEnabled = flag.toLowerCase() === 'true'
+  
+  if (isEnabled) {
+    if (!process.env.DEPRECATED_DOMAIN || !process.env.CURRENT_DOMAIN) {
+      console.error(
+        `Custom document loader for the agent is enabled but the deprecated domain and updated domain is not set`,
+      )
+    }
+    console.warn(
+      `Custom document loader for the agent is enabled. Resolution of all URLs from the deprecated domain(${process.env.DEPRECATED_DOMAIN}) will actually be resolved from the current, updated domain(${process.env.CURRENT_DOMAIN})`,
+    )
+  }
+}
+
+// Run validation once
+validateCustomLoaderConfig()
+
 export const IsCustomDocumentLoaderEnabled = (): boolean => {
   const flag = process.env.ENABLE_CUSTOM_DOCUMENT_LOADER ?? 'false'
-  const isCustomDocumentLoaderEnabled = flag.toLowerCase() === 'true'
-
-  if (isCustomDocumentLoaderEnabled) {
-    if (!process.env.DEPRECATED_DOMAIN || !process.env.CURRENT_DOMAIN) {
-      console.error(
-        `Custom document loader for the agent is enabled but the deprecated domain and updated domain is not set`,
-      )
-    }
-    console.warn(
-      `Custom document loader for the agent is enabled. Resolution of all URLs from the deprecated domain(${process.env.DEPRECATED_DOMAIN}) will actually be resolved from the current, updated domain(${process.env.CURRENT_DOMAIN})`,
-    )
-  }
-
-  return isCustomDocumentLoaderEnabled
+  return flag.toLowerCase() === 'true'
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b597c95 and 5b1ceb7.

📒 Files selected for processing (1)
  • src/utils/config.ts (1 hunks)
🧰 Additional context used
🪛 ESLint
src/utils/config.ts

[error] 7-7: Unexpected console statement.

(no-console)


[error] 11-11: Unexpected console statement.

(no-console)

🪛 GitHub Check: Validate
src/utils/config.ts

[failure] 11-11:
Unexpected console statement


[failure] 7-7:
Unexpected console statement

Comment thread src/utils/config.ts Outdated
Comment thread src/utils/config.ts
Comment thread src/utils/config.ts Outdated
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/utils/config.ts (2)

1-3: Consider using camelCase for function names.

The function uses PascalCase naming (IsCustomDocumentLoaderEnabled), which is unconventional for TypeScript/JavaScript functions. Typically, functions use camelCase (e.g., isCustomDocumentLoaderEnabled). PascalCase is usually reserved for classes and type names.

However, if this follows an established project convention, you can safely ignore this suggestion.


11-13: Consider specifying which domain environment variables are missing.

The error message correctly indicates misconfiguration but could be more actionable by specifying which of DEPRECATED_DOMAIN or CURRENT_DOMAIN is undefined.

Apply this diff to improve error clarity:

-      throw new Error(
-        `Custom document loader for the agent is enabled but the deprecated domain and updated domain is not set`,
-      )
+      const missingVars = []
+      if (!process.env.DEPRECATED_DOMAIN) missingVars.push('DEPRECATED_DOMAIN')
+      if (!process.env.CURRENT_DOMAIN) missingVars.push('CURRENT_DOMAIN')
+      throw new Error(
+        `Custom document loader is enabled but required environment variable(s) are missing: ${missingVars.join(', ')}`
+      )
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a8e27b4 and f271bb4.

📒 Files selected for processing (1)
  • src/utils/config.ts (1 hunks)
🧰 Additional context used
🪛 ESLint
src/utils/config.ts

[error] 7-7: Unexpected console statement.

(no-console)


[error] 8-8: Unexpected console statement.

(no-console)


[error] 15-15: Unexpected console statement.

(no-console)

🪛 GitHub Actions: Continuous Integration
src/utils/config.ts

[error] 7-7: Unexpected console statement. (no-console)

🪛 GitHub Check: Validate
src/utils/config.ts

[failure] 15-15:
Unexpected console statement


[failure] 8-8:
Unexpected console statement


[failure] 7-7:
Unexpected console statement

🔇 Additional comments (1)
src/utils/config.ts (1)

20-21: LGTM!

The return statement correctly returns the validated boolean flag.

Comment thread src/utils/config.ts
Comment thread src/utils/config.ts
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
src/utils/customDocumentLoader.ts (2)

14-14: Non-null assertion still unsafe despite validation added.

From the previous review discussion, validation was added in config.ts but it only logs console.error without throwing. This means the agent can still start when DEPRECATED_DOMAIN is missing, causing a runtime crash here during document loading instead of failing fast at startup.

As previously suggested, either:

  • Option 1: Make config.ts throw instead of just logging, or
  • Option 2: Validate and cache the env vars at module load:
+const DEPRECATED_DOMAIN = process.env.DEPRECATED_DOMAIN
+const CURRENT_DOMAIN = process.env.CURRENT_DOMAIN
+
+if (!DEPRECATED_DOMAIN || !CURRENT_DOMAIN) {
+  throw new Error(
+    'DEPRECATED_DOMAIN and CURRENT_DOMAIN must be set when using CustomDocumentLoader'
+  )
+}
+
 function isW3CDeprecatedCredeblSchema(url: string, agentContext: AgentContext): boolean {
   agentContext.config.logger.debug(
     `Checking if w3c url(${url}) contains deprecated domain for agent: ${agentContext.config.label}`,
   )
-  return url.startsWith(process.env.DEPRECATED_DOMAIN!)
+  return url.startsWith(DEPRECATED_DOMAIN)
 }

22-22: Same non-null assertion issue.

This line has the same problem as line 14. If you adopt the module-level validation approach, update this line too:

 function resolvableCredeblSchemaUrl(url: string, agent: AgentContext): string {
   agent.config.logger.debug(`Replacing deprecated domain with updated domain`)
-  const updatedUrl = url.replace(process.env.DEPRECATED_DOMAIN!, process.env.CURRENT_DOMAIN!)
+  const updatedUrl = url.replace(DEPRECATED_DOMAIN, CURRENT_DOMAIN)
 
   return updatedUrl
 }
🧹 Nitpick comments (1)
src/utils/customDocumentLoader.ts (1)

45-47: Preserve error stack trace in error wrapping.

String interpolating ${error} (line 46) loses the original stack trace and error details. Check if CredoError supports a cause parameter to preserve the full error context.

If CredoError supports it, apply:

     } catch (error) {
-      throw new CredoError(`Failed to load document for ${url}: ${error}`)
+      throw new CredoError(`Failed to load document for ${url}`, { cause: error })
     }

Otherwise, at minimum log the full error before throwing:

     } catch (error) {
+      agentContext.config.logger.error(`Document loading failed for ${url}`, { error })
       throw new CredoError(`Failed to load document for ${url}: ${error}`)
     }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f271bb4 and 586dfe3.

📒 Files selected for processing (2)
  • src/utils/config.ts (1 hunks)
  • src/utils/customDocumentLoader.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/utils/config.ts
🧰 Additional context used
🪛 ESLint
src/utils/customDocumentLoader.ts

[error] 4-4: All imports in the declaration are only used as types. Use import type.

(@typescript-eslint/consistent-type-imports)

🪛 GitHub Actions: Continuous Integration
src/utils/customDocumentLoader.ts

[error] 4-4: ESLint: All imports in the declaration are only used as types. Use 'import type'. (consistent-type-imports)

🪛 GitHub Check: Validate
src/utils/customDocumentLoader.ts

[failure] 4-4:
All imports in the declaration are only used as types. Use import type

Comment thread src/utils/customDocumentLoader.ts Outdated
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/utils/customDocumentLoader.ts (1)

14-14: Non-null assertion creates runtime crash risk.

Despite previous review feedback, the non-null assertion on process.env.DEPRECATED_DOMAIN! remains. Even if validation exists in config.ts, if it only logs an error without throwing, the code will crash here when the environment variable is undefined.

Move the validation to module scope and use validated constants:

+const DEPRECATED_DOMAIN = process.env.DEPRECATED_DOMAIN
+const CURRENT_DOMAIN = process.env.CURRENT_DOMAIN
+
+if (!DEPRECATED_DOMAIN || !CURRENT_DOMAIN) {
+  throw new Error(
+    'DEPRECATED_DOMAIN and CURRENT_DOMAIN must be set when using CustomDocumentLoader'
+  )
+}
+
 function isW3CDeprecatedCredeblSchema(url: string, agentContext: AgentContext): boolean {
   agentContext.config.logger.debug(
     `Checking if w3c url(${url}) contains deprecated domain for agent: ${agentContext.config.label}`,
   )
-  return url.startsWith(process.env.DEPRECATED_DOMAIN!)
+  return url.startsWith(DEPRECATED_DOMAIN)
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 586dfe3 and 1bd804a.

📒 Files selected for processing (1)
  • src/utils/customDocumentLoader.ts (1 hunks)
🔇 Additional comments (1)
src/utils/customDocumentLoader.ts (1)

1-5: LGTM! Imports are correctly structured.

All type-only imports properly use import type, and value imports use regular import syntax. The past review concerns about type imports have been addressed.

Comment thread src/utils/customDocumentLoader.ts Outdated
Comment thread src/utils/customDocumentLoader.ts
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Comment thread src/utils/config.ts Outdated
Comment thread src/utils/customDocumentLoader.ts Outdated
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
@sonarqubecloud
Copy link
Copy Markdown

@GHkrishna GHkrishna requested a review from sairanjit October 16, 2025 13:50
@GHkrishna GHkrishna merged commit dfefc8e into main Oct 16, 2025
9 checks passed
@GHkrishna GHkrishna deleted the feat/agent-custom-document-loader branch October 16, 2025 13:52
tipusinghaw pushed a commit that referenced this pull request Apr 28, 2026
* feat: add agent custom document loader

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: formatting

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: formatting

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* feat: add appropriate warning for incorrect custom document loader initiation

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: formatting

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: formatting

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: error handling

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: formatting

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: formatting

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: coderabbit suggestions

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: reviewer suggestions

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

---------

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Defines a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants