Skip to content

feat: Enhance logging in adapter and app registries with verbose messages for initialization and registration processes#250

Merged
frontegg-david merged 7 commits intomainfrom
add-logs
Feb 22, 2026
Merged

feat: Enhance logging in adapter and app registries with verbose messages for initialization and registration processes#250
frontegg-david merged 7 commits intomainfrom
add-logs

Conversation

@frontegg-david
Copy link
Contributor

@frontegg-david frontegg-david commented Feb 21, 2026

Summary by CodeRabbit

  • New Features

    • Exposed plugin-name accessor in registries.
  • Improvements

    • Widespread structured, scoped logging and a concise scope readiness summary across the SDK.
    • Added timing/duration reporting for many request handlers and richer session/auth observability.
    • Finalization logs now report content part counts and total content bytes.
    • Adjusted log levels for clearer operational signals.
    • Improved JWT detection and stricter bearer token extraction.
  • Tests

    • Updated tests to use a mock logger for logging assertions.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 21, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds optional FrontMcpLogger instrumentation and timing across registries, instances, flows, transports, MCP handlers, and plugins; replaces some console logs; introduces PluginRegistry.getPluginNames and EmptyPluginRegistry.getPluginNames; tightens bearer token extraction and improves JWT leakage detection.

Changes

