Skip to content

feat(kernel): resilience layers — breaker + injected observability + WebBotAuth signing (Phase A slice 2) - #34

Merged
yakimoto merged 2 commits into
mainfrom
feat/kernel-resilience-slice2
Jul 15, 2026
Merged

feat(kernel): resilience layers — breaker + injected observability + WebBotAuth signing (Phase A slice 2)#34
yakimoto merged 2 commits into
mainfrom
feat/kernel-resilience-slice2

Conversation

@yakimoto

@yakimoto yakimoto commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Fills the inert slice-1 resilience seams in @wave-av/kernel with concrete, opt-in, inert-by-default implementations. A client constructed without a resilience config behaves exactly like the slice-1 SDK wrapper — existing callers are unaffected.

Layers

1. Circuit breaker (createCircuitBreaker, opossum-backed)

  • Implements the CircuitBreakerLike seam. Params match the WAVE reference: errorThresholdPercentage 50 / resetTimeout 30000 / rollingCountTimeout 60000 / volumeThreshold 5.
  • A single breaker guards the whole Kernel call path (shared rolling window). WaveKernel.run(action) routes SDK actions through it when configured; when open it rejects fast with CircuitOpenError.
  • Per-call timeout defaults to disabled (timeout: false) — the SDK already owns request timeouts.

2. Observability capture (injected, no hard dep)

  • No @sentry/nextjs / Next dependency — this SDK is consumed by non-Next hosts too.
  • The seam accepts optional injected captureError / captureBreadcrumb hooks the consumer wires to their own reporter. Breaker open/halfOpen/close transitions emit breadcrumbs (category: 'kernel'); run() errors go through captureError tagged service: 'kernel'.

3. WebBotAuth signing (createWebBotAuthSigner)

  • Implements the SignerLike seam: RFC-9421 HTTP Message Signatures with Ed25519 via @noble/ed25519 (lightweight, maintained, runs on Node/browser/edge — no framework dep).
  • Wired into the SDK's request path via a custom fetch (the SDK exposes a fetch option), so it adapts to the @onkernel/sdk call path rather than transplanting a fetch layer.
  • Fail-open: any signing error is reported via the injected hook and the request proceeds unsigned — signing never blocks a request. Opt-in (only when a key is configured).

Deps added

opossum ^10, @noble/ed25519 ^3 (runtime); @types/opossum ^8 (dev).

Test coverage

12 → 26 tests: circuit-breaker (pass-through, error propagation, trip → CircuitOpenError + breadcrumb, serviceName naming), WebBotAuth (header shape, Signature-Agent coverage, signature verifies against reconstructed base, fail-open), and client resilience wiring (inert run(), breaker routing + captureError, signer construction).

pnpm --filter @wave-av/kernel type-check / test / build all exit 0. dist/ cleaned before commit.

🤖 Generated with Claude Code


Note

Medium Risk
Introduces request signing with private keys and changes failure/shedding behavior when callers opt into the breaker; fail-open signing limits blast radius but misconfiguration could send unsigned or fast-failing traffic.

Overview
Implements the previously inert resilience seams in @wave-av/kernel with concrete, opt-in layers. Clients without resilience config behave like before.

Circuit breaker: Adds createCircuitBreaker (opossum) with WAVE-aligned defaults, open/half-open/close breadcrumbs via injected captureBreadcrumb, and CircuitOpenError when shedding. WaveKernel.run() runs SDK actions through the breaker when configured.

WebBotAuth: Adds createWebBotAuthSigner (RFC-9421 Ed25519 via @noble/ed25519). When a signer is passed in resilience, the client wraps the SDK fetch to attach signature headers; signing fails open (unsigned request + optional captureError).

Observability: Formalizes CaptureError / CaptureBreadcrumb on ResilienceHooks (no hard Sentry dep). Breaker errors from run() and signing failures can be reported through injected hooks.

Public exports and vitest coverage are expanded for breaker, signer, and client wiring. Runtime deps: opossum, @noble/ed25519.

Reviewed by Cursor Bugbot for commit bdf0897. Configure here.


Summary by cubic

