refactor(sdk)!: switch to apikey values; update sdk e2e tests and examples#168
Conversation
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThis PR replaces SDK ChangesSDK configuration and execution updates
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
achuvyas-kv
force-pushed
the
feat/sdk-value-api-keys
branch
from
8b3e523 to
dba6da9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (10)
tests/e2e/sdk/package.json (2)
12-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider pinning the internal SDK dependency more tightly.
A caret range against a same-repo workspace package is fragile: if
runners/sdk's version diverges from what satisfies^0.9.0, npm may resolve from the registry instead of the local workspace, silently breaking e2e tests against a stale/unpublished SDK build.🤖 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 `@tests/e2e/sdk/package.json` around lines 12 - 17, The e2e package is depending on the internal SDK with a loose caret range, which can allow npm to resolve a published registry version instead of the local workspace build. Tighten the dependency in the package manifest for the SDK entry used by the e2e tests so it always points to the intended workspace version, and keep the exact package name consistent with the `runners/sdk` source to avoid accidental stale resolution.
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGlob pattern relies on shell expansion.
src/**/*.e2e.test.tsis unquoted, so it's expanded by the shell rather than Node's--testrunner. Withoutglobstarenabled,**behaves like*and won't recurse into subdirectories if tests are later nested (e.g.src/foo/bar.e2e.test.ts).🤖 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 `@tests/e2e/sdk/package.json` at line 9, The test script in package.json is relying on shell glob expansion because src/**/*.e2e.test.ts is unquoted. Update the test command so the Node --test runner receives the glob directly, using the existing test script entry as the place to fix it. This will ensure nested e2e tests under src continue to be discovered correctly without depending on shell globstar behavior.tests/e2e/sdk/src/helpers/mockServer.ts (1)
89-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueJudge detection relies on fragile prompt-text matching.
Detecting the judge call via
systemMsg.includes("security evaluator")couples this mock to the exact wording of the core evaluator's judge prompt. If that prompt text changes, this mock will silently misroute judge/attacker calls, causing tests to pass or fail for the wrong reasons.🤖 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 `@tests/e2e/sdk/src/helpers/mockServer.ts` around lines 89 - 90, Judge detection is currently tied to the exact system prompt text in mockServer’s request routing, which is brittle. Update the mock’s judge/attacker classification in the request handler to use a stable signal instead of systemMsg.includes("security evaluator"), such as an explicit marker/field on the payload or a dedicated request shape emitted by the caller. Keep the change localized to the mockServer logic that decides isJudge so prompt wording changes in the evaluator do not affect test routing.runners/sdk/src/hunt.ts (1)
101-116: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoffSerialize SDK runs or document single-flight usage
withTempEnv()mutates globalprocess.envwhilerunOnce()is pending, andrunners/sdk/src/run.tsdoes the same. If two calls overlap in one process, they can clobber each other's credentials or restore stale env; either guard these entry points with a mutex or document that concurrenthunt()/run()invocations aren't supported.🤖 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 `@runners/sdk/src/hunt.ts` around lines 101 - 116, Serialize the environment-mutating SDK entry points: both hunt() here and run() in runners/sdk/src/run.ts use withTempEnv/process.env while async work is pending, so overlapping calls can clobber credentials. Fix this by adding a single-flight/mutex around these paths, or clearly document in hunt() and run() that concurrent invocations in the same process are not supported; reference the hunt() wrapper and the runOnce() flow when applying the guard.runners/sdk/examples/_helpers/mockLane.ts (1)
64-123: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueNo
errorhandler on the request stream.
req.on("data", ...)/req.on("end", ...)are wired but there's noreq.on("error", ...). If a client aborts mid-request, the unhandled'error'event on the stream can throw. Low risk for a local example server, but worth a defensive handler.🤖 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 `@runners/sdk/examples/_helpers/mockLane.ts` around lines 64 - 123, The request stream in createServer lacks an error handler, so aborted or malformed requests can trigger an unhandled event. Update the req listener setup in mockLane.ts to handle req.on("error", ...) alongside the existing data/end handlers, and make sure the server responds or safely exits the request path without crashing when a stream error occurs.runners/sdk/examples/target-mcp-url.ts (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePlaceholder token isn't actually substituted.
"Bearer $MCP_TOKEN"is a literal string, not env interpolation — readers may mistakenly think it's resolved at runtime.🤖 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 `@runners/sdk/examples/target-mcp-url.ts` at line 9, The Authorization header in the target MCP URL example uses a literal placeholder string, which can mislead readers into thinking it is interpolated at runtime. Update the example near urlHeaders so it clearly shows a real token source from the environment or otherwise indicates that the value is not substituted automatically, and keep the fix centered on the target-mcp-url example setup.runners/sdk/src/types.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLocal
RunListenernow diverges from core'sCoreRunListener.The file no longer re-exports
RunListenerfrom core; instead it defines a structurally similar local interface. Perrunners/sdk/src/run.ts'swrapListener, this local shape is manually mapped onto core'sCoreRunListenerfield-by-field. If core adds/renames a lifecycle hook in a future version, this local interface (andwrapListener) won't pick it up automatically, and the mismatch would only surface as a silent behavioral gap rather than a compile error.Consider adding a lightweight compile-time check (e.g. a type test asserting the wrapped shape is still assignable to/from
CoreRunListener) to catch drift early when bumping@keyvaluesystems/agent-opfor-core.Also applies to: 165-191
🤖 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 `@runners/sdk/src/types.ts` at line 1, The local RunListener shape in types.ts and the manual mapping in wrapListener should be protected against drift from CoreRunListener. Add a lightweight compile-time type check near the RunListener definition or wrapListener in runners/sdk/src/run.ts that asserts the wrapped listener remains assignable to/from CoreRunListener, so any added or renamed lifecycle hook in `@keyvaluesystems/agent-opfor-core` fails during compilation instead of silently mismatching at runtime.runners/sdk/src/internal/buildRunConfig.ts (2)
134-144: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSilent fallback to "anthropic" on unmatched model names.
If a model string doesn't match any known substring pattern,
inferProvidersilently defaults to"anthropic", which will setANTHROPIC_API_KEYinstead of surfacing a clear "unrecognized model — specifyproviderexplicitly" error. This can hide misconfiguration until the target auth fails downstream.♻️ Proposed fix: fail fast on unrecognized model strings
function inferProvider(model: string): ProviderName { const lower = model.toLowerCase(); if (lower.includes("claude") || lower.includes("anthropic")) return "anthropic"; if (lower.includes("gpt") || lower.includes("o1") || lower.includes("o3")) return "openai"; if (lower.includes("gemini")) return "google"; if (lower.includes("llama") || lower.includes("mixtral")) return "groq"; if (lower.includes("deepseek")) return "deepseek"; - return "anthropic"; + throw new Error( + `Unable to infer provider for model "${model}". Pass a ModelSpec object with an explicit "provider" field instead of a bare model string.` + ); }🤖 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 `@runners/sdk/src/internal/buildRunConfig.ts` around lines 134 - 144, The inferProvider helper is silently defaulting unmatched model names to "anthropic", which can mask misconfiguration. Update inferProvider in buildRunConfig so it no longer returns a fallback provider for unknown strings; instead, fail fast with a clear error telling the caller the model is unrecognized and that provider must be specified explicitly. Make sure the change is applied where inferProvider is used so the BuildRunConfig flow does not continue with an incorrect provider/API key selection.
5-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport these core types from the owning module.
@keyvaluesystems/agent-opfor-corere-exports these from./execute/types.js, so import them directly from@keyvaluesystems/agent-opfor-core/execute/types.jsinstead of the package root to avoid the barrel import.🤖 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 `@runners/sdk/src/internal/buildRunConfig.ts` around lines 5 - 10, The import in buildRunConfig should come from the owning execute/types module instead of the package root barrel. Update the type-only import in buildRunConfig to reference `@keyvaluesystems/agent-opfor-core/execute/types.js` directly for RunConfig, AgentTargetConfig, McpTargetConfig, and EvaluatorSelection so the file bypasses the root re-export.Source: Coding guidelines
runners/sdk/src/run.ts (1)
2-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame barrel-vs-subpath import inconsistency as
buildRunConfig.ts.
UnifiedRunReportis imported from the bare@keyvaluesystems/agent-opfor-corespecifier whileRunListeneron the next line correctly uses a direct subpath (/execute/runListener.js). As per coding guidelines: "Do not use barrel re-exports; import symbols directly from the file that owns them."🤖 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 `@runners/sdk/src/run.ts` around lines 2 - 3, The imports in run.ts are inconsistent with the direct-subpath rule: UnifiedRunReport is coming from the package barrel while RunListener already uses a file-owned subpath. Update the import for UnifiedRunReport to use its owning module path, matching the style used by CoreRunListener and the pattern in buildRunConfig.ts, and avoid any barrel re-export imports in this file.Source: Coding guidelines
🤖 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 `@runners/sdk/examples/ci-gate.ts`:
- Around line 24-28: The CI gate logic in ci-gate.ts exits too early and skips
the cleanup in the surrounding try/finally. Update the results handling in the
main script flow so it sets process.exitCode = 1 instead of calling
process.exit(1), allowing the code to continue into the existing finally block
and complete lane.close() naturally. Use the ci-gate results check and the
lane.close() cleanup path as the key symbols when making the change.
In `@runners/sdk/src/run.ts`:
- Around line 19-41: Global process.env mutation in run() is unsafe because
concurrent calls can overwrite each other’s credentials before runAll()
completes. Update run() and buildRunConfig/runAll usage so credentials are
passed through RunOptions/RunConfig instead of writing to process.env, or add
single-flight serialization if shared env state is required; make sure the fix
addresses the env handling around buildRunConfig, runAll, and transformReport.
In `@tests/e2e/sdk/src/helpers/mockServer.ts`:
- Around line 82-90: The mock request parsing in the response handler currently
trusts external HTTP input by casting the result of JSON.parse(body) to a typed
object, which bypasses runtime validation. Update the parsing logic in
mockServer’s request handling to validate the body with Zod before accessing
parsed.messages, and keep the existing fallback behavior for malformed input.
Use a Zod schema that matches the expected structure so systemMsg and the
isJudge check read from validated data instead of a raw cast.
---
Nitpick comments:
In `@runners/sdk/examples/_helpers/mockLane.ts`:
- Around line 64-123: The request stream in createServer lacks an error handler,
so aborted or malformed requests can trigger an unhandled event. Update the req
listener setup in mockLane.ts to handle req.on("error", ...) alongside the
existing data/end handlers, and make sure the server responds or safely exits
the request path without crashing when a stream error occurs.
In `@runners/sdk/examples/target-mcp-url.ts`:
- Line 9: The Authorization header in the target MCP URL example uses a literal
placeholder string, which can mislead readers into thinking it is interpolated
at runtime. Update the example near urlHeaders so it clearly shows a real token
source from the environment or otherwise indicates that the value is not
substituted automatically, and keep the fix centered on the target-mcp-url
example setup.
In `@runners/sdk/src/hunt.ts`:
- Around line 101-116: Serialize the environment-mutating SDK entry points: both
hunt() here and run() in runners/sdk/src/run.ts use withTempEnv/process.env
while async work is pending, so overlapping calls can clobber credentials. Fix
this by adding a single-flight/mutex around these paths, or clearly document in
hunt() and run() that concurrent invocations in the same process are not
supported; reference the hunt() wrapper and the runOnce() flow when applying the
guard.
In `@runners/sdk/src/internal/buildRunConfig.ts`:
- Around line 134-144: The inferProvider helper is silently defaulting unmatched
model names to "anthropic", which can mask misconfiguration. Update
inferProvider in buildRunConfig so it no longer returns a fallback provider for
unknown strings; instead, fail fast with a clear error telling the caller the
model is unrecognized and that provider must be specified explicitly. Make sure
the change is applied where inferProvider is used so the BuildRunConfig flow
does not continue with an incorrect provider/API key selection.
- Around line 5-10: The import in buildRunConfig should come from the owning
execute/types module instead of the package root barrel. Update the type-only
import in buildRunConfig to reference
`@keyvaluesystems/agent-opfor-core/execute/types.js` directly for RunConfig,
AgentTargetConfig, McpTargetConfig, and EvaluatorSelection so the file bypasses
the root re-export.
In `@runners/sdk/src/run.ts`:
- Around line 2-3: The imports in run.ts are inconsistent with the
direct-subpath rule: UnifiedRunReport is coming from the package barrel while
RunListener already uses a file-owned subpath. Update the import for
UnifiedRunReport to use its owning module path, matching the style used by
CoreRunListener and the pattern in buildRunConfig.ts, and avoid any barrel
re-export imports in this file.
In `@runners/sdk/src/types.ts`:
- Line 1: The local RunListener shape in types.ts and the manual mapping in
wrapListener should be protected against drift from CoreRunListener. Add a
lightweight compile-time type check near the RunListener definition or
wrapListener in runners/sdk/src/run.ts that asserts the wrapped listener remains
assignable to/from CoreRunListener, so any added or renamed lifecycle hook in
`@keyvaluesystems/agent-opfor-core` fails during compilation instead of silently
mismatching at runtime.
In `@tests/e2e/sdk/package.json`:
- Around line 12-17: The e2e package is depending on the internal SDK with a
loose caret range, which can allow npm to resolve a published registry version
instead of the local workspace build. Tighten the dependency in the package
manifest for the SDK entry used by the e2e tests so it always points to the
intended workspace version, and keep the exact package name consistent with the
`runners/sdk` source to avoid accidental stale resolution.
- Line 9: The test script in package.json is relying on shell glob expansion
because src/**/*.e2e.test.ts is unquoted. Update the test command so the Node
--test runner receives the glob directly, using the existing test script entry
as the place to fix it. This will ensure nested e2e tests under src continue to
be discovered correctly without depending on shell globstar behavior.
In `@tests/e2e/sdk/src/helpers/mockServer.ts`:
- Around line 89-90: Judge detection is currently tied to the exact system
prompt text in mockServer’s request routing, which is brittle. Update the mock’s
judge/attacker classification in the request handler to use a stable signal
instead of systemMsg.includes("security evaluator"), such as an explicit
marker/field on the payload or a dedicated request shape emitted by the caller.
Keep the change localized to the mockServer logic that decides isJudge so prompt
wording changes in the evaluator do not affect test routing.
🪄 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: CHILL
Plan: Pro
Run ID: ac12a643-b40f-4fc9-ae07-0a83f48170c8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (40)
.github/workflows/ci.yml.prettierignoredocs/sdk.mdpackage.jsonrunners/sdk/examples/README.mdrunners/sdk/examples/_helpers/mockLane.tsrunners/sdk/examples/_helpers/requireEnv.tsrunners/sdk/examples/agents/echo-agent.mjsrunners/sdk/examples/catalog-list.tsrunners/sdk/examples/ci-gate.tsrunners/sdk/examples/hunt-basic.tsrunners/sdk/examples/opfor-class.tsrunners/sdk/examples/report-json-html.tsrunners/sdk/examples/run-evaluators.functional.tsrunners/sdk/examples/run-suite.functional.tsrunners/sdk/examples/run-with-progress-and-listeners.tsrunners/sdk/examples/run-with-strategy.tsrunners/sdk/examples/target-http-json-paths.tsrunners/sdk/examples/target-http-openai-format.tsrunners/sdk/examples/target-http-stateful.tsrunners/sdk/examples/target-local-script.tsrunners/sdk/examples/target-mcp-stdio.tsrunners/sdk/examples/target-mcp-url.tsrunners/sdk/src/hunt.tsrunners/sdk/src/index.tsrunners/sdk/src/internal/buildRunConfig.tsrunners/sdk/src/opfor.tsrunners/sdk/src/run.tsrunners/sdk/src/types.tsrunners/sdk/tests/hunt.smoke.test.tsrunners/sdk/tests/hunt.test.tsrunners/sdk/tests/public-dts.test.tsrunners/sdk/tests/run.test.tsrunners/sdk/tests/tsconfig.jsonrunners/sdk/tests/types.test.tstests/e2e/sdk/README.mdtests/e2e/sdk/package.jsontests/e2e/sdk/src/helpers/mockServer.tstests/e2e/sdk/src/run.e2e.test.tstests/e2e/sdk/tsconfig.json
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/e2e/sdk/src/helpers/mockServer.ts (1)
60-114: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider adding
req/servererror handlers for resilience.No
errorlistener on the request stream or theserverinstance; an unexpected socket error would surface as an unhandled'error'event. Low risk in this sandboxed e2e helper, but worth a defensive addition if flakiness appears in CI.🤖 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 `@tests/e2e/sdk/src/helpers/mockServer.ts` around lines 60 - 114, Add defensive error handling in the mock server setup to avoid unhandled socket failures. In createServer’s request handler, add an error listener on req, and also attach an error listener to the server instance returned by createServer so unexpected stream or server errors are logged or safely ignored instead of crashing the e2e helper.runners/sdk/src/internal/envLock.ts (1)
1-24: 🧹 Nitpick | 🔵 TrivialGlobal lock serializes all
run()/hunt()calls process-wide.The single module-level
lastchain means any overlapping SDK calls (including acrossrun()andhunt()) execute strictly one-at-a-time, even against different targets. This is the intended safety mechanism for the temporaryprocess.envmutation, but it removes parallelism for users who fan out multiple runs. Consider documenting this behavior so consumers don't expect concurrent throughput from parallelPromise.all([run(...), run(...)]).🤖 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 `@runners/sdk/src/internal/envLock.ts` around lines 1 - 24, Document the process-wide serialization behavior in withEnvLock so users know the module-level last chain forces all overlapping run()/hunt() calls to execute one-at-a-time, even across different targets. Update the relevant SDK docs or function-level comments around withEnvLock, run(), and hunt() to explicitly mention that parallel Promise.all fan-out will be queued because of the temporary process.env mutation safety mechanism.
🤖 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.
Nitpick comments:
In `@runners/sdk/src/internal/envLock.ts`:
- Around line 1-24: Document the process-wide serialization behavior in
withEnvLock so users know the module-level last chain forces all overlapping
run()/hunt() calls to execute one-at-a-time, even across different targets.
Update the relevant SDK docs or function-level comments around withEnvLock,
run(), and hunt() to explicitly mention that parallel Promise.all fan-out will
be queued because of the temporary process.env mutation safety mechanism.
In `@tests/e2e/sdk/src/helpers/mockServer.ts`:
- Around line 60-114: Add defensive error handling in the mock server setup to
avoid unhandled socket failures. In createServer’s request handler, add an error
listener on req, and also attach an error listener to the server instance
returned by createServer so unexpected stream or server errors are logged or
safely ignored instead of crashing the e2e helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3f73f2d8-f0f0-432a-a52d-4704c5ca7709
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (44)
.github/workflows/ci.yml.prettierignoredocs/sdk.mdpackage.jsonrunners/sdk/examples/README.mdrunners/sdk/examples/_helpers/mockLane.tsrunners/sdk/examples/_helpers/requireEnv.tsrunners/sdk/examples/agents/echo-agent.mjsrunners/sdk/examples/catalog-list.tsrunners/sdk/examples/ci-gate.tsrunners/sdk/examples/hunt-basic.tsrunners/sdk/examples/opfor-class.tsrunners/sdk/examples/report-json-html.tsrunners/sdk/examples/run-evaluators.functional.tsrunners/sdk/examples/run-suite.functional.tsrunners/sdk/examples/run-suite.real.tsrunners/sdk/examples/run-with-progress-and-listeners.tsrunners/sdk/examples/run-with-strategy.tsrunners/sdk/examples/target-http-json-paths.tsrunners/sdk/examples/target-http-openai-format.tsrunners/sdk/examples/target-http-stateful.tsrunners/sdk/examples/target-local-script.tsrunners/sdk/examples/target-mcp-stdio.tsrunners/sdk/examples/target-mcp-url.tsrunners/sdk/package.jsonrunners/sdk/src/hunt.tsrunners/sdk/src/index.tsrunners/sdk/src/internal/buildRunConfig.tsrunners/sdk/src/internal/envLock.tsrunners/sdk/src/opfor.tsrunners/sdk/src/report.tsrunners/sdk/src/run.tsrunners/sdk/src/types.tsrunners/sdk/tests/hunt.smoke.test.tsrunners/sdk/tests/hunt.test.tsrunners/sdk/tests/public-dts.test.tsrunners/sdk/tests/run.test.tsrunners/sdk/tests/tsconfig.jsonrunners/sdk/tests/types.test.tstests/e2e/sdk/README.mdtests/e2e/sdk/package.jsontests/e2e/sdk/src/helpers/mockServer.tstests/e2e/sdk/src/run.e2e.test.tstests/e2e/sdk/tsconfig.json
✅ Files skipped from review due to trivial changes (4)
- runners/sdk/examples/target-local-script.ts
- runners/sdk/examples/report-json-html.ts
- .prettierignore
- runners/sdk/tests/hunt.test.ts
🚧 Files skipped from review as they are similar to previous changes (31)
- runners/sdk/examples/target-http-openai-format.ts
- tests/e2e/sdk/package.json
- runners/sdk/examples/target-mcp-url.ts
- runners/sdk/examples/catalog-list.ts
- runners/sdk/examples/hunt-basic.ts
- runners/sdk/examples/target-mcp-stdio.ts
- runners/sdk/examples/run-with-progress-and-listeners.ts
- runners/sdk/src/index.ts
- runners/sdk/examples/target-http-json-paths.ts
- runners/sdk/examples/opfor-class.ts
- runners/sdk/examples/target-http-stateful.ts
- runners/sdk/examples/run-evaluators.functional.ts
- runners/sdk/examples/_helpers/requireEnv.ts
- runners/sdk/tests/public-dts.test.ts
- tests/e2e/sdk/tsconfig.json
- runners/sdk/examples/run-with-strategy.ts
- runners/sdk/examples/run-suite.functional.ts
- runners/sdk/examples/ci-gate.ts
- .github/workflows/ci.yml
- runners/sdk/examples/_helpers/mockLane.ts
- runners/sdk/src/hunt.ts
- runners/sdk/src/opfor.ts
- runners/sdk/tests/hunt.smoke.test.ts
- runners/sdk/examples/agents/echo-agent.mjs
- runners/sdk/examples/README.md
- docs/sdk.md
- runners/sdk/tests/types.test.ts
- tests/e2e/sdk/src/run.e2e.test.ts
- package.json
- runners/sdk/src/internal/buildRunConfig.ts
- runners/sdk/tests/tsconfig.json
2 participants
Problem
Opfor’s SDK required API keys to be supplied indirectly via environment variables (and some examples/E2E relied on
OPFOR_API_KEY). This made programmatic usage awkward for real apps that manage secrets in code/config rather than global env var wiring.Solution
Make the SDK accept actual API key values directly in
run()options (models + HTTP targets). Internally, the SDK temporarily injects the appropriate provider env var only for the duration of the call to stay compatible with core’s model factory, without requiring the application to rely on env vars.Changes
ModelConfig: replaceapiKeyEnvwithapiKeyHttpTargetConfig: replaceapiKeyEnvwithapiKey(sent asAuthorization: Bearer ...)run(): sets/restores provider env vars during the call based onapiKeyinputsOpforclass: stops mutatingprocess.envin constructor; supports defaultbrainforhunt()docs/sdk.md,runners/sdk/examples/**, andtests/e2e/sdk/**to useapiKeyvalues.prettierignoreentry forrunners/sdk/.tsup/(generated)Issue
N/A
How to test
npm install npm run format npm run lint npm run typecheck npm test --workspace=runners/sdk npm run build npm run test:e2e:sdk(Optional) run a couple SDK examples:
Screenshots
N/A (tests + generated reports are produced by
report-json-html.tsif you run it).Summary by CodeRabbit
apiKeyconfiguration for targets and models.