Cohort / File(s) Summary
Adapter Registry
libs/sdk/src/adapter/adapter.registry.ts, libs/sdk/src/adapter/__tests__/adapter.registry.test.ts
Switched adapter init/completion logs to verbose; test updated to assert verbose-level log.
App Registry & Instances
libs/sdk/src/app/app.registry.ts, libs/sdk/src/app/instances/app.local.instance.ts, libs/sdk/src/app/instances/app.remote.instance.ts
Injected optional logger usage and per-app initialization logs; AppLocalInstance gains verbose lifecycle logs; EmptyPluginRegistry adds getPluginNames(): string[].
Plugin Registry & Interface
libs/sdk/src/plugin/plugin.registry.ts, libs/sdk/src/common/interfaces/internal/registry.interface.ts
Added optional child logger and init logs; new public getPluginNames(): string[] on the plugin registry interface/implementation.
Scope & ScopeRegistry
libs/sdk/src/scope/scope.instance.ts, libs/sdk/src/scope/scope.registry.ts
Extensive scoped logging added across registry initializations, event/elicitation stores, skill/session manager, UI precompilation, and a consolidated "scope ready" summary.
Flow Instance & Auth Flow
libs/sdk/src/flows/flow.instance.ts, libs/sdk/src/auth/flows/session.verify.flow.ts
FlowInstance now uses FrontMcpLogger (added member); SessionVerifyFlow adds logger, maskSub helper, and multiple verbose/info/warn logs.
FrontMcp Core
libs/sdk/src/front-mcp/front-mcp.ts
Added optional logger member and lifecycle logs (bootstrap/start/handler/server creation); replaced console.log with logger.info where applicable.
Transport Flows
libs/sdk/src/transport/flows/handle.stateless-http.flow.ts, libs/sdk/src/transport/flows/handle.streamable-http.flow.ts
Per-stage scoped logging, increased verbosity, added try/catch around transport invocations to log and rethrow errors.
MCP Handlers (timing & levels)
libs/sdk/src/transport/mcp-handlers/*.handler.ts
Added timing (start/duration) and completion logs across many handlers; adjusted log levels and request wording; preserved control flow.
Agent / Tool Flows
libs/sdk/src/agent/flows/call-agent.flow.ts, libs/sdk/src/tool/flows/call-tool.flow.ts
Finalization logs now report contentParts and new contentBytes (sum of JSON byte lengths) instead of contentLength; no behavioral changes.
Plugin: CodeCall
plugins/plugin-codecall/src/codecall.plugin.ts, plugins/plugin-codecall/src/__tests__/codecall.plugin.test.ts
Replaced console logs with FrontMcpLogger; tests reworked to inject/mock logger and assert logger calls.
Auth & Token Utils
libs/auth/src/authorization/authorization.class.ts, libs/auth/src/session/utils/auth-token.utils.ts, libs/auth/src/authorization/__tests__/authorization.class.test.ts
Replaced regex JWT leak check with segment-based detection using base64urlDecode; tightened Bearer token extraction regex (no trimming); added base64urlDecode mock in tests and expanded test cases.
Scope Registry Init
libs/sdk/src/scope/scope.registry.ts
Added scoped logger member and verbose per-scope initialization logs and completion summary.
CI Workflow
.github/workflows/push.yml
Added timeout-minutes to Unit Tests and E2E Tests jobs.

Sequence Diagram(s)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐇
I hopped through modules with a tiny log in tow,
I whispered verbose secrets where the handlers go,
Timings tick like carrots, plugins hum their names,
Sessions masked and tidy in my burrowed frames,
Debug carrots gone — verbose crumbs lead the show.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 65.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the primary objective: enhancing logging with verbose messages for initialization and registration processes across adapter and app registries, which aligns with the majority of changes throughout the codebase.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-logs

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

Copy link
Contributor

@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

🧹 Nitpick comments (2)
libs/sdk/src/scope/scope.instance.ts (1)

426-440: Extract the scope-ready summary into a helper (and add a no-entries fallback).
Keeps scope.instance.ts lean and avoids emitting an empty summary string.

♻️ Suggested refactor (summary callsite)
-    const counts: string[] = [];
-    const appsCount = this.scopeApps.getApps().length;
-    const toolsCount = this.scopeTools.getTools(true).length;
-    const resourcesCount = this.scopeResources.getResources().length;
-    const promptsCount = this.scopePrompts.getPrompts().length;
-    const agentsCount = this.scopeAgents.getAgents().length;
-    const skillsCount = this.scopeSkills.getSkills().length;
-    if (appsCount > 0) counts.push(`${appsCount} app${appsCount !== 1 ? 's' : ''}`);
-    if (toolsCount > 0) counts.push(`${toolsCount} tool${toolsCount !== 1 ? 's' : ''}`);
-    if (resourcesCount > 0) counts.push(`${resourcesCount} resource${resourcesCount !== 1 ? 's' : ''}`);
-    if (promptsCount > 0) counts.push(`${promptsCount} prompt${promptsCount !== 1 ? 's' : ''}`);
-    if (agentsCount > 0) counts.push(`${agentsCount} agent${agentsCount !== 1 ? 's' : ''}`);
-    if (skillsCount > 0) counts.push(`${skillsCount} skill${skillsCount !== 1 ? 's' : ''}`);
-    this.logger.info(`Scope ready — ${counts.join(', ')}`);
+    this.logScopeReadySummary();
private logScopeReadySummary(): void {
  const counts: string[] = [];
  const appsCount = this.scopeApps.getApps().length;
  const toolsCount = this.scopeTools.getTools(true).length;
  const resourcesCount = this.scopeResources.getResources().length;
  const promptsCount = this.scopePrompts.getPrompts().length;
  const agentsCount = this.scopeAgents.getAgents().length;
  const skillsCount = this.scopeSkills.getSkills().length;
  if (appsCount > 0) counts.push(`${appsCount} app${appsCount !== 1 ? 's' : ''}`);
  if (toolsCount > 0) counts.push(`${toolsCount} tool${toolsCount !== 1 ? 's' : ''}`);
  if (resourcesCount > 0) counts.push(`${resourcesCount} resource${resourcesCount !== 1 ? 's' : ''}`);
  if (promptsCount > 0) counts.push(`${promptsCount} prompt${promptsCount !== 1 ? 's' : ''}`);
  if (agentsCount > 0) counts.push(`${agentsCount} agent${agentsCount !== 1 ? 's' : ''}`);
  if (skillsCount > 0) counts.push(`${skillsCount} skill${skillsCount !== 1 ? 's' : ''}`);
  const summary = counts.length ? counts.join(', ') : 'no entries';
  this.logger.info(`Scope ready — ${summary}`);
}

As per coding guidelines, "Keep scope.instance.ts lean by using helper functions for feature-specific registration logic".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/sdk/src/scope/scope.instance.ts` around lines 426 - 440, Extract the
inline summary-building block in scope.instance.ts into a private helper method
named logScopeReadySummary (or similar) that computes
appsCount/toolsCount/resourcesCount/promptsCount/agentsCount/skillsCount from
this.scopeApps, this.scopeTools, this.scopeResources, this.scopePrompts,
this.scopeAgents, this.scopeSkills, builds pluralized entries only for non-zero
counts, and sets summary to counts.join(', ') or a fallback 'no entries' when
empty; then replace the original block with a call to
this.logScopeReadySummary() so the file is lean and no empty summary is emitted.
libs/sdk/src/app/instances/app.local.instance.ts (1)

65-103: Avoid building log payloads when the logger is absent.
The optional logger means arrays/strings are built even if nothing will be logged. Consider guarding the heavier formatting (tool lists + summary) behind a logger check.

♻️ Suggested tweak to guard log payload construction
-    const toolNames = this.appTools.getTools(true).map((t) => t.metadata.name);
-    this.logger?.verbose(
-      `App ${this.metadata.name}: ${toolNames.length} tool(s) registered: [${toolNames.join(', ')}]`,
-    );
+    if (this.logger) {
+      const toolNames = this.appTools.getTools(true).map((t) => t.metadata.name);
+      this.logger.verbose(
+        `App ${this.metadata.name}: ${toolNames.length} tool(s) registered: [${toolNames.join(', ')}]`,
+      );
+    }
@@
-    const parts: string[] = [];
-    const toolCount = this.appTools.getTools(true).length;
-    const resourceCount = this.appResources.getResources().length;
-    const promptCount = this.appPrompts.getPrompts().length;
-    const adapterCount = this.appAdapters.getAdapters().length;
-    const pluginCount = this.appPlugins.getPlugins().length;
-    if (toolCount > 0) parts.push(`${toolCount} tool${toolCount !== 1 ? 's' : ''}`);
-    if (resourceCount > 0) parts.push(`${resourceCount} resource${resourceCount !== 1 ? 's' : ''}`);
-    if (promptCount > 0) parts.push(`${promptCount} prompt${promptCount !== 1 ? 's' : ''}`);
-    if (adapterCount > 0) parts.push(`${adapterCount} adapter${adapterCount !== 1 ? 's' : ''}`);
-    if (pluginCount > 0) parts.push(`${pluginCount} plugin${pluginCount !== 1 ? 's' : ''}`);
-    this.logger?.info(`${this.metadata.name}: ${parts.length > 0 ? parts.join(', ') : 'no entries'}`);
+    if (this.logger) {
+      const parts: string[] = [];
+      const toolCount = this.appTools.getTools(true).length;
+      const resourceCount = this.appResources.getResources().length;
+      const promptCount = this.appPrompts.getPrompts().length;
+      const adapterCount = this.appAdapters.getAdapters().length;
+      const pluginCount = this.appPlugins.getPlugins().length;
+      if (toolCount > 0) parts.push(`${toolCount} tool${toolCount !== 1 ? 's' : ''}`);
+      if (resourceCount > 0) parts.push(`${resourceCount} resource${resourceCount !== 1 ? 's' : ''}`);
+      if (promptCount > 0) parts.push(`${promptCount} prompt${promptCount !== 1 ? 's' : ''}`);
+      if (adapterCount > 0) parts.push(`${adapterCount} adapter${adapterCount !== 1 ? 's' : ''}`);
+      if (pluginCount > 0) parts.push(`${pluginCount} plugin${pluginCount !== 1 ? 's' : ''}`);
+      this.logger.info(`${this.metadata.name}: ${parts.length > 0 ? parts.join(', ') : 'no entries'}`);
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/sdk/src/app/instances/app.local.instance.ts` around lines 65 - 103, The
code builds lists and computes counts (toolNames, toolCount, resourceCount,
promptCount, adapterCount, pluginCount and the parts array) even when
this.logger is undefined; wrap the heavy log payload construction and related
calls to this.appTools.getTools(true), this.appResources.getResources(),
this.appPrompts.getPrompts(), this.appAdapters.getAdapters(), and
this.appPlugins.getPlugins() behind a guard like if (this.logger) so you only
map/join and compose the summary string when a logger exists, and otherwise skip
that work and avoid allocating arrays/strings while preserving earlier registry
initialization (leave await this.appTools.ready etc. as-is).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@libs/sdk/src/auth/flows/session.verify.flow.ts`:
- Around line 414-418: The current logger.info call in the SessionVerifyFlow
logs user.sub directly; instead compute a masked identifier (e.g., mask all but
last 4 characters or truncate and append "...") into a variable like maskedSub
and use that in the logger payload (keep hasSession as-is). Locate the logger
created via this.scopeLogger.child('SessionVerifyFlow') and update the
logger.info call to send maskedSub instead of user.sub so full user identifiers
are not written to logs.

---

Nitpick comments:
In `@libs/sdk/src/app/instances/app.local.instance.ts`:
- Around line 65-103: The code builds lists and computes counts (toolNames,
toolCount, resourceCount, promptCount, adapterCount, pluginCount and the parts
array) even when this.logger is undefined; wrap the heavy log payload
construction and related calls to this.appTools.getTools(true),
this.appResources.getResources(), this.appPrompts.getPrompts(),
this.appAdapters.getAdapters(), and this.appPlugins.getPlugins() behind a guard
like if (this.logger) so you only map/join and compose the summary string when a
logger exists, and otherwise skip that work and avoid allocating arrays/strings
while preserving earlier registry initialization (leave await
this.appTools.ready etc. as-is).

In `@libs/sdk/src/scope/scope.instance.ts`:
- Around line 426-440: Extract the inline summary-building block in
scope.instance.ts into a private helper method named logScopeReadySummary (or
similar) that computes
appsCount/toolsCount/resourcesCount/promptsCount/agentsCount/skillsCount from
this.scopeApps, this.scopeTools, this.scopeResources, this.scopePrompts,
this.scopeAgents, this.scopeSkills, builds pluralized entries only for non-zero
counts, and sets summary to counts.join(', ') or a fallback 'no entries' when
empty; then replace the original block with a call to
this.logScopeReadySummary() so the file is lean and no empty summary is emitted.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 21, 2026

Performance Test Results

Status: ✅ All tests passed

Summary

Project Tests Passed Warnings Failed Leaks
✅ demo-e2e-agents 4 4 0 0 0
✅ demo-e2e-cache 11 11 0 0 0
✅ demo-e2e-codecall 4 4 0 0 0
✅ demo-e2e-config 4 4 0 0 0
✅ demo-e2e-direct 3 3 0 0 0
✅ demo-e2e-elicitation 1 1 0 0 0
✅ demo-e2e-errors 4 4 0 0 0
✅ demo-e2e-hooks 3 3 0 0 0
✅ demo-e2e-multiapp 4 4 0 0 0
✅ demo-e2e-notifications 3 3 0 0 0
✅ demo-e2e-openapi 2 2 0 0 0
✅ demo-e2e-providers 4 4 0 0 0
✅ demo-e2e-public 4 4 0 0 0
✅ demo-e2e-redis 14 14 0 0 0
✅ demo-e2e-remember 4 4 0 0 0
✅ demo-e2e-remote 5 5 0 0 0
✅ demo-e2e-serverless 2 2 0 0 0
✅ demo-e2e-skills 15 15 0 0 0
✅ demo-e2e-standalone 2 2 0 0 0
✅ demo-e2e-transport-recreation 3 3 0 0 0
✅ demo-e2e-ui 4 4 0 0 0

Total: 100 tests across 21 projects

📊 View full report in workflow run


Generated at: 2026-02-22T12:59:57.826Z
Commit: 39049d9f

Copy link
Contributor

@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 (4)
libs/sdk/src/tool/flows/call-tool.flow.ts (1)

872-875: contentBytes is not actually bytes

JSON.stringify(part).length counts UTF‑16 code units, not bytes, so the metric name is misleading (especially for non‑ASCII content). Prefer Buffer.byteLength or rename the metric to reflect character length.

💡 Suggested fix
-      contentBytes: Array.isArray(result.content)
-        ? result.content.reduce((sum, part) => sum + JSON.stringify(part).length, 0)
-        : 0,
+      contentBytes: Array.isArray(result.content)
+        ? result.content.reduce((sum, part) => sum + Buffer.byteLength(JSON.stringify(part), 'utf8'), 0)
+        : 0,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/sdk/src/tool/flows/call-tool.flow.ts` around lines 872 - 875, The metric
contentBytes in call-tool.flow.ts is computed using JSON.stringify(part).length
which counts UTF‑16 code units, not actual bytes; update the calculation in the
block that defines contentParts and contentBytes (referencing the contentParts
and contentBytes variables) to use a byte-aware method (e.g.,
Buffer.byteLength(JSON.stringify(part), 'utf8')) for each part, summing those
values, or alternatively rename contentBytes to contentChars if you intend to
keep the character-length semantics; ensure the change is applied to the ternary
branch that handles Array.isArray(result.content).
.github/workflows/push.yml (1)

165-165: Verify the 10‑minute cap won’t cause flaky timeouts (especially E2E).

Please confirm recent run durations and set timeouts with enough headroom (often E2E > unit). Consider separate caps (e.g., unit shorter, E2E longer) or making the values easy to tune.

Also applies to: 217-217

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/push.yml at line 165, Check recent CI run durations and
ensure the global timeout-minutes: 10 in the workflow won't cause flaky E2E
timeouts; update the workflow to apply per-job timeouts (use the YAML key
timeout-minutes under specific jobs like e2e and unit rather than a single
global cap), increase the E2E timeout to a safe headroom above observed max runs
(e.g., 2x or +X minutes), and/or expose the timeout as a configurable workflow
input or environment variable so it’s easy to tune; look for the timeout-minutes
entry and the job names (e.g., e2e, unit, integration) and adjust those entries
accordingly.
libs/sdk/src/agent/flows/call-agent.flow.ts (1)

485-487: contentBytes name is misleading; this measures character count, not bytes.

JSON.stringify(part).length returns the JavaScript string length (UTF-16 code units), not the byte count. For ASCII content they're roughly equivalent, but for non-ASCII characters (emoji, CJK, accented characters), the actual UTF-8 byte size can be 2–4× larger.

If this is intended as a rough estimate for logging, consider renaming to contentChars or estimatedContentLength. If actual byte measurement is needed for metrics/quotas, use Buffer.byteLength() or TextEncoder:

Option 1: Rename to reflect actual semantics
-      contentBytes: Array.isArray(result.content)
-        ? result.content.reduce((sum, part) => sum + JSON.stringify(part).length, 0)
+      contentChars: Array.isArray(result.content)
+        ? result.content.reduce((sum, part) => sum + JSON.stringify(part).length, 0)
Option 2: Calculate actual bytes (if needed)
-      contentBytes: Array.isArray(result.content)
-        ? result.content.reduce((sum, part) => sum + JSON.stringify(part).length, 0)
+      contentBytes: Array.isArray(result.content)
+        ? result.content.reduce((sum, part) => sum + Buffer.byteLength(JSON.stringify(part), 'utf8'), 0)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/sdk/src/agent/flows/call-agent.flow.ts` around lines 485 - 487, The
property contentBytes on the object is misleading because it currently counts
UTF-16 string length via JSON.stringify(part).length; update the logic around
result.content (used in the contentBytes calculation) to either (a) rename the
property to contentChars or estimatedContentLength if you want to keep the
character-count semantics, or (b) compute actual UTF-8 byte length for metrics
by replacing JSON.stringify(part).length with a UTF-8 byte calculation (e.g.,
Buffer.byteLength(JSON.stringify(part), 'utf8') in Node, with a TextEncoder
fallback for browser environments) so contentBytes truly reflects bytes.
libs/sdk/src/auth/flows/session.verify.flow.ts (1)

110-111: Consider extracting the child logger to a class property.

The same child logger this.scopeLogger.child('SessionVerifyFlow') is instantiated in 8 different methods throughout this class. Creating it once as a private property would reduce redundancy and minor overhead.

♻️ Suggested refactor

Add a private getter or property for the logger:

 export default class SessionVerifyFlow extends FlowBase<typeof name> {
+  private get logger() {
+    return this.scopeLogger.child('SessionVerifyFlow');
+  }
+
   /**
    * Create an anonymous session with consistent structure...
    */
   private createAnonymousSession(options: AnonymousSessionOptions): void {
     const { authMode, issuer, scopes = ['anonymous'], sessionIdHeader } = options;
-    const logger = this.scopeLogger.child('SessionVerifyFlow');
-    logger.verbose('createAnonymousSession', { authMode, hasExistingSession: !!sessionIdHeader });
+    this.logger.verbose('createAnonymousSession', { authMode, hasExistingSession: !!sessionIdHeader });

Then replace all const logger = this.scopeLogger.child('SessionVerifyFlow'); calls with direct usage of this.logger.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/sdk/src/auth/flows/session.verify.flow.ts` around lines 110 - 111, The
repeated call to this.scopeLogger.child('SessionVerifyFlow') should be extracted
into a single class-level logger to avoid duplication; add a private property or
getter (e.g., private readonly logger =
this.scopeLogger.child('SessionVerifyFlow') or get logger() { return
this.scopeLogger.child('SessionVerifyFlow'); }) on the SessionVerifyFlow class,
then replace all occurrences of const logger =
this.scopeLogger.child('SessionVerifyFlow'); with direct usage of this.logger
and remove the duplicated local declarations across the methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@libs/sdk/src/scope/scope.instance.ts`:
- Around line 426-440: The current inline summary builder produces a trailing
dash when all counts are zero and should be extracted into a small helper;
implement a helper function (e.g., buildScopeSummary or formatScopeCounts) that
accepts the individual counts (appsCount, toolsCount, resourcesCount,
promptsCount, agentsCount, skillsCount) or the Scope accessors (this.scopeApps,
this.scopeTools, etc.), builds the concise non-zero list, and returns a fallback
string like "no items" (or "empty") when the list is empty; replace the inline
block that builds counts and calls this.logger.info(`Scope ready —
${counts.join(', ')}`) with a single call to this helper and log `Scope ready —
${summary}` to keep scope.instance.ts lean and avoid the trailing dash.

In `@plugins/plugin-codecall/src/__tests__/codecall.plugin.test.ts`:
- Around line 118-121: The test helper createPluginWithLogger currently uses an
unsafe type cast (p as any) to mock the plugin DI; replace this with a typed
approach: narrow p to unknown then assert the get method or use
jest.spyOn(CodeCallPlugin.prototype, 'get') with an appropriate generic/type
annotation so the mockReturnValue({ child: () => mockLogger }) is type-safe;
ensure references to CodeCallPlugin, createPluginWithLogger, get, mockLogger,
and jest.fn are preserved and that no `any` remains.

---

Duplicate comments:
In `@libs/sdk/src/auth/flows/session.verify.flow.ts`:
- Around line 428-432: The info log in SessionVerifyFlow uses logger (created
from this.scopeLogger.child('SessionVerifyFlow')) and currently logs the full
user.sub which may contain PII; change the log to mask or truncate user.sub
(e.g., show only first 6 and last 4 chars or replace all but last 4 with
asterisks) before passing it to logger.info and keep hasSession as-is so logs no
longer contain the raw identifier but still provide traceable context.

---

Nitpick comments:
In @.github/workflows/push.yml:
- Line 165: Check recent CI run durations and ensure the global timeout-minutes:
10 in the workflow won't cause flaky E2E timeouts; update the workflow to apply
per-job timeouts (use the YAML key timeout-minutes under specific jobs like e2e
and unit rather than a single global cap), increase the E2E timeout to a safe
headroom above observed max runs (e.g., 2x or +X minutes), and/or expose the
timeout as a configurable workflow input or environment variable so it’s easy to
tune; look for the timeout-minutes entry and the job names (e.g., e2e, unit,
integration) and adjust those entries accordingly.

In `@libs/sdk/src/agent/flows/call-agent.flow.ts`:
- Around line 485-487: The property contentBytes on the object is misleading
because it currently counts UTF-16 string length via
JSON.stringify(part).length; update the logic around result.content (used in the
contentBytes calculation) to either (a) rename the property to contentChars or
estimatedContentLength if you want to keep the character-count semantics, or (b)
compute actual UTF-8 byte length for metrics by replacing
JSON.stringify(part).length with a UTF-8 byte calculation (e.g.,
Buffer.byteLength(JSON.stringify(part), 'utf8') in Node, with a TextEncoder
fallback for browser environments) so contentBytes truly reflects bytes.

In `@libs/sdk/src/auth/flows/session.verify.flow.ts`:
- Around line 110-111: The repeated call to
this.scopeLogger.child('SessionVerifyFlow') should be extracted into a single
class-level logger to avoid duplication; add a private property or getter (e.g.,
private readonly logger = this.scopeLogger.child('SessionVerifyFlow') or get
logger() { return this.scopeLogger.child('SessionVerifyFlow'); }) on the
SessionVerifyFlow class, then replace all occurrences of const logger =
this.scopeLogger.child('SessionVerifyFlow'); with direct usage of this.logger
and remove the duplicated local declarations across the methods.

In `@libs/sdk/src/tool/flows/call-tool.flow.ts`:
- Around line 872-875: The metric contentBytes in call-tool.flow.ts is computed
using JSON.stringify(part).length which counts UTF‑16 code units, not actual
bytes; update the calculation in the block that defines contentParts and
contentBytes (referencing the contentParts and contentBytes variables) to use a
byte-aware method (e.g., Buffer.byteLength(JSON.stringify(part), 'utf8')) for
each part, summing those values, or alternatively rename contentBytes to
contentChars if you intend to keep the character-length semantics; ensure the
change is applied to the ternary branch that handles
Array.isArray(result.content).

Copy link
Contributor

@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 (1)
libs/sdk/src/scope/scope.instance.ts (1)

181-184: Consider clarifying that this is the initial tool set.

The log runs before system/skill tools are registered, so the list can be incomplete when elicitation or skills add tools later. A small wording tweak avoids confusion.

💡 Suggested tweak
-    this.logger.verbose(`ToolRegistry initialized (${toolNames.length} tool(s): [${toolNames.join(', ')}])`);
+    this.logger.verbose(`ToolRegistry initialized (initial ${toolNames.length} tool(s): [${toolNames.join(', ')}])`);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/sdk/src/scope/scope.instance.ts` around lines 181 - 184, The logger
message in ScopeInstance that prints toolNames (using this.scopeTools = new
ToolRegistry(...), await this.scopeTools.ready, this.scopeTools.getTools(true),
and this.logger.verbose) can be misleading because system/skill tools may be
registered later; change the verbose log to indicate this is the
initial/initially loaded tool set (e.g., "Initial ToolRegistry initialized" or
"ToolRegistry initialized (initial tool set ...)") so it clearly states the list
may be incomplete until later registrations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@libs/auth/src/authorization/authorization.class.ts`:
- Around line 244-266: validateNoTokenLeakage currently seeds detection from
json.indexOf('eyJ') which misses valid JWT payloads not starting with that
prefix; update the logic in validateNoTokenLeakage to stop requiring
startsWith('eyJ') and instead detect any three dot-separated base64url segments
(use the existing JWT_SEGMENT regex and the seg1/seg2/seg3 checks) — e.g., scan
for a '.' delimiter, extract the preceding seg1 and following seg2/seg3 ensuring
they match JWT_SEGMENT and have reasonable lengths, and throw
TokenLeakDetectedError when a three-segment base64url pattern is found; keep the
performance-minded index/substring approach but remove the hardcoded 'eyJ'
pre-check.

In `@libs/sdk/src/auth/flows/session.verify.flow.ts`:
- Around line 104-109: maskSub currently assumes sub is always defined and will
throw if user.sub is missing; update SessionVerifyFlow.maskSub to guard against
null/undefined (e.g., treat undefined as an empty string or return a safe
placeholder like '***' or 'unknown') so logging never crashes, and update any
other usages (notably the deriveTypedUser call sites and the other maskSub
occurrence around lines 427-430) to pass the possibly-undefined value safely;
ensure the method signature and callers handle optional strings so missing
user.sub is masked without throwing.

---

Nitpick comments:
In `@libs/sdk/src/scope/scope.instance.ts`:
- Around line 181-184: The logger message in ScopeInstance that prints toolNames
(using this.scopeTools = new ToolRegistry(...), await this.scopeTools.ready,
this.scopeTools.getTools(true), and this.logger.verbose) can be misleading
because system/skill tools may be registered later; change the verbose log to
indicate this is the initial/initially loaded tool set (e.g., "Initial
ToolRegistry initialized" or "ToolRegistry initialized (initial tool set ...)")
so it clearly states the list may be incomplete until later registrations.

Copy link
Contributor

@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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@libs/auth/src/authorization/authorization.class.ts`:
- Around line 244-284: The validateNoTokenLeakage function currently flags any
three-segment base64url-like string and causes false positives for
domains/namespaces; update the heuristic to, upon detecting a candidate triplet
(in validateNoTokenLeakage), base64url-decode the first two segments and
JSON.parse the header (segment1) to ensure it contains a JWT-like "alg" (and
optionally "typ") field (and optionally validate payload JSON) before throwing
TokenLeakDetectedError, and add unit tests asserting that inputs like { url:
"api.example.com" }, { domain: "service.example.org" }, and { namespace:
"com.example.app" } do NOT throw while valid JWT-like strings DO throw; use the
existing TokenLeakDetectedError for failures and keep the early index-based
scanning to locate candidates.

---

Duplicate comments:
In `@libs/sdk/src/auth/flows/session.verify.flow.ts`:
- Around line 428-431: The log call in session.verify.flow.ts currently avoids
leaking PII by using maskSub(user.sub) inside this.logger.info and should be
left as-is; ensure the masked identifier remains used anywhere else logging the
user (e.g., in SessionVerifyFlow or any methods that call this.logger.info) and
do not replace maskSub with the raw user.sub so PII remains redacted.

In `@libs/sdk/src/scope/scope.instance.ts`:
- Around line 425-441: No changes required: the formatScopeSummary() helper
correctly builds the summary and returns 'empty' when there are no entries;
leave the existing call to this.logger.info(`Scope ready —
${this.formatScopeSummary()}`) and retain the add helper and calls to
this.scopeApps.getApps(), this.scopeTools.getTools(true),
this.scopeResources.getResources(), this.scopePrompts.getPrompts(),
this.scopeAgents.getAgents(), and this.scopeSkills.getSkills() as-is.

@frontegg-david frontegg-david merged commit 051951d into main Feb 22, 2026
80 of 81 checks passed
@frontegg-david frontegg-david deleted the add-logs branch February 22, 2026 13:00
@coderabbitai coderabbitai bot mentioned this pull request Feb 22, 2026
frontegg-david added a commit that referenced this pull request Feb 22, 2026
* Cherry-pick: fix: Update default authentication mode to public and remove warning for unauthenticated usage (#245)

Cherry-picked from #244 (merged to release/0.11.x)
Original commit: 48f5226

Co-authored-by: agentfront[bot] <agentfront[bot]@users.noreply.github.com>
Co-authored-by: frontegg-david <69419539+frontegg-david@users.noreply.github.com>

* feat: Add new spec fields to skill metadata including license, compatibility, and resources (#247)

* feat: Add new spec fields to skill metadata including license, compatibility, and resources

* feat: Enhance skill directory loading with improved error handling and path management

* feat: Improve skill markdown parser by refining body extraction and updating metadata serialization

* Cherry-pick: feat: Enhance Docker support with package manager selection and improved E2E testing for #248 (#249)

* feat: Enhance Docker support with package manager selection and improved E2E testing (#246)

* feat: Enhance Docker support with package manager selection and improved E2E testing

* fix: Improve Dockerfile patch verification by using fixed string search

* fix: Update Dockerfile generation to enable corepack for non-npm package managers and improve error handling in patch verification

* fix: Update @enclave-vm/core and @enclave-vm/ast dependencies to version 2.11.0

* fix: Refactor Docker image cleanup and enhance GitHub CI dependency installation commands

* fix: Update Dockerfile to prune devDependencies and adjust file copy commands for production stage

* fix: Add package manager option to installation and quickstart documentation

* fix: Add .dockerignore template to improve Docker build context management

* fix: Add .dockerignore template to improve Docker build context management

* fix: Update Node.js version in Dockerfile and CI configurations to 22

* fix: Update Dockerfile and CI configurations to use Node.js version 24

* fix: Update run-e2e.sh to rewrite localhost URLs for Docker accessibility in package-lock.json and yarn.lock

* fix: Simplify Docker availability detection and update build commands for Verdaccio registry

* feat: Bump package versions to 0.11.1 and update rollback test expectations

* feat: Update package.json engine requirements for npm, yarn, and pnpm to node>=22

* feat: Enhance logging in adapter and app registries with verbose messages for initialization and registration processes (#250)

* feat: Enhance logging in adapter and app registries with verbose messages for initialization and registration processes

* feat: Enhance logging and timeout settings in workflows and handlers

* feat: Improve JWT detection and enhance logging in session verification flows

* feat: Enhance JWT detection logic and improve session masking in verification flows

* feat: Add base64url decoding for JWT header validation and enhance token leakage checks

* feat: Update @enclave-vm/core and @enclave-vm/ast dependencies to version 2.11.1

* feat: Improve logging and add new resource fields in skill metadata

* feat: Enhance error handling in skill directory loader and improve plugin registration logging

* feat: Enhance error handling in skill directory loader and improve plugin registration logging

* feat: Adjust performance test expectations for CI/local runner variance

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: agentfront[bot] <agentfront[bot]@users.noreply.github.com>
frontegg-david added a commit that referenced this pull request Feb 22, 2026
* Cherry-pick: fix: Update default authentication mode to public and remove warning for unauthenticated usage (#245)

Cherry-picked from #244 (merged to release/0.11.x)
Original commit: 48f5226

Co-authored-by: agentfront[bot] <agentfront[bot]@users.noreply.github.com>
Co-authored-by: frontegg-david <69419539+frontegg-david@users.noreply.github.com>

* feat: Add new spec fields to skill metadata including license, compatibility, and resources (#247)

* feat: Add new spec fields to skill metadata including license, compatibility, and resources

* feat: Enhance skill directory loading with improved error handling and path management

* feat: Improve skill markdown parser by refining body extraction and updating metadata serialization

* Cherry-pick: feat: Enhance Docker support with package manager selection and improved E2E testing for #248 (#249)

* feat: Enhance Docker support with package manager selection and improved E2E testing (#246)

* feat: Enhance Docker support with package manager selection and improved E2E testing

* fix: Improve Dockerfile patch verification by using fixed string search

* fix: Update Dockerfile generation to enable corepack for non-npm package managers and improve error handling in patch verification

* fix: Update @enclave-vm/core and @enclave-vm/ast dependencies to version 2.11.0

* fix: Refactor Docker image cleanup and enhance GitHub CI dependency installation commands

* fix: Update Dockerfile to prune devDependencies and adjust file copy commands for production stage

* fix: Add package manager option to installation and quickstart documentation

* fix: Add .dockerignore template to improve Docker build context management

* fix: Add .dockerignore template to improve Docker build context management

* fix: Update Node.js version in Dockerfile and CI configurations to 22

* fix: Update Dockerfile and CI configurations to use Node.js version 24

* fix: Update run-e2e.sh to rewrite localhost URLs for Docker accessibility in package-lock.json and yarn.lock

* fix: Simplify Docker availability detection and update build commands for Verdaccio registry

* feat: Bump package versions to 0.11.1 and update rollback test expectations

* feat: Update package.json engine requirements for npm, yarn, and pnpm to node>=22

* feat: Enhance logging in adapter and app registries with verbose messages for initialization and registration processes (#250)

* feat: Enhance logging in adapter and app registries with verbose messages for initialization and registration processes

* feat: Enhance logging and timeout settings in workflows and handlers

* feat: Improve JWT detection and enhance logging in session verification flows

* feat: Enhance JWT detection logic and improve session masking in verification flows

* feat: Add base64url decoding for JWT header validation and enhance token leakage checks

* feat: Update @enclave-vm/core and @enclave-vm/ast dependencies to version 2.11.1

* feat: Improve logging and add new resource fields in skill metadata

* feat: Enhance error handling in skill directory loader and improve plugin registration logging

* feat: Enhance error handling in skill directory loader and improve plugin registration logging

* feat: Adjust performance test expectations for CI/local runner variance

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: agentfront[bot] <agentfront[bot]@users.noreply.github.com>
frontegg-david added a commit that referenced this pull request Feb 22, 2026
* Cherry-pick: fix: Update default authentication mode to public and remove warning for unauthenticated usage (#245)

Cherry-picked from #244 (merged to release/0.11.x)
Original commit: 48f5226




* feat: Add new spec fields to skill metadata including license, compatibility, and resources (#247)

* feat: Add new spec fields to skill metadata including license, compatibility, and resources

* feat: Enhance skill directory loading with improved error handling and path management

* feat: Improve skill markdown parser by refining body extraction and updating metadata serialization

* Cherry-pick: feat: Enhance Docker support with package manager selection and improved E2E testing for #248 (#249)

* feat: Enhance Docker support with package manager selection and improved E2E testing (#246)

* feat: Enhance Docker support with package manager selection and improved E2E testing

* fix: Improve Dockerfile patch verification by using fixed string search

* fix: Update Dockerfile generation to enable corepack for non-npm package managers and improve error handling in patch verification

* fix: Update @enclave-vm/core and @enclave-vm/ast dependencies to version 2.11.0

* fix: Refactor Docker image cleanup and enhance GitHub CI dependency installation commands

* fix: Update Dockerfile to prune devDependencies and adjust file copy commands for production stage

* fix: Add package manager option to installation and quickstart documentation

* fix: Add .dockerignore template to improve Docker build context management

* fix: Add .dockerignore template to improve Docker build context management

* fix: Update Node.js version in Dockerfile and CI configurations to 22

* fix: Update Dockerfile and CI configurations to use Node.js version 24

* fix: Update run-e2e.sh to rewrite localhost URLs for Docker accessibility in package-lock.json and yarn.lock

* fix: Simplify Docker availability detection and update build commands for Verdaccio registry

* feat: Bump package versions to 0.11.1 and update rollback test expectations

* feat: Update package.json engine requirements for npm, yarn, and pnpm to node>=22

* feat: Enhance logging in adapter and app registries with verbose messages for initialization and registration processes (#250)

* feat: Enhance logging in adapter and app registries with verbose messages for initialization and registration processes

* feat: Enhance logging and timeout settings in workflows and handlers

* feat: Improve JWT detection and enhance logging in session verification flows

* feat: Enhance JWT detection logic and improve session masking in verification flows

* feat: Add base64url decoding for JWT header validation and enhance token leakage checks

* feat: Update @enclave-vm/core and @enclave-vm/ast dependencies to version 2.11.1

* feat: Improve logging and add new resource fields in skill metadata

* feat: Enhance error handling in skill directory loader and improve plugin registration logging

* feat: Enhance error handling in skill directory loader and improve plugin registration logging

* feat: Adjust performance test expectations for CI/local runner variance

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: agentfront[bot] <agentfront[bot]@users.noreply.github.com>
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.

1 participant