Adds opt-in resilience to @wave-av/kernel: a shared circuit breaker, injected observability hooks, and WebBotAuth request signing. Defaults keep behavior unchanged when no resilience config is provided.

  • New Features

    • Circuit breaker via createCircuitBreaker (opossum): WaveKernel.run() routes calls through it, fast-fails when open, and emits breadcrumbs. Defaults: errorThresholdPercentage 50, resetTimeout 30s, rollingCountTimeout 60s, volumeThreshold 5; breaker timeout disabled.
    • Injected observability: captureError and captureBreadcrumb hooks, no hard @sentry/nextjs dep; errors are tagged service: 'kernel'.
    • WebBotAuth signing via createWebBotAuthSigner (@noble/ed25519): RFC-9421 headers, wired through the SDK fetch option, fail-open on errors; optional Signature-Agent is covered.
    • RFC-9421 fix: @path is path-only and @query is now included in the default covered components.
  • Dependencies

    • Added opossum, @noble/ed25519; dev: @types/opossum.

Written for commit 7eab4af. Summary will update on new commits.

Review in cubic

…WebBotAuth signing (Phase A slice 2)

Fill the inert slice-1 resilience seams with concrete, opt-in implementations.
All three layers are inert by default — a client constructed without a
`resilience` config behaves exactly like the slice-1 SDK wrapper.

- Circuit breaker: opossum-backed `createCircuitBreaker` implementing the
  CircuitBreakerLike seam (errorThresholdPercentage 50 / resetTimeout 30000 /
  rollingCountTimeout 60000 / volumeThreshold 5). `WaveKernel.run()` guards the
  SDK call path through the breaker when configured; open trips a
  CircuitOpenError. State transitions emit breadcrumbs via an injected hook.
- Observability: no hard Sentry/Next dep. The seam accepts injected
  captureError / captureBreadcrumb hooks (service: 'kernel' intent), so non-Next
  hosts can wire their own reporter.
- WebBotAuth signing: `createWebBotAuthSigner` implementing the SignerLike seam —
  RFC-9421 Ed25519 request signing via @noble/ed25519, wired into the SDK's
  fetch path. Fail-open: any signing error is captured and the request proceeds
  unsigned. Opt-in (only when a key is configured).

New deps: opossum ^10, @noble/ed25519 ^3, @types/opossum ^8 (dev).
Tests: 12 -> 26 (breaker, signer, client resilience wiring). type-check/test/build all green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 15, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_e5e485e1-6f3a-4cb1-b1cf-5956b0fb3f21)

@changeset-bot

changeset-bot Bot commented Jul 15, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 7eab4af

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds opt-in opossum circuit breaking and RFC-9421 WebBotAuth signing to the TypeScript Kernel SDK, expands resilience contracts and exports, wires both capabilities into WaveKernel, and adds focused tests.

Changes

Kernel resilience

Layer / File(s) Summary
Resilience contracts and exports
sdk-typescript/packages/kernel/package.json, sdk-typescript/packages/kernel/src/resilience.ts, sdk-typescript/packages/kernel/src/index.ts
Adds resilience observability types, circuit-breaker and signer exports, and the required cryptography and breaker packages.
Circuit breaker implementation
sdk-typescript/packages/kernel/src/circuit-breaker.ts, sdk-typescript/packages/kernel/src/__tests__/circuit-breaker.test.ts
Adds an opossum-backed breaker with configurable thresholds, state breadcrumbs, open-circuit error translation, and coverage for closed, failed, and open states.
WebBotAuth signer
sdk-typescript/packages/kernel/src/web-bot-auth.ts, sdk-typescript/packages/kernel/src/__tests__/web-bot-auth.test.ts
Adds configurable Ed25519 RFC-9421 signing, optional signature-agent support, signature verification coverage, and fail-open error handling.
WaveKernel resilience wiring
sdk-typescript/packages/kernel/src/client.ts, sdk-typescript/packages/kernel/src/__tests__/client.test.ts
Routes actions through an optional breaker and requests through an optional signing fetch wrapper while reporting captured errors.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant WaveKernel
  participant OpossumCircuitBreaker
  participant WebBotAuthSigner
  participant KernelSDK
  participant Fetch
  Caller->>WaveKernel: run(action)
  WaveKernel->>OpossumCircuitBreaker: fire(action)
  OpossumCircuitBreaker-->>WaveKernel: result or circuit-open error
  KernelSDK->>WebBotAuthSigner: sign(request)
  WebBotAuthSigner-->>KernelSDK: signature headers
  KernelSDK->>Fetch: send signed request
Loading

Possibly related PRs

  • wave-av/sdks#33: Introduced the WaveKernel resilience wiring extended by this change.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately summarizes the main change: opt-in kernel resilience with circuit breaking, observability hooks, and WebBotAuth signing.
Description check ✅ Passed The description is clearly related to the changeset and matches the added resilience, signing, and dependency updates.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/kernel-resilience-slice2
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch feat/kernel-resilience-slice2

Warning

Review ran into problems

🔥 Problems

These MCP integrations need to be re-authenticated in the Integrations settings: Sentry


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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@sdk-typescript/packages/kernel/package.json`:
- Around line 69-71: Update the kernel package’s engines declaration and
corresponding CI/runtime baseline to require Node 22 or newer, matching the
requirements of `@noble/ed25519` and opossum; keep the dependency versions
unchanged.

In `@sdk-typescript/packages/kernel/src/circuit-breaker.ts`:
- Around line 77-103: Isolate each captureBreadcrumb invocation registered on
this.breaker so a throwing breadcrumb sink cannot escape the synchronous open,
halfOpen, or close lifecycle listener or alter breaker behavior. Catch and
suppress hook errors while preserving the existing breaker transitions and
breadcrumb payloads. Add a regression test covering a throwing captureBreadcrumb
during a lifecycle event, including timer-driven transitions if supported by the
existing test setup.

In `@sdk-typescript/packages/kernel/src/web-bot-auth.ts`:
- Around line 56-58: Defer string key conversion from the constructor into the
fail-open signing path so malformed hex cannot throw during WebBotAuthSigner
construction. Update the signing method to convert and use its local privateKey
when calling ed25519.signAsync, preserving graceful failure without blocking
requests.
- Around line 166-174: Wire WebBotAuthSigner.sign() failures to the shared
resilience.captureError hook so fail-open signing errors are observable,
preferably by having WaveKernel inject that hook when constructing the signer.
In sdk-typescript/packages/kernel/src/web-bot-auth.ts lines 166-174, update the
signer’s error path; in lines 177-194, update the JSDoc example to pass the same
captureError function to createWebBotAuthSigner and resilience.captureError. In
sdk-typescript/packages/kernel/src/client.ts lines 99-131, ensure WaveKernel
provides the shared hook to the signer, or explicitly document the remaining
limitation if the signer type cannot support injection.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e6f92d27-5415-4b01-bdf0-0c91e7f1d636

📥 Commits

Reviewing files that changed from the base of the PR and between d274020 and bdf0897.

⛔ Files ignored due to path filters (1)
  • sdk-typescript/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (9)
  • sdk-typescript/packages/kernel/package.json
  • sdk-typescript/packages/kernel/src/__tests__/circuit-breaker.test.ts
  • sdk-typescript/packages/kernel/src/__tests__/client.test.ts
  • sdk-typescript/packages/kernel/src/__tests__/web-bot-auth.test.ts
  • sdk-typescript/packages/kernel/src/circuit-breaker.ts
  • sdk-typescript/packages/kernel/src/client.ts
  • sdk-typescript/packages/kernel/src/index.ts
  • sdk-typescript/packages/kernel/src/resilience.ts
  • sdk-typescript/packages/kernel/src/web-bot-auth.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: Cursor Approval Agent: Pull Request Router and Approver
🧰 Additional context used
🔍 Remote MCP

Additional review context

  • Opossum’s fire() returns a Promise; the breaker emits reject, timeout, failure, open, close, and halfOpen, transitions to halfOpen after resetTimeout, and uses a rolling stats window governed by rollingCountTimeout and rollingCountBuckets. The docs also show volumeThreshold can block opening until enough requests exist, and list defaults of timeout=10000, resetTimeout=30000, rollingCountTimeout=10000, rollingCountBuckets=10, errorThresholdPercentage=50, volumeThreshold=0. (nodeshift.dev)

  • @noble/ed25519 v3 docs say the main APIs work on Uint8Array, with signAsync/verifyAsync/getPublicKeyAsync available; the v3 release notes explicitly say string hex inputs are prohibited. (npmjs.com)

  • Review check: if WebBotAuthConfig.privateKey accepts strings, the implementation should convert them to bytes before signing, or it will conflict with the current @noble/ed25519 API expectations. (npmjs.com)

🔇 Additional comments (8)
sdk-typescript/packages/kernel/src/circuit-breaker.ts (1)

16-75: LGTM!

Also applies to: 106-144

sdk-typescript/packages/kernel/src/__tests__/client.test.ts (2)

10-13: LGTM!

Also applies to: 65-84, 96-101


86-94: 🎯 Functional Correctness

No issue here — hex keys are decoded to bytes before signAsync, and the signing path is already covered in sdk-typescript/packages/kernel/src/__tests__/web-bot-auth.test.ts.

			> Likely an incorrect or invalid review comment.
sdk-typescript/packages/kernel/src/resilience.ts (1)

2-53: LGTM!

Also applies to: 63-65, 80-92

sdk-typescript/packages/kernel/src/index.ts (1)

43-65: LGTM!

sdk-typescript/packages/kernel/src/web-bot-auth.ts (1)

60-81: LGTM! RFC-9421 signature-base construction, base64/base64url helpers, and nonce generation are internally consistent (the exact params string used for signing is the one emitted in Signature-Input), and match the @noble/ed25519 v3 API (signAsync, etc.hexToBytes).

Also applies to: 94-165

sdk-typescript/packages/kernel/src/__tests__/web-bot-auth.test.ts (1)

1-102: LGTM!

sdk-typescript/packages/kernel/src/client.ts (1)

22-28: LGTM! run() correctly runs the action directly when no breaker is configured (unchanged behavior) and reports breaker errors via the injected hook before rethrowing; the conditional fetch wiring preserves the SDK default when no signer is configured.

Also applies to: 44-46, 62-76, 78-97

Comment on lines +69 to +71
"@noble/ed25519": "^3.1.0",
"@onkernel/sdk": "^0.78.0",
"opossum": "^10.0.0"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd -HI -a '^(package\.json|\.nvmrc|\.node-version|\.tool-versions)$' . \
  -x sh -c 'echo "== $1"; rg -n "\"engines\"|\"node\"|node-version" "$1" || true' sh {}

rg -n -g '*.{yml,yaml,json}' 'node-version|setup-node|NODE_VERSION|engines' . || true

Repository: wave-av/sdks

Length of output: 20928


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== package.json (root and kernel)"
sed -n '1,120p' sdk-typescript/package.json | cat -n
echo
sed -n '1,120p' sdk-typescript/packages/kernel/package.json | cat -n

echo
echo "== lockfile entries for `@noble/ed25519` and opossum"
python3 - <<'PY'
from pathlib import Path
p = Path('sdk-typescript/pnpm-lock.yaml')
text = p.read_text()
for needle in ['`@noble/ed25519`', 'opossum']:
    print(f"\n## {needle}")
    idx = 0
    while True:
        idx = text.find(needle, idx)
        if idx == -1:
            break
        start = text.rfind('\n', 0, idx)
        end = text.find('\n', idx)
        print(text[max(0, start-250):min(len(text), end+400)])
        print("\n---")
        idx += len(needle)
PY

Repository: wave-av/sdks

Length of output: 9458


🏁 Script executed:

#!/bin/bash
set -euo pipefail

for needle in '`@noble/ed25519`' 'opossum'; do
  echo "===== $needle ====="
  rg -n -C 4 --fixed-strings "$needle" sdk-typescript/pnpm-lock.yaml
  echo
done

Repository: wave-av/sdks

Length of output: 2788


🌐 Web query:

npm @noble/ed25519 3.1.0 engines node opossum 10.0.0 engines node

💡 Result:

The Node.js engine requirements for the specified package versions are as follows: - @noble/ed25519 version 3.1.0 requires Node.js v20.19 or later [1][2][3]. - opossum version 10.0.0 requires Node.js v22 or later (Node.js >= 22) [4][5]. This version introduced a breaking change by removing support for Node.js 20 [6][7].

Citations:


🌐 Web query:

site:npmjs.com opossum 10.0.0 engines node

💡 Result:

The opossum npm package, which serves as a circuit breaker for Node.js, specifies its Node.js engine requirement as ">=22" in its metadata [1]. This means that the package is intended for use in environments running Node.js version 22 or newer [1]. Users should ensure their Node.js environment meets this requirement to avoid potential compatibility issues during installation or execution [2][3].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '1426,1440p' sdk-typescript/pnpm-lock.yaml
echo '---'
sed -n '2210,2225p' sdk-typescript/pnpm-lock.yaml
echo '---'
sed -n '2374,2386p' sdk-typescript/pnpm-lock.yaml

Repository: wave-av/sdks

Length of output: 1943


Raise the kernel Node engine floor. sdk-typescript/packages/kernel/package.json:65-71 still says node: >=18.0.0, but @noble/ed25519@3.1.0 needs Node 20.19+ and opossum@10.0.0 needs Node 22+, so this package no longer matches its declared runtime support. Either pin older compatible versions or bump the documented engines/CI baseline.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sdk-typescript/packages/kernel/package.json` around lines 69 - 71, Update the
kernel package’s engines declaration and corresponding CI/runtime baseline to
require Node 22 or newer, matching the requirements of `@noble/ed25519` and
opossum; keep the dependency versions unchanged.

Comment on lines +77 to +103
const breadcrumb = options.captureBreadcrumb;
if (breadcrumb) {
this.breaker.on('open', () =>
breadcrumb({
category: this.serviceName,
message: 'Circuit breaker opened',
level: 'warning',
data: { state: 'open' },
}),
);
this.breaker.on('halfOpen', () =>
breadcrumb({
category: this.serviceName,
message: 'Circuit breaker half-open',
level: 'info',
data: { state: 'halfOpen' },
}),
);
this.breaker.on('close', () =>
breadcrumb({
category: this.serviceName,
message: 'Circuit breaker closed',
level: 'info',
data: { state: 'close' },
}),
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Do not let breadcrumb sinks alter breaker behavior.

A throwing captureBreadcrumb escapes these synchronous lifecycle listeners, potentially replacing the action error or crashing on timer-driven transitions. Opossum emits these lifecycle events, and Node EventEmitter listeners run synchronously. (nodeshift.dev)

Proposed fix
     const breadcrumb = options.captureBreadcrumb;
     if (breadcrumb) {
+      const capture = (event: Parameters<CaptureBreadcrumb>[0]) => {
+        try {
+          void Promise.resolve(breadcrumb(event)).catch(() => undefined);
+        } catch {
+          // Observability must not affect circuit operation.
+        }
+      };
       this.breaker.on('open', () =>
-        breadcrumb({
+        capture({
           category: this.serviceName,
           message: 'Circuit breaker opened',
           level: 'warning',
           data: { state: 'open' },
         }),
       );
       this.breaker.on('halfOpen', () =>
-        breadcrumb({
+        capture({
           category: this.serviceName,
           message: 'Circuit breaker half-open',
           level: 'info',
           data: { state: 'halfOpen' },
         }),
       );
       this.breaker.on('close', () =>
-        breadcrumb({
+        capture({
           category: this.serviceName,
           message: 'Circuit breaker closed',
           level: 'info',
           data: { state: 'close' },

Add a regression test with a throwing breadcrumb hook.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const breadcrumb = options.captureBreadcrumb;
if (breadcrumb) {
this.breaker.on('open', () =>
breadcrumb({
category: this.serviceName,
message: 'Circuit breaker opened',
level: 'warning',
data: { state: 'open' },
}),
);
this.breaker.on('halfOpen', () =>
breadcrumb({
category: this.serviceName,
message: 'Circuit breaker half-open',
level: 'info',
data: { state: 'halfOpen' },
}),
);
this.breaker.on('close', () =>
breadcrumb({
category: this.serviceName,
message: 'Circuit breaker closed',
level: 'info',
data: { state: 'close' },
}),
);
}
const breadcrumb = options.captureBreadcrumb;
if (breadcrumb) {
const capture = (event: Parameters<CaptureBreadcrumb>[0]) => {
try {
void Promise.resolve(breadcrumb(event)).catch(() => undefined);
} catch {
// Observability must not affect circuit operation.
}
};
this.breaker.on('open', () =>
capture({
category: this.serviceName,
message: 'Circuit breaker opened',
level: 'warning',
data: { state: 'open' },
}),
);
this.breaker.on('halfOpen', () =>
capture({
category: this.serviceName,
message: 'Circuit breaker half-open',
level: 'info',
data: { state: 'halfOpen' },
}),
);
this.breaker.on('close', () =>
capture({
category: this.serviceName,
message: 'Circuit breaker closed',
level: 'info',
data: { state: 'close' },
}),
);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sdk-typescript/packages/kernel/src/circuit-breaker.ts` around lines 77 - 103,
Isolate each captureBreadcrumb invocation registered on this.breaker so a
throwing breadcrumb sink cannot escape the synchronous open, halfOpen, or close
lifecycle listener or alter breaker behavior. Catch and suppress hook errors
while preserving the existing breaker transitions and breadcrumb payloads. Add a
regression test covering a throwing captureBreadcrumb during a lifecycle event,
including timer-driven transitions if supported by the existing test setup.

Comment on lines +56 to +58
function toBytes(key: Uint8Array | string): Uint8Array {
return typeof key === 'string' ? ed25519.etc.hexToBytes(key) : key;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Constructor can throw synchronously, breaking the fail-open guarantee.

toBytes() calls ed25519.etc.hexToBytes() directly in the constructor (Line 87), outside any try/catch. A malformed hex string (odd length, non-hex chars) throws synchronously from new WebBotAuthSigner(...)/createWebBotAuthSigner(...) — i.e. at signer construction time, before any request or sign() call. This contradicts the file's own contract: "Signing is OPT-IN... and FAILS OPEN... A signing failure must never block a request." A bad key (e.g. malformed env var) crashes app init instead of degrading gracefully.

🛡️ Proposed fix — defer key conversion into the fail-open path
 export class WebBotAuthSigner implements SignerLike {
-  private readonly privateKey: Uint8Array;
-
-  constructor(private readonly config: WebBotAuthConfig) {
-    this.privateKey = toBytes(config.privateKey);
-  }
+  constructor(private readonly config: WebBotAuthConfig) {}

   async sign(input: {
     method: string;
     url: string;
     headers: Record<string, string>;
   }): Promise<Record<string, string>> {
     try {
+      const privateKey = toBytes(this.config.privateKey);
       const url = new URL(input.url);

And use the local privateKey in the ed25519.signAsync(...) call instead of this.privateKey.

Also applies to: 83-88

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sdk-typescript/packages/kernel/src/web-bot-auth.ts` around lines 56 - 58,
Defer string key conversion from the constructor into the fail-open signing path
so malformed hex cannot throw during WebBotAuthSigner construction. Update the
signing method to convert and use its local privateKey when calling
ed25519.signAsync, preserving graceful failure without blocking requests.

Comment on lines +166 to +174
return headers;
} catch (error) {
this.config.captureError?.(error, {
service: 'kernel',
component: 'web-bot-auth',
});
return {};
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

WebBotAuthConfig.captureError and ResilienceHooks.captureError are unwired — signing failures can go unreported. WebBotAuthSigner.sign() only reports errors through its own this.config.captureError (a hook set on WebBotAuthConfig), which is entirely separate from the resilience.captureError hook WaveKernel exposes. The documented integration path never connects the two, so a signing failure is silently swallowed with no observability, contradicting the PR's stated goal that signing "reports errors through the injected hook."

  • sdk-typescript/packages/kernel/src/web-bot-auth.ts#L166-L174: this is the root cause — the signer's fail-open catch has no path to the shared resilience.captureError hook.
  • sdk-typescript/packages/kernel/src/web-bot-auth.ts#L177-L194: update the JSDoc example to also pass a shared captureError into createWebBotAuthSigner(...) (the same function passed to resilience.captureError), so the recommended usage doesn't silently drop errors.
  • sdk-typescript/packages/kernel/src/client.ts#L99-L131: this catch's resilience.captureError call cannot observe WebBotAuthSigner's internal signing failures (only its own method/URL extraction errors here); note this limitation or have WaveKernel inject resilience.captureError into the signer at construction if the signer type supports it.
📍 Affects 2 files
  • sdk-typescript/packages/kernel/src/web-bot-auth.ts#L166-L174 (this comment)
  • sdk-typescript/packages/kernel/src/web-bot-auth.ts#L177-L194
  • sdk-typescript/packages/kernel/src/client.ts#L99-L131
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sdk-typescript/packages/kernel/src/web-bot-auth.ts` around lines 166 - 174,
Wire WebBotAuthSigner.sign() failures to the shared resilience.captureError hook
so fail-open signing errors are observable, preferably by having WaveKernel
inject that hook when constructing the signer. In
sdk-typescript/packages/kernel/src/web-bot-auth.ts lines 166-174, update the
signer’s error path; in lines 177-194, update the JSDoc example to pass the same
captureError function to createWebBotAuthSigner and resilience.captureError. In
sdk-typescript/packages/kernel/src/client.ts lines 99-131, ensure WaveKernel
provides the shared hook to the signer, or explicitly document the remaining
limitation if the signer type cannot support injection.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
signatureInput: string,
label = 'sig1',
): string {
const params = signatureInput.replace(new RegExp(`^${label}=`), '');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Semgrep identified an issue in your code:
RegExp() called with a label function argument, this might allow an attacker to cause a Regular Expression Denial-of-Service (ReDoS) within your application as RegExP blocks the main thread. For this reason, it is recommended to use hardcoded regexes instead. If your regex is run on user-controlled input, consider performing input validation or use a regex checking/sanitization library such as https://www.npmjs.com/package/recheck to verify that the regex does not appear vulnerable to ReDoS.

Dataflow graph
flowchart LR
    classDef invis fill:white, stroke: none
    classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none

    subgraph File0["<b>sdk-typescript/packages/kernel/src/__tests__/web-bot-auth.test.ts</b>"]
        direction LR
        %% Source

        subgraph Source
            direction LR

            v0["<a href=https://github.com/wave-av/sdks/blob/7eab4af663bb1151efc7c9d1f8fbafa8894b86c5/sdk-typescript/packages/kernel/src/__tests__/web-bot-auth.test.ts#L21 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 21] label</a>"]
        end
        %% Intermediate

        subgraph Traces0[Traces]
            direction TB

            v2["<a href=https://github.com/wave-av/sdks/blob/7eab4af663bb1151efc7c9d1f8fbafa8894b86c5/sdk-typescript/packages/kernel/src/__tests__/web-bot-auth.test.ts#L21 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 21] label</a>"]

            v3["<a href=https://github.com/wave-av/sdks/blob/7eab4af663bb1151efc7c9d1f8fbafa8894b86c5/sdk-typescript/packages/kernel/src/__tests__/web-bot-auth.test.ts#L23 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 23] `</a>"]
        end
            v2 --> v3
        %% Sink

        subgraph Sink
            direction LR

            v1["<a href=https://github.com/wave-av/sdks/blob/7eab4af663bb1151efc7c9d1f8fbafa8894b86c5/sdk-typescript/packages/kernel/src/__tests__/web-bot-auth.test.ts#L23 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 23] new RegExp(`^${label}=`)</a>"]
        end
    end
    %% Class Assignment
    Source:::invis
    Sink:::invis

    Traces0:::invis
    File0:::invis

    %% Connections

    Source --> Traces0
    Traces0 --> Sink


Loading

To resolve this comment:

🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.

💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

  • /fp <comment> for false positive
  • /ar <comment> for acceptable risk
  • /other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by detect-non-literal-regexp.

You can view more details about this finding in the Semgrep AppSec Platform.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Risk: medium. Not approving: Cursor Bugbot skipped (usage limit reached) so automated review is incomplete. Human review is needed; no reviewers were assigned because only the PR author is available in this repo.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@yakimoto
yakimoto merged commit 5c668ca into main Jul 15, 2026
15 of 16 checks passed
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