feat: @kody/tools prebuilt library + executor/agent tests + admin Test tool UI - #8
Open
Chafficui wants to merge 13 commits into
Open
feat: @kody/tools prebuilt library + executor/agent tests + admin Test tool UI#8Chafficui wants to merge 13 commits into
Chafficui wants to merge 13 commits into
Conversation
Custom tools can now carry an HMAC signing secret, a bearer or api-key auth block (with optional env-var resolution), and a retry policy. The fields are all optional so existing configs keep parsing.
…ools Adds a new @kody/tools package providing a fluent Toolkit builder plus ready-made tools (httpGet, httpPost, webhook, slack, linear, sendgridEmail). The Toolkit bundles each tool's SiteConfig customTool entry alongside a runtime handler map. Handlers close over the auth/secrets the caller passes at construction time, so the agent loop never has to read process.env itself and tests can swap in a stub fetch.
Adds 13 executor tests (happy path, 500 error, timeout, invalid JSON, unknown tool, invalid arguments, knowledge_search, create_ticket, HMAC signing, bearer auth, env-var auth, transient retry, in-process registry dispatch) and 6 agent tests (plain stream, tool call + final answer, maxToolCalls, error propagation, provider onError, onToken ordering).
The server now depends on the @kody/tools workspace package so it can import the ToolRegistry and consume Toolkit exports at startup.
POST /api/admin/sites/:siteId/tools/:toolName/test runs a single custom tool with the JSON arguments from the request body and returns the structured ToolCallResult (ok, name, result, truncated, displayText, plus the tool's endpoint for the response viewer). Args are capped at 16 KB, results are truncated to 10 KB on the wire. The executor's internal cap is raised to 50 KB so this admin route is the authoritative truncation point — the agent loop can still rely on the executor never returning megabytes of upstream text to the LLM.
Each custom tool in the SiteEdit tools section now has a 'Test tool' button that opens a modal where an admin can paste JSON arguments, fire the call against the new admin endpoint, and inspect the result. UX: modal traps focus, supports Esc to close, exposes a Copy curl button for users who want to script it. The argument editor validates JSON in real time and the Run button stays disabled until the input parses. The response region is aria-live=polite so screen readers announce the outcome.
Covers the basic Toolkit flow, every pre-built factory, the auth / signing / retry knobs, and how to wire the in-process ToolRegistry into the server's ToolExecutor at startup.
Without this entry the new package's tests are silently skipped when running pnpm test from the repo root.
The default demo site previously had siteId "kody-website" with kody.codai.app in allowedOrigins and a stack of knowledge sources pointing to the hosted docs. None of that makes sense for a self- hosted install: - The siteId named the hosted fork, which is business-sensitive. - The allowedOrigins check would 403 the demo page on every localhost install. - The knowledge sources would render content the self-hoster has no relationship to. Replace with a localhost-only "demo" site: - siteId: "demo" (generic; no fork name leak). - allowedOrigins derived from env.PORT plus the same common dev ports as before. - Knowledge sources kept generic; the kody.codai.app URLs are removed. - Demo page at GET / now embeds the widget with data-site-id="demo". This is a no-op for self-hosters; the demo experience still works on http://localhost:3456 with Ollama. The hosted product is unaffected — it is a separate consumer of the @kody/server package and does not depend on the demo seed. UX note: a self-hoster cloning the repo and running pnpm install && pnpm dev now gets a working chat on http://localhost:3456 with zero hosted-only references in any seed or default config. They configure their own AI backend via /admin without any pre-seeded hosted knowledge.
📝 WalkthroughWalkthroughAdds the ChangesCustom tools workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SiteEdit
participant AdminAPI
participant ToolExecutor
participant CustomTool
SiteEdit->>AdminAPI: Submit tool arguments
AdminAPI->>ToolExecutor: Execute configured tool
ToolExecutor->>CustomTool: Send authenticated or signed request
CustomTool-->>ToolExecutor: Return tool payload
ToolExecutor-->>AdminAPI: Return structured result
AdminAPI-->>SiteEdit: Display result and truncation status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 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 `@packages/admin/src/components/ToolTester.tsx`:
- Around line 132-143: The curlExample useMemo must generate a command matching
tool.endpoint.method instead of hardcoding POST, and include every entry from
tool.endpoint.headers as escaped curl -H arguments. Preserve the existing JSON
body handling while ensuring the method, headers, and dependency list update
whenever the endpoint configuration changes.
In `@packages/admin/src/pages/SiteEdit.tsx`:
- Around line 1463-1470: Update the “Test tool” action in the custom-tool row to
prevent testing when the tool has no name, including a clear user-facing message
instead of navigating or invoking the test flow with an empty toolName; preserve
the existing behavior for named tools and locate the change around the onClick
handler that calls setTestingToolIdx.
In `@packages/server/src/app.ts`:
- Line 86: Update the admin route setup around createAdminToolsRouter to apply a
trusted-origin CORS allowlist instead of the reflected origin/credential
behavior, and enforce CSRF-token or bearer authentication on state-changing
admin endpoints. Keep non-admin API CORS behavior unchanged while ensuring
cookie-authenticated requests from untrusted same-site subdomains cannot execute
or read admin operations.
In `@packages/server/src/services/tools/executor.ts`:
- Around line 95-118: Update packages/server/src/services/tools/executor.ts
around ToolExecutor to use a shared executor/registry instance, dispatch
in-process handlers only when the tool is declared in config.tools.customTools,
and fail explicitly when such a tool uses the in-process marker instead of
falling through to fetch; update packages/tools/src/toolkit.ts around
Toolkit.export() to replace about:blank with an explicit non-fetchable marker
that ToolExecutor can recognize and assert.
- Around line 177-185: Move the resolveAuthValue call in executeCustomTool into
the existing error-handling path before authentication headers are applied, or
otherwise catch and convert its failure into the same structured tool failure
returned by execute. Preserve the never-throw contract of execute and ensure
misconfigured environment-based authentication cannot escape to runAgent or the
admin route.
- Line 105: Update the result construction in the tool executor to remove the
unreachable string check and serialize the ToolHandlerResult directly with
JSON.stringify(out).
- Around line 193-250: Update the retry loop around endpoint.method to attach
one stable idempotency key for the entire call, allowing receivers to
deduplicate repeated POST, PUT, and PATCH requests. Restrict AbortError timeout
retries to GET requests, while preserving applicable status retries. Narrow
isRetryableError so it does not treat every TypeError as retryable; only retry
confirmed transient network failures.
In `@packages/server/tests/integration/admin/tools.test.ts`:
- Around line 124-138: Add an integration test alongside the existing missing
and non-object argument cases that posts a valid object exceeding the tools
endpoint’s MAX_ARG_BYTES limit and asserts a 413 response. Reuse the established
request setup and endpoint symbols from the surrounding tests.
In `@packages/server/tests/unit/tools/agent.test.ts`:
- Line 1: Restore global fetch stubs after each test in this suite by importing
afterEach and registering afterEach(() => vi.unstubAllGlobals()) alongside the
existing Vitest imports in agent.test.ts. Keep the cleanup scoped to this test
file and preserve the current test behavior.
In `@packages/shared/src/validators/site-config.ts`:
- Around line 186-194: Redact write-only secrets from all admin site responses
by updating the response paths using listSites(), getSiteConfig(), createSite(),
and updateSite(). Strip endpoint.secret and auth.value before JSON
serialization, while preserving the existing toPublicConfig() behavior and
allowing these fields in write requests.
In `@packages/tools/package.json`:
- Around line 8-13: Reorder the conditions in the package export map so the
"types" condition appears before "import" under the "." entry, while preserving
both existing targets unchanged.
In `@packages/tools/src/http.ts`:
- Around line 122-133: Restrict the retry branches in the HTTP request flow to
idempotent methods by checking the request method before retrying transient
responses or retryable errors. Allow non-idempotent methods such as POST and
PATCH to return or propagate the initial result/error unless the caller provides
a provider-supported idempotency mechanism, using the existing request options
and retry helpers rather than changing unrelated behavior.
- Around line 72-76: Update the GET/DELETE query-parameter branch in the HTTP
helper to guard against a null body before calling Object.entries, treating null
as an empty query object while preserving the existing handling of defined
object bodies. Add a regression test covering null bodies for both GET and
DELETE and verify the structured-result contract remains intact.
- Around line 101-106: Add SSRF protection to the agent-driven httpGet/httpPost
flow before fetcher is called: resolve and reject non-public loopback, private,
link-local, and metadata destinations, and disable redirects or validate every
redirect target before following it. Keep URL syntax validation, but ensure
caller-supplied URLs cannot reach blocked networks through direct requests or
redirects.
In `@packages/tools/src/prebuilt.ts`:
- Around line 138-162: Update the httpPost handler to use the shared
httpArgsSchema, extending that schema with a required body: z.string() field,
instead of manually validating url and body. Extract or reuse shared buildUrl
and parseJsonObject helpers so httpPost and httpGet use identical path joining
and headers parsing, while preserving the existing POST body JSON parsing and
validation responses.
- Around line 81-88: Update the headers parsing in the prebuilt tool validation
flow and its corresponding logic around the second headers handling site to
validate the parsed JSON before assigning it to headersObj: require a non-null,
non-array plain object whose every value is a string, and return the existing
invalid-headers error for any other shape. Preserve valid string-valued header
objects.
- Around line 64-95: Restrict model-controlled URLs before any network call: add
shared validation for the resolved URL used by both the `handler`/GET flow and
`httpPost`, requiring the `https:` scheme and, when configured at construction
time, membership in the supplied host allowlist. Reject invalid URLs with the
existing structured error result and ensure validation occurs before `httpCall`.
In `@packages/tools/src/toolkit.ts`:
- Around line 105-115: Update addAs to detect collisions using both the
requested alias and tool.definition.function.name. Reject registration when the
incoming original function name is already represented by an existing entry
under a different alias, while preserving the current renamed definition and
successful registration behavior for unique names.
In `@packages/tools/tests/toolkit.test.ts`:
- Around line 100-168: Extend the “prebuilt factories” tests to cover linear()
and sendgridEmail(), including request construction and successful responses.
Add failure-path tests for malformed query, headers, and payload JSON, non-2xx
responses, and httpCall retry/backoff behavior for transient statuses, asserting
the final result and fetch-call/retry counts while preserving the existing
mockFetch setup.
🪄 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 Plus
Run ID: c95a1b05-b3e0-44f7-9612-2fbedff492b7
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
packages/admin/src/components/ToolTester.tsxpackages/admin/src/lib/api.tspackages/admin/src/pages/SiteEdit.tsxpackages/server/package.jsonpackages/server/src/app.tspackages/server/src/index.tspackages/server/src/routes/admin/tools.tspackages/server/src/services/agent.tspackages/server/src/services/tools/executor.tspackages/server/tests/integration/admin/tools.test.tspackages/server/tests/unit/tools/agent.test.tspackages/server/tests/unit/tools/executor.test.tspackages/shared/src/validators/index.tspackages/shared/src/validators/site-config.tspackages/tools/README.mdpackages/tools/package.jsonpackages/tools/src/http.tspackages/tools/src/index.tspackages/tools/src/prebuilt.tspackages/tools/src/toolkit.tspackages/tools/src/types.tspackages/tools/tests/http.test.tspackages/tools/tests/toolkit.test.tspackages/tools/tsconfig.jsonpackages/tools/vitest.config.tsvitest.config.ts
Comment on lines
+64
to
+95
| handler: async (args: Record<string, unknown>) => { | ||
| const parsed = httpArgsSchema.safeParse(args); | ||
| if (!parsed.success) { | ||
| return { ok: false, message: `Invalid args: ${parsed.error.message}` }; | ||
| } | ||
| const fullUrl = parsed.data.path | ||
| ? `${parsed.data.url.replace(/\/+$/, "")}/${parsed.data.path.replace(/^\/+/, "")}` | ||
| : parsed.data.url; | ||
|
|
||
| let queryObj: Record<string, unknown> | undefined; | ||
| if (parsed.data.query) { | ||
| try { | ||
| queryObj = JSON.parse(parsed.data.query); | ||
| } catch { | ||
| return { ok: false, message: "query must be a valid JSON object string" }; | ||
| } | ||
| } | ||
| let headersObj: Record<string, string> | undefined; | ||
| if (parsed.data.headers) { | ||
| try { | ||
| headersObj = JSON.parse(parsed.data.headers); | ||
| } catch { | ||
| return { ok: false, message: "headers must be a valid JSON object string" }; | ||
| } | ||
| } | ||
|
|
||
| const result = await httpCall({ | ||
| url: fullUrl, | ||
| method: "GET", | ||
| headers: headersObj, | ||
| body: queryObj, | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
SSRF (CWE-918): Server-Side Request Forgery (SSRF)
Reachability: External
Agent-controlled url reaches fetch with no scheme/host restrictions (SSRF).
z.string().url() accepts http://169.254.169.254/..., http://localhost:5432, and non-HTTP schemes. Since the argument comes from model output, a prompt-injected page or user turn can drive requests to internal services and return the response body to the caller. Gate the resolved URL on an allowlist (scheme https: plus an optional host allowlist supplied at construction time) before calling httpCall.
Same sink exists in httpPost (Line 158-162), which does not validate the URL at all.
🛡️ Sketch of a scheme/host guard
+const ALLOWED_PROTOCOLS = new Set(["https:"]);
+
+function assertAllowedUrl(raw: string): string | null {
+ try {
+ const u = new URL(raw);
+ return ALLOWED_PROTOCOLS.has(u.protocol) ? u.toString() : null;
+ } catch {
+ return null;
+ }
+} const fullUrl = parsed.data.path
? `${parsed.data.url.replace(/\/+$/, "")}/${parsed.data.path.replace(/^\/+/, "")}`
: parsed.data.url;
+ if (!assertAllowedUrl(fullUrl)) {
+ return { ok: false, message: "url must be an absolute https URL" };
+ }🤖 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 `@packages/tools/src/prebuilt.ts` around lines 64 - 95, Restrict
model-controlled URLs before any network call: add shared validation for the
resolved URL used by both the `handler`/GET flow and `httpPost`, requiring the
`https:` scheme and, when configured at construction time, membership in the
supplied host allowlist. Reject invalid URLs with the existing structured error
result and ensure validation occurs before `httpCall`.
Comment on lines
+105
to
+115
| addAs(name: string, tool: Tool): this { | ||
| if (this.entries.some((e) => e.name === name)) { | ||
| throw new Error(`Tool "${name}" is already registered in this toolkit`); | ||
| } | ||
| const renamed: Tool = { | ||
| definition: { ...tool.definition, function: { ...tool.definition.function, name } }, | ||
| handler: tool.handler, | ||
| }; | ||
| this.entries.push({ name, definition: renamed.definition, handler: renamed.handler }); | ||
| return this; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
addAs only checks the new name for collisions.
add(webhook(url)) followed by addAs("webhook", slack(...)) throws, but the reverse order is fine — yet export() then produces two entries whose definition.function.name differ from what callers expect. Cheap to make consistent by also rejecting when tool.definition.function.name is already present under a different alias.
🤖 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 `@packages/tools/src/toolkit.ts` around lines 105 - 115, Update addAs to detect
collisions using both the requested alias and tool.definition.function.name.
Reject registration when the incoming original function name is already
represented by an existing entry under a different alias, while preserving the
current renamed definition and successful registration behavior for unique
names.
…it collisions httpCall: resolve the URL hostname and reject loopback / private / link-local / cloud-metadata destinations before any network call; allow a caller-supplied allowlist. Disable automatic redirect following so redirect targets can be SSRF-checked. Restrict retries to idempotent methods (GET/HEAD/OPTIONS/PUT/ DELETE); for POST/PATCH, retries are only attempted when the caller supplies an idempotency key, which is attached to every attempt. Timeouts on non-GET requests are no longer retried (the server may have already accepted the side effect). Narrow isRetryable to the errno codes that represent transient network failures instead of treating every TypeError as retryable. prebuilt: extract buildUrl + parseJsonObject helpers and route both httpGet and httpPost through a single runHttpCall, using the shared httpArgsSchema. Validate parsed headers as a plain object whose values are all strings and return the existing invalid-headers error otherwise. Re-throw httpCall SSRF errors as structured handler failures so the executor's never-throw contract holds for agent-driven URLs. toolkit: addAs and add now detect collisions on BOTH the requested alias and the incoming tool's original function name, so the order in which tools are registered cannot silently produce two entries pointing at the same function name. Replace the about:blank placeholder with a non-fetchable kody://inproc/__kody_internal__ marker the server executor recognises as the in-process dispatch signal. Reorder the package export map so the 'types' condition precedes 'import' to match Node's documented precedence and avoid bundler ambiguity. UX: no user-visible behaviour change beyond the agent now receiving a clear, structured error when a request targets a private or loopback host. Hosting-leak guardrail: drop in the shared check-hosting-leaks.mjs script under scripts/ so the public repo keeps enforcing the no-hosted-vendor-keys / no-fork-name policy on every PR.
…, CSRF-safe admin executor: in-process dispatch now requires the tool to also be declared in config.tools.customTools with the new kody://inproc/__kody_internal__ marker URL, so a registered handler that was never advertised to the agent fails explicitly instead of silently executing. Move resolveAuthValue into the existing try/catch so a missing env-var auth becomes a structured 'Tool execution failed (auth)' failure that respects the never-throw contract. Attach a stable random Idempotency-Key header for the entire retry loop when the call is actually retried (idempotent method or retry configured) so the receiver can deduplicate. Restrict timeout retries to GET — POST/PUT/PATCH may have already landed server-side. Narrow isRetryableError to known transient errno codes instead of treating every TypeError as retryable. Remove the unreachable string check in the in-process branch — the handler always returns a ToolHandlerResult object. shared: add redactConfigSecrets(siteConfig) which returns a shallow clone of the SiteConfig with endpoint.secret and auth.value stripped to empty strings. The admin sites router applies it before JSON-serialising every list/get/create/update response so write-only secrets never round-trip through the admin UI. toPublicConfig keeps its existing behaviour for the public widget endpoint; write requests still accept fresh secrets. server: replace the global CORS reflection middleware in app.ts with a short-circuit on OPTIONS preflight and no Access-Control-Allow-Origin header. The public widget API already enforces a per-site allowedOrigins allowlist in siteAuth; the admin API now requires a Bearer token for every state-changing method (POST/PUT/PATCH/DELETE) — cookie auth is retained only for read-only methods, so the React admin SPA continues to work via its existing Authorization header while cross-origin attackers can no longer ride the kody_session cookie. UX: admin UI behaviour unchanged; the React SPA already sends Bearer on every request. The clear 'auth' / 'not declared' error messages surface in the ToolTester response region without any layout shift. tests: extend executor.test.ts with a positive in-process dispatch (declared), a negative in-process dispatch (no declaration, must fail), an idempotency-key attachment check for non-idempotent retries, and an auth env-var-missing failure path.
…s empty tool name ToolTester: the 'Copy as curl' snippet now uses tool.endpoint.method (defaulting to POST) and iterates every entry in tool.endpoint.headers, rendering each as a -H 'Name: value' line. The body is shell-escaped via a POSIX-safe single-quote transform so embedded quotes don't break the command. Dependency list updated to include tool.endpoint so the snippet recomputes whenever the configured method or headers change. SiteEdit: pressing 'Test tool' on a row whose name is empty (or only whitespace) now shows a clear, accessible 'Tool #N needs a name before you can test it' alert above the list and does NOT open the ToolTester modal. The error clears when the dialog closes or when a subsequent test-tool action succeeds. UX: the alert uses role='alert' + aria-live='polite' so screen readers announce the validation message immediately without grabbing focus from the form. The ToolTester curl snippet is now identical to what the backend actually receives, so admins copying it can paste it into a terminal without manually editing the method or adding headers.
…leanup tools: extend prebuilt factories tests to cover linear() and sendgridEmail() on both the success path (request construction, headers, auth) and the failure path (missing required fields, GraphQL errors, non-2xx upstream). Add failure-path coverage for malformed query / headers / payload JSON and for non-2xx upstream responses on the agent-driven HTTP tools. Pin httpCall's retry behaviour: GET is retried on transient 503, POST is NOT retried without an idempotency key, and POST IS retried with one — the same Idempotency-Key is attached to every attempt so a downstream receiver can dedupe. Add a SSRF-protection test that hammers a representative set of private / loopback / link-local / cloud-metadata / non-http destinations and asserts every call is rejected before fetch fires. Add a positive test that the low-level httpCall helper honours an explicit allowlist when one is supplied. toolkit: pin the new collision-detection behaviour — add() and addAs() both reject when the incoming tool's original function name is already registered under a different alias. Update the existing 'export() returns customTools compatible with SiteConfig' assertion to use the new INPROC_TOOL_MARKER instead of about:blank. http: add null-body regression tests for both GET and DELETE, plus an undefined-body test for GET, to guard the Object.entries guard. server: add a 413 test alongside the existing missing / non-object argument cases — sends a 20 KB payload and asserts the response status plus the 'too large' error message. server agent: add an afterEach(() => vi.unstubAllGlobals()) inside the runAgent suite so the global fetch stub installed per test does not leak into the next test file that runs in the same worker.
7 tasks
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/admin/src/pages/SiteEdit.tsx (1)
1490-1500: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemoving a tool while the tester is open can switch the tester to a different tool.
testingToolIdxholds a positional index. The remove handler splicescustomToolswithout resetting it. If the user removes a row above the tool under test, the index now points at the following tool, andToolTesterat Line 1859 silently renders that tool instead.🐛 Proposed fix
onClick={() => { const customTools = [...form.tools.customTools]; customTools.splice(tIdx, 1); updateTools("customTools", customTools); + setTestingToolIdx(null); + setToolTestError(null); }}🤖 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 `@packages/admin/src/pages/SiteEdit.tsx` around lines 1490 - 1500, Update the custom tool removal handler near the “Remove tool” button to reset testingToolIdx when the removed tool is currently being tested, and adjust the index when removing an item before the active tester index so it continues targeting the same tool. Keep the existing customTools splice and updateTools behavior unchanged.packages/server/src/app.ts (1)
55-74: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy liftImplement allowlist-based CORS for widget APIs.
The blanket
OPTIONSresponse blocks the widget’s JSON requests because their preflight receives no CORS headers.siteAuthalso validates origins but does not setAccess-Control-Allow-Originon actual responses. Handle allowed widget preflights and responses without reflecting origins or enabling credentials.🤖 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 `@packages/server/src/app.ts` around lines 55 - 74, Update the CORS handling around the OPTIONS middleware and siteAuth flow so widget API routes (/api/chat, /api/tickets, /api/sessions, and /api/feedback) validate the request Origin against the siteAuth allowlist and return Access-Control-Allow-Origin only for allowed origins. Add matching headers for allowed widget preflight requests, including permitted methods and headers, while preserving the no-credentials and no-origin-reflection behavior; ensure actual allowed widget responses also receive the origin header.packages/server/src/services/tools/executor.ts (1)
254-264: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winOmit the request body for
GETendpoints
GETis permitted by the endpoint schema, butfetchrejects the unconditionalbody: bodyTextbefore sending the request.isRetryableErrordoes not retry thisTypeError.For
GET, append the serialized arguments to the URL and omit the body. Ifendpoint.secretis set, sign the transmitted query representation instead ofbodyText.HEADis not permitted by the current schema.🤖 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 `@packages/server/src/services/tools/executor.ts` around lines 254 - 264, The request flow in the retry loop must handle GET endpoints without a body. Update the code around the fetch call to append serialized arguments to the URL for GET, omit the body, and sign the transmitted query representation when endpoint.secret is set; preserve body transmission and signing for non-GET methods.
🤖 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 `@packages/admin/src/components/ToolTester.tsx`:
- Around line 142-145: The generated curl command in ToolTester includes
unredacted endpoint.headers values. Before displaying or copying the command,
add an explicit warning that configured header values are included, covering
both the rendered command and clipboard content while preserving the existing
command generation.
In `@packages/server/src/middleware/admin-auth.ts`:
- Around line 44-48: Update the cookie extraction in the admin authentication
middleware around cookieHeader and the kody_session match so it only matches
kody_session at the start of the header or immediately after a cookie separator.
Preserve extraction of the token value while preventing similarly named cookies
such as x_kody_session from being selected.
- Around line 27-29: Update the bearerToken extraction near authHeader so the
Authorization scheme comparison is case-insensitive and the extracted token is
trimmed. Preserve undefined for missing or non-Bearer headers, while accepting
casing variations such as “bearer”.
In `@packages/server/src/services/tools/executor.ts`:
- Around line 266-267: Replace the full-body response.text() call in the tool
execution flow with bounded streaming from response.body, accumulating data only
up to MAX_TOOL_RESPONSE_BYTES and stopping or aborting once the limit is
reached. Preserve the existing capped response handling around the text result
and ensure the timer cleanup in the response-processing path remains intact,
including for chunked or unknown-length responses.
- Around line 202-210: Update the retry logic around effectiveMax and the
AbortError handling to remove the unreachable idempotencyKey-based branches.
Preserve the existing single-attempt behavior for non-idempotent calls, and
enforce the stated safety rule by allowing AbortError retries only for GET
requests or an explicit retry opt-in, using the surrounding retry-loop symbols.
- Around line 217-246: Update the HMAC signing logic around createHmac and
baseHeaders to add a timestamp header and sign a canonical string containing the
timestamp, Idempotency-Key, and bodyText, preserving consistent handling when
the idempotency key is absent. Document the exact signed-string format and
verification requirements in the package README so receivers can reproduce the
signature.
In `@packages/server/tests/unit/tools/executor.test.ts`:
- Around line 316-318: In the test assertion around mockFetch.mock.calls, first
assert that mockFetch was called before destructuring mockFetch.mock.calls[0].
Keep the existing header type and non-empty Idempotency-Key assertions after
this call-existence check so early executor returns produce the intended test
failure.
- Around line 279-284: Update the test endpoint in the executor test to use the
imported INPROC_TOOL_MARKER from `@kody/tools` instead of the hardcoded
"kody://inproc/__kody_internal__" URL value, adding the import alongside the
existing imports. Preserve the endpoint structure and other test values
unchanged.
In `@packages/shared/src/validators/site-config.ts`:
- Around line 353-370: Update SiteStore.updateSite to merge each incoming custom
tool with its existing stored endpoint credentials when secret or auth is
omitted. Adjust redactConfigSecrets so redacted credential fields are omitted
rather than emitted as empty values, while preserving explicitly supplied
non-secret tool fields and existing credentials.
In `@packages/tools/src/http.ts`:
- Around line 166-178: Update assertSafeTarget and the subsequent fetch path so
the connection uses only the addresses validated by the initial DNS lookup,
preventing a second untrusted hostname resolution. Prefer passing a custom
undici dispatcher with a lookup implementation restricted to the validated
addresses while preserving the original hostname in the Host header; otherwise
connect directly to a validated address and retain the original host header.
- Around line 123-133: Update isBlockedHost in packages/tools/src/http.ts (lines
123-133) to remove surrounding brackets from IPv6 hostnames before net.isIP, and
extend isPrivateOrLoopbackIPv6 to recognize hexadecimal IPv4-mapped addresses.
Tighten the assertions in packages/tools/tests/toolkit.test.ts (lines 208-225)
for http://[::1]/x and http://[fc00::1]/x to require their specific rejection
reasons rather than the broad alternation.
- Around line 290-296: Update the abort handling condition in the request retry
flow to use the existing methodIsIdempotent result instead of checking method
!== "GET". Preserve the idempotencyKey exception so non-idempotent requests
without a key still return the status-0 network error, while idempotent methods
such as PUT and DELETE remain eligible for configured retries.
In `@packages/tools/src/prebuilt.ts`:
- Around line 119-159: Update the HTTP request flow around queryObj and httpCall
so the validated query parameters are merged into the request URL, while
preserving any existing URL query parameters and explicit body-derived
parameters for GET and DELETE. Ensure query values reach fetch, and add a test
covering that the supplied query string is included in the fetched URL.
In `@packages/tools/tests/http.test.ts`:
- Around line 113-135: Update the URL expectations in the three httpCall
regression tests for null or undefined bodies to assert “https://x.test” without
a trailing slash, while preserving the existing query-parameter and DELETE
method assertions.
In `@packages/tools/tests/toolkit.test.ts`:
- Around line 227-239: Update the test around httpCall to use a loopback or
otherwise blocklisted URL so it would be rejected without the allowedHosts
bypass, while retaining an allowlist entry that permits that host. Keep the
existing successful mock response and assertion, ensuring the test specifically
verifies that allowedHosts skips the private-address safety check.
In `@scripts/check-hosting-leaks.mjs`:
- Around line 36-45: Update the FORBIDDEN list in check-hosting-leaks to add a
pattern that rejects the documented kody-website name everywhere, including
README.md. Use the same entry structure as the existing hostname and secret
patterns, while preserving all current forbidden checks.
- Around line 204-209: Update the scan loop around the candidates filter and the
existing explicit-file list so explicitly listed top-level files bypass the
SCAN_EXTENSIONS check. Use the same identifying path or filename set that adds
those files, ensuring Dockerfile.server, .env.example, and .env are scanned
while retaining extension filtering for other candidates.
---
Outside diff comments:
In `@packages/admin/src/pages/SiteEdit.tsx`:
- Around line 1490-1500: Update the custom tool removal handler near the “Remove
tool” button to reset testingToolIdx when the removed tool is currently being
tested, and adjust the index when removing an item before the active tester
index so it continues targeting the same tool. Keep the existing customTools
splice and updateTools behavior unchanged.
In `@packages/server/src/app.ts`:
- Around line 55-74: Update the CORS handling around the OPTIONS middleware and
siteAuth flow so widget API routes (/api/chat, /api/tickets, /api/sessions, and
/api/feedback) validate the request Origin against the siteAuth allowlist and
return Access-Control-Allow-Origin only for allowed origins. Add matching
headers for allowed widget preflight requests, including permitted methods and
headers, while preserving the no-credentials and no-origin-reflection behavior;
ensure actual allowed widget responses also receive the origin header.
In `@packages/server/src/services/tools/executor.ts`:
- Around line 254-264: The request flow in the retry loop must handle GET
endpoints without a body. Update the code around the fetch call to append
serialized arguments to the URL for GET, omit the body, and sign the transmitted
query representation when endpoint.secret is set; preserve body transmission and
signing for non-GET methods.
🪄 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 Plus
Run ID: 628695a1-74a4-4ca3-a308-6345b44af0c7
📒 Files selected for processing (19)
packages/admin/src/components/ToolTester.tsxpackages/admin/src/pages/SiteEdit.tsxpackages/server/src/app.tspackages/server/src/middleware/admin-auth.tspackages/server/src/routes/admin/sites.tspackages/server/src/services/tools/executor.tspackages/server/tests/integration/admin/tools.test.tspackages/server/tests/unit/tools/agent.test.tspackages/server/tests/unit/tools/executor.test.tspackages/shared/src/validators/index.tspackages/shared/src/validators/site-config.tspackages/tools/package.jsonpackages/tools/src/http.tspackages/tools/src/index.tspackages/tools/src/prebuilt.tspackages/tools/src/toolkit.tspackages/tools/tests/http.test.tspackages/tools/tests/toolkit.test.tsscripts/check-hosting-leaks.mjs
Comment on lines
+142
to
+145
| for (const [k, v] of Object.entries(tool.endpoint.headers ?? {})) { | ||
| if (typeof v !== "string") continue; | ||
| headerLines.push(` -H ${shq(`${k}: ${v}`)}`); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C6 'redact|REDACT|\*\*\*' packages/server/src/routes/admin/sites.ts
rg -n -C4 'endpoint' packages/server/src/routes/admin/sites.tsRepository: Chafficui/kody
Length of output: 2253
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- redaction helper definitions and usages ---'
rg -n -C8 'redactConfigSecrets|function fetchSite|fetchSite|headers' packages --glob '*.ts' --glob '*.tsx' | head -n 260
printf '%s\n' '--- candidate files ---'
fd -i 'redact|site|ToolTester' packagesRepository: Chafficui/kody
Length of output: 18694
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- redaction helper ---'
rg -n -C12 'redactConfigSecrets' packages/shared packages/server packages/admin --glob '*.ts' --glob '*.tsx'
printf '%s\n' '--- site configuration types and tests ---'
rg -n -C8 'headers|redact|REDACT|placeholder|secret' packages/shared packages/server/tests packages/admin --glob '*.ts' --glob '*.tsx' | head -n 320
printf '%s\n' '--- relevant component and form flow ---'
cat -n packages/admin/src/components/ToolTester.tsx | sed -n '120,160p'
cat -n packages/admin/src/pages/SiteEdit.tsx | sed -n '170,215p'Repository: Chafficui/kody
Length of output: 38134
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- complete redaction implementation ---'
cat -n packages/shared/src/validators/site-config.ts | sed -n '335,385p'
printf '%s\n' '--- endpoint schema and secret-related fields ---'
rg -n -C12 'endpointSchema|headers:|secret:|auth:' packages/shared/src/validators/site-config.ts
printf '%s\n' '--- admin response redaction tests ---'
rg -n -C12 'redactConfigSecrets|headers|secret|auth.value' packages/server/tests/integration/admin/sites.test.ts packages/shared/tests/site-config.test.tsRepository: Chafficui/kody
Length of output: 6986
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
helper = Path("packages/shared/src/validators/site-config.ts").read_text()
routes = Path("packages/server/src/routes/admin/sites.ts").read_text()
component = Path("packages/admin/src/components/ToolTester.tsx").read_text()
match = re.search(
r"export function redactConfigSecrets\(config: SiteConfig\): SiteConfig \{.*?\n\}",
helper,
re.S,
)
assert match, "redactConfigSecrets was not found"
body = match.group(0)
assert "headers:" not in body, "The helper assigns a redacted headers value"
assert "secret: tool.endpoint.secret ? \"\"" in body
assert "value: \"\"" in body
assert routes.count("redactConfigSecrets(") >= 4
assert "Object.entries(tool.endpoint.headers ?? {})" in component
assert "navigator.clipboard.writeText(text)" in component
print("redactConfigSecrets clears endpoint.secret and endpoint.auth.value only")
print("admin site responses pass through redactConfigSecrets")
print("ToolTester renders endpoint.headers and can copy the generated command")
PYRepository: Chafficui/kody
Length of output: 349
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
helper = Path("packages/shared/src/validators/site-config.ts").read_text()
routes = Path("packages/server/src/routes/admin/sites.ts").read_text()
component = Path("packages/admin/src/components/ToolTester.tsx").read_text()
match = re.search(
r"export function redactConfigSecrets\(config: SiteConfig\): SiteConfig \{.*?\n\}",
helper,
re.S,
)
assert match, "redactConfigSecrets was not found"
body = match.group(0)
assert "headers:" not in body, "The helper assigns a redacted headers value"
assert 'secret: tool.endpoint.secret ? ""' in body
assert 'value: ""' in body
assert routes.count("redactConfigSecrets(") >= 4
assert "Object.entries(tool.endpoint.headers ?? {})" in component
assert "navigator.clipboard.writeText(text)" in component
print("redactConfigSecrets clears endpoint.secret and endpoint.auth.value only")
print("admin site responses pass through redactConfigSecrets")
print("ToolTester renders endpoint.headers and can copy the generated command")
PYRepository: Chafficui/kody
Length of output: 349
State that generated curl commands include configured header values.
redactConfigSecrets clears only endpoint.secret and endpoint.auth.value. endpoint.headers remains unchanged. ToolTester includes these values in the generated command and clipboard content. Add an explicit warning before displaying or copying the command.
🤖 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 `@packages/admin/src/components/ToolTester.tsx` around lines 142 - 145, The
generated curl command in ToolTester includes unredacted endpoint.headers
values. Before displaying or copying the command, add an explicit warning that
configured header values are included, covering both the rendered command and
clipboard content while preserving the existing command generation.
Comment on lines
+27
to
+29
| const authHeader = req.headers.authorization; | ||
| const bearerToken = | ||
| authHeader?.startsWith("Bearer ") ? authHeader.slice(7) : undefined; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The Bearer scheme comparison is case-sensitive.
RFC 7235 defines the authentication scheme token as case-insensitive. A client that sends Authorization: bearer <token> receives 401 on every state-changing admin request. Compare the scheme case-insensitively and trim the token.
🐛 Proposed fix
const authHeader = req.headers.authorization;
- const bearerToken =
- authHeader?.startsWith("Bearer ") ? authHeader.slice(7) : undefined;
+ const bearerMatch = authHeader?.match(/^Bearer\s+(.+)$/i);
+ const bearerToken = bearerMatch?.[1].trim() || undefined;📝 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 authHeader = req.headers.authorization; | |
| const bearerToken = | |
| authHeader?.startsWith("Bearer ") ? authHeader.slice(7) : undefined; | |
| const authHeader = req.headers.authorization; | |
| const bearerMatch = authHeader?.match(/^Bearer\s+(.+)$/i); | |
| const bearerToken = bearerMatch?.[1].trim() || undefined; |
🤖 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 `@packages/server/src/middleware/admin-auth.ts` around lines 27 - 29, Update
the bearerToken extraction near authHeader so the Authorization scheme
comparison is case-insensitive and the extracted token is trimmed. Preserve
undefined for missing or non-Bearer headers, while accepting casing variations
such as “bearer”.
Comment on lines
+44
to
48
| const cookieHeader = req.headers.cookie; | ||
| if (cookieHeader) { | ||
| const match = cookieHeader.match(/kody_session=([^;]+)/); | ||
| token = match?.[1]; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
The cookie regex is not anchored to a cookie boundary.
/kody_session=([^;]+)/ matches inside any cookie name that ends with kody_session. A cookie header of x_kody_session=attacker; kody_session=real yields attacker, because the regex matches the first occurrence. An attacker who can set a cookie on the domain, for example from a subdomain, can therefore override which token the middleware validates. If the attacker plants a token from a session they control, the admin operates under that session.
Anchor the match to the start of the header or to a ; separator, or parse the header with cookie-parser.
🔒 Proposed fix
const cookieHeader = req.headers.cookie;
if (cookieHeader) {
- const match = cookieHeader.match(/kody_session=([^;]+)/);
+ const match = cookieHeader.match(/(?:^|;\s*)kody_session=([^;]+)/);
token = match?.[1];
}📝 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 cookieHeader = req.headers.cookie; | |
| if (cookieHeader) { | |
| const match = cookieHeader.match(/kody_session=([^;]+)/); | |
| token = match?.[1]; | |
| } | |
| const cookieHeader = req.headers.cookie; | |
| if (cookieHeader) { | |
| const match = cookieHeader.match(/(?:^|;\s*)kody_session=([^;]+)/); | |
| token = match?.[1]; | |
| } |
🤖 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 `@packages/server/src/middleware/admin-auth.ts` around lines 44 - 48, Update
the cookie extraction in the admin authentication middleware around cookieHeader
and the kody_session match so it only matches kody_session at the start of the
header or immediately after a cookie separator. Preserve extraction of the token
value while preventing similarly named cookies such as x_kody_session from being
selected.
Comment on lines
+202
to
+210
| const methodIsIdempotent = IDEMPOTENT_METHODS.has(endpoint.method); | ||
| const maxAttempts = endpoint.retry?.maxAttempts ?? 1; | ||
| const baseDelay = endpoint.retry?.baseDelayMs ?? 250; | ||
| // Stable idempotency key for the whole call so receivers can dedupe | ||
| // retried POST/PUT/PATCH requests. Only generated when the call may | ||
| // actually be retried (idempotent method or explicit retry config). | ||
| const idempotencyKey = | ||
| methodIsIdempotent || (maxAttempts > 1) ? randomUUID() : undefined; | ||
| const effectiveMax = methodIsIdempotent || idempotencyKey ? maxAttempts : 1; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
effectiveMax and the abort guard contain unreachable logic.
idempotencyKey is defined whenever methodIsIdempotent is true or maxAttempts > 1. Therefore:
- Line 210:
methodIsIdempotent || idempotencyKey ? maxAttempts : 1yieldsmaxAttemptsin every case wheremaxAttempts > 1, and yields1only whenmaxAttemptsis already1. The ternary has no effect. - Line 302:
!idempotencyKeyis true only whenmaxAttempts === 1and the method is not idempotent. In that case the loop cannot retry anyway, and the fall-through at Line 315 produces the sametimeoutlabel.
The stated intent, "AbortError retries are only safe for GET", is not enforced. Either drop the dead branches, or gate timeout retries on an explicit opt-in.
♻️ Simplification
- const idempotencyKey =
- methodIsIdempotent || (maxAttempts > 1) ? randomUUID() : undefined;
- const effectiveMax = methodIsIdempotent || idempotencyKey ? maxAttempts : 1;
+ // A key is only useful when the request may actually be replayed.
+ const idempotencyKey = maxAttempts > 1 ? randomUUID() : undefined;
+ const effectiveMax = maxAttempts; const isAbort = err instanceof Error && err.name === "AbortError";
- if (isAbort && endpoint.method !== "GET" && !idempotencyKey) {
- return { ... };
- }Also applies to: 298-310
🤖 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 `@packages/server/src/services/tools/executor.ts` around lines 202 - 210,
Update the retry logic around effectiveMax and the AbortError handling to remove
the unreachable idempotencyKey-based branches. Preserve the existing
single-attempt behavior for non-idempotent calls, and enforce the stated safety
rule by allowing AbortError retries only for GET requests or an explicit retry
opt-in, using the surrounding retry-loop symbols.
Comment on lines
+217
to
+246
| const baseHeaders: Record<string, string> = { | ||
| "Content-Type": "application/json", | ||
| ...endpoint.headers, | ||
| }; | ||
| if (idempotencyKey) baseHeaders["Idempotency-Key"] = idempotencyKey; | ||
| if (endpoint.secret) { | ||
| baseHeaders["X-Kody-Signature"] = createHmac("sha256", endpoint.secret) | ||
| .update(bodyText) | ||
| .digest("hex"); | ||
| } | ||
| if (endpoint.auth) { | ||
| try { | ||
| const value = resolveAuthValue(endpoint.auth.value, endpoint.auth.fromEnv ?? false); | ||
| if (endpoint.auth.type === "bearer") { | ||
| baseHeaders["Authorization"] = `Bearer ${value}`; | ||
| } else { | ||
| const headerName = endpoint.auth.headerName ?? "Authorization"; | ||
| baseHeaders[headerName] = value; | ||
| } | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : "Auth configuration error"; | ||
| return { | ||
| toolCallId: callId, | ||
| name: tool.name, | ||
| result: `Tool returned error ${response.status}: ${text.slice(0, 500)}`, | ||
| ok: false, | ||
| result: `Tool execution failed (auth): ${message}`, | ||
| displayText: tool.description.slice(0, 50), | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
The HMAC signature covers only the body, so receivers cannot detect replay or header tampering.
X-Kody-Signature is computed over bodyText. The signature omits a timestamp and omits Idempotency-Key. A captured request stays valid forever, and an attacker on the path can change the idempotency key without invalidating the signature. That defeats the deduplication the key is meant to provide.
Add a timestamp header and include it, plus the idempotency key, in the signed string.
🔒 Proposed signing scheme
if (endpoint.secret) {
- baseHeaders["X-Kody-Signature"] = createHmac("sha256", endpoint.secret)
- .update(bodyText)
- .digest("hex");
+ const timestamp = Math.floor(Date.now() / 1000).toString();
+ baseHeaders["X-Kody-Timestamp"] = timestamp;
+ baseHeaders["X-Kody-Signature"] = createHmac("sha256", endpoint.secret)
+ .update(`${timestamp}.${idempotencyKey ?? ""}.${bodyText}`)
+ .digest("hex");
}Document the signed string format in the package README so receivers can verify it.
📝 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 baseHeaders: Record<string, string> = { | |
| "Content-Type": "application/json", | |
| ...endpoint.headers, | |
| }; | |
| if (idempotencyKey) baseHeaders["Idempotency-Key"] = idempotencyKey; | |
| if (endpoint.secret) { | |
| baseHeaders["X-Kody-Signature"] = createHmac("sha256", endpoint.secret) | |
| .update(bodyText) | |
| .digest("hex"); | |
| } | |
| if (endpoint.auth) { | |
| try { | |
| const value = resolveAuthValue(endpoint.auth.value, endpoint.auth.fromEnv ?? false); | |
| if (endpoint.auth.type === "bearer") { | |
| baseHeaders["Authorization"] = `Bearer ${value}`; | |
| } else { | |
| const headerName = endpoint.auth.headerName ?? "Authorization"; | |
| baseHeaders[headerName] = value; | |
| } | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : "Auth configuration error"; | |
| return { | |
| toolCallId: callId, | |
| name: tool.name, | |
| result: `Tool returned error ${response.status}: ${text.slice(0, 500)}`, | |
| ok: false, | |
| result: `Tool execution failed (auth): ${message}`, | |
| displayText: tool.description.slice(0, 50), | |
| }; | |
| } | |
| } | |
| const baseHeaders: Record<string, string> = { | |
| "Content-Type": "application/json", | |
| ...endpoint.headers, | |
| }; | |
| if (idempotencyKey) baseHeaders["Idempotency-Key"] = idempotencyKey; | |
| if (endpoint.secret) { | |
| const timestamp = Math.floor(Date.now() / 1000).toString(); | |
| baseHeaders["X-Kody-Timestamp"] = timestamp; | |
| baseHeaders["X-Kody-Signature"] = createHmac("sha256", endpoint.secret) | |
| .update(`${timestamp}.${idempotencyKey ?? ""}.${bodyText}`) | |
| .digest("hex"); | |
| } | |
| if (endpoint.auth) { | |
| try { | |
| const value = resolveAuthValue(endpoint.auth.value, endpoint.auth.fromEnv ?? false); | |
| if (endpoint.auth.type === "bearer") { | |
| baseHeaders["Authorization"] = `Bearer ${value}`; | |
| } else { | |
| const headerName = endpoint.auth.headerName ?? "Authorization"; | |
| baseHeaders[headerName] = value; | |
| } | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : "Auth configuration error"; | |
| return { | |
| toolCallId: callId, | |
| name: tool.name, | |
| ok: false, | |
| result: `Tool execution failed (auth): ${message}`, | |
| displayText: tool.description.slice(0, 50), | |
| }; | |
| } | |
| } |
🤖 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 `@packages/server/src/services/tools/executor.ts` around lines 217 - 246,
Update the HMAC signing logic around createHmac and baseHeaders to add a
timestamp header and sign a canonical string containing the timestamp,
Idempotency-Key, and bodyText, preserving consistent handling when the
idempotency key is absent. Document the exact signed-string format and
verification requirements in the package README so receivers can reproduce the
signature.
Comment on lines
+119
to
+159
| let queryObj: Record<string, unknown> | undefined; | ||
| try { | ||
| queryObj = parseJsonObject(parsed.data.query, "query", "any"); | ||
| } catch (err) { | ||
| return { | ||
| ok: false, | ||
| message: err instanceof Error ? err.message : "Invalid query JSON", | ||
| }; | ||
| } | ||
| let headersObj: Record<string, string> | undefined; | ||
| try { | ||
| headersObj = parseJsonObject( | ||
| parsed.data.headers, | ||
| "headers", | ||
| "string", | ||
| ) as Record<string, string> | undefined; | ||
| } catch (err) { | ||
| return { | ||
| ok: false, | ||
| message: err instanceof Error ? err.message : "Invalid headers JSON", | ||
| }; | ||
| } | ||
|
|
||
| let bodyObj: unknown; | ||
| if (parsed.data.body) { | ||
| try { | ||
| bodyObj = JSON.parse(parsed.data.body); | ||
| } catch { | ||
| return { ok: false, message: "body must be a valid JSON string" }; | ||
| } | ||
| } | ||
|
|
||
| let result: HttpCallResult; | ||
| try { | ||
| result = await httpCall({ | ||
| url: fullUrl, | ||
| method: defaultMethod, | ||
| headers: headersObj, | ||
| body: bodyObj, | ||
| allowedHosts, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
queryObj is parsed and validated, then discarded.
Line 121 parses query into queryObj, and the invalid-JSON branch returns an error. The httpCall invocation at lines 153-159 never passes queryObj. Every agent-supplied query value is dropped, so http_get silently ignores the parameter that its own schema documents at lines 25-26 and advertises to the model.
httpCall serializes the body option into the query string for GET and DELETE. Merge queryObj into the URL so both methods keep working with an explicit body.
🐛 Proposed fix
let result: HttpCallResult;
try {
+ let requestUrl = fullUrl;
+ if (queryObj) {
+ const params = new URLSearchParams();
+ for (const [k, v] of Object.entries(queryObj)) {
+ if (v === undefined || v === null) continue;
+ params.set(k, String(v));
+ }
+ const qs = params.toString();
+ if (qs) {
+ requestUrl = `${fullUrl}${fullUrl.includes("?") ? "&" : "?"}${qs}`;
+ }
+ }
result = await httpCall({
- url: fullUrl,
+ url: requestUrl,
method: defaultMethod,
headers: headersObj,
body: bodyObj,
allowedHosts,
});Add a test that asserts the query string reaches fetch.
📝 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
| let queryObj: Record<string, unknown> | undefined; | |
| try { | |
| queryObj = parseJsonObject(parsed.data.query, "query", "any"); | |
| } catch (err) { | |
| return { | |
| ok: false, | |
| message: err instanceof Error ? err.message : "Invalid query JSON", | |
| }; | |
| } | |
| let headersObj: Record<string, string> | undefined; | |
| try { | |
| headersObj = parseJsonObject( | |
| parsed.data.headers, | |
| "headers", | |
| "string", | |
| ) as Record<string, string> | undefined; | |
| } catch (err) { | |
| return { | |
| ok: false, | |
| message: err instanceof Error ? err.message : "Invalid headers JSON", | |
| }; | |
| } | |
| let bodyObj: unknown; | |
| if (parsed.data.body) { | |
| try { | |
| bodyObj = JSON.parse(parsed.data.body); | |
| } catch { | |
| return { ok: false, message: "body must be a valid JSON string" }; | |
| } | |
| } | |
| let result: HttpCallResult; | |
| try { | |
| result = await httpCall({ | |
| url: fullUrl, | |
| method: defaultMethod, | |
| headers: headersObj, | |
| body: bodyObj, | |
| allowedHosts, | |
| }); | |
| let queryObj: Record<string, unknown> | undefined; | |
| try { | |
| queryObj = parseJsonObject(parsed.data.query, "query", "any"); | |
| } catch (err) { | |
| return { | |
| ok: false, | |
| message: err instanceof Error ? err.message : "Invalid query JSON", | |
| }; | |
| } | |
| let headersObj: Record<string, string> | undefined; | |
| try { | |
| headersObj = parseJsonObject( | |
| parsed.data.headers, | |
| "headers", | |
| "string", | |
| ) as Record<string, string> | undefined; | |
| } catch (err) { | |
| return { | |
| ok: false, | |
| message: err instanceof Error ? err.message : "Invalid headers JSON", | |
| }; | |
| } | |
| let bodyObj: unknown; | |
| if (parsed.data.body) { | |
| try { | |
| bodyObj = JSON.parse(parsed.data.body); | |
| } catch { | |
| return { ok: false, message: "body must be a valid JSON string" }; | |
| } | |
| } | |
| let result: HttpCallResult; | |
| try { | |
| let requestUrl = fullUrl; | |
| if (queryObj) { | |
| const params = new URLSearchParams(); | |
| for (const [k, v] of Object.entries(queryObj)) { | |
| if (v === undefined || v === null) continue; | |
| params.set(k, String(v)); | |
| } | |
| const qs = params.toString(); | |
| if (qs) { | |
| requestUrl = `${fullUrl}${fullUrl.includes("?") ? "&" : "?"}${qs}`; | |
| } | |
| } | |
| result = await httpCall({ | |
| url: requestUrl, | |
| method: defaultMethod, | |
| headers: headersObj, | |
| body: bodyObj, | |
| allowedHosts, | |
| }); |
🤖 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 `@packages/tools/src/prebuilt.ts` around lines 119 - 159, Update the HTTP
request flow around queryObj and httpCall so the validated query parameters are
merged into the request URL, while preserving any existing URL query parameters
and explicit body-derived parameters for GET and DELETE. Ensure query values
reach fetch, and add a test covering that the supplied query string is included
in the fetched URL.
Comment on lines
+113
to
+135
| it("treats a null body as no query params for GET (regression)", async () => { | ||
| mockFetch.mockResolvedValue({ ok: true, status: 200, text: async () => "" }); | ||
| await httpCall({ url: "https://x.test", method: "GET", body: null }); | ||
| const [url] = mockFetch.mock.calls[0]; | ||
| expect(url).toBe("https://x.test/"); | ||
| expect(url).not.toContain("?"); | ||
| }); | ||
|
|
||
| it("treats a null body as no query params for DELETE (regression)", async () => { | ||
| mockFetch.mockResolvedValue({ ok: true, status: 200, text: async () => "" }); | ||
| const r = await httpCall({ url: "https://x.test", method: "DELETE", body: null }); | ||
| expect(r.ok).toBe(true); | ||
| const [url, opts] = mockFetch.mock.calls[0]; | ||
| expect(url).toBe("https://x.test/"); | ||
| expect(opts.method).toBe("DELETE"); | ||
| }); | ||
|
|
||
| it("treats an undefined body as no query params for GET", async () => { | ||
| mockFetch.mockResolvedValue({ ok: true, status: 200, text: async () => "" }); | ||
| await httpCall({ url: "https://x.test", method: "GET" }); | ||
| const [url] = mockFetch.mock.calls[0]; | ||
| expect(url).toBe("https://x.test/"); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'finalUrl|new URL\(|toString\(\)' packages/tools/src/http.ts
rg -n 'x\.test|api\.example\.com/ping' packages/tools/tests/http.test.ts packages/tools/tests/toolkit.test.tsRepository: Chafficui/kody
Length of output: 2702
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- packages/tools/src/http.ts ---'
sed -n '130,265p' packages/tools/src/http.ts
printf '%s\n' '--- relevant test sections ---'
sed -n '100,175p' packages/tools/tests/http.test.ts
sed -n '145,172p' packages/tools/tests/toolkit.test.ts
printf '%s\n' '--- static URL-flow verifier ---'
python3 - <<'PY'
from pathlib import Path
source = Path("packages/tools/src/http.ts").read_text()
assert "let finalUrl = url;" in source
assert 'if (qs) finalUrl = `${url}${url.includes("?") ? "&" : "?"}${qs}`;' in source
assert "fetcher(finalUrl" in source
# The source preserves the input URL when params.toString() is empty.
url = "https://x.test"
body = None
params = {}
qs = "&".join(f"{k}={v}" for k, v in params.items())
final_url = url
if qs:
final_url = f"{url}{'&' if '?' in url else '?'}{qs}"
assert final_url == "https://x.test"
print(f"null/undefined body URL: {final_url}")
toolkit = Path("packages/tools/tests/toolkit.test.ts").read_text()
assert 'expect(calledUrl).toBe("https://api.example.com/ping");' in toolkit
print("toolkit contract: https://api.example.com/ping")
PYRepository: Chafficui/kody
Length of output: 8044
Expect "https://x.test" without a trailing slash. httpCall preserves the input URL when body is null or undefined. Update all three assertions.
🤖 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 `@packages/tools/tests/http.test.ts` around lines 113 - 135, Update the URL
expectations in the three httpCall regression tests for null or undefined bodies
to assert “https://x.test” without a trailing slash, while preserving the
existing query-parameter and DELETE method assertions.
Comment on lines
+227
to
+239
| it("httpGet allows loopback when an allowlist is passed via the low-level helper", async () => { | ||
| mockFetch.mockResolvedValue({ | ||
| ok: true, | ||
| status: 200, | ||
| text: async () => JSON.stringify({ ok: true }), | ||
| }); | ||
| const result = await httpCall({ | ||
| url: "https://api.example.com/ping", | ||
| method: "GET", | ||
| allowedHosts: ["api.example.com"], | ||
| }); | ||
| expect(result.ok).toBe(true); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
The allowlist test does not exercise the allowlist bypass.
The test name states "allows loopback", but the URL is https://api.example.com/ping, a public host. That host passes assertSafeTarget with or without allowedHosts, so the assertion cannot fail if the early return at packages/tools/src/http.ts lines 154-160 is removed.
Use a host that the blocklist rejects. That pins the documented behavior: an explicit allowlist skips the private-address check.
♻️ Proposed change
- it("httpGet allows loopback when an allowlist is passed via the low-level helper", async () => {
+ it("httpCall allows a blocked host when it is named in the allowlist", async () => {
mockFetch.mockResolvedValue({
ok: true,
status: 200,
text: async () => JSON.stringify({ ok: true }),
});
const result = await httpCall({
- url: "https://api.example.com/ping",
+ url: "http://127.0.0.1:8080/ping",
method: "GET",
- allowedHosts: ["api.example.com"],
+ allowedHosts: ["127.0.0.1"],
});
expect(result.ok).toBe(true);
+ expect(mockFetch).toHaveBeenCalledTimes(1);
});📝 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
| it("httpGet allows loopback when an allowlist is passed via the low-level helper", async () => { | |
| mockFetch.mockResolvedValue({ | |
| ok: true, | |
| status: 200, | |
| text: async () => JSON.stringify({ ok: true }), | |
| }); | |
| const result = await httpCall({ | |
| url: "https://api.example.com/ping", | |
| method: "GET", | |
| allowedHosts: ["api.example.com"], | |
| }); | |
| expect(result.ok).toBe(true); | |
| }); | |
| it("httpCall allows a blocked host when it is named in the allowlist", async () => { | |
| mockFetch.mockResolvedValue({ | |
| ok: true, | |
| status: 200, | |
| text: async () => JSON.stringify({ ok: true }), | |
| }); | |
| const result = await httpCall({ | |
| url: "http://127.0.0.1:8080/ping", | |
| method: "GET", | |
| allowedHosts: ["127.0.0.1"], | |
| }); | |
| expect(result.ok).toBe(true); | |
| expect(mockFetch).toHaveBeenCalledTimes(1); | |
| }); |
🤖 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 `@packages/tools/tests/toolkit.test.ts` around lines 227 - 239, Update the test
around httpCall to use a loopback or otherwise blocklisted URL so it would be
rejected without the allowedHosts bypass, while retaining an allowlist entry
that permits that host. Keep the existing successful mock response and
assertion, ensuring the test specifically verifies that allowedHosts skips the
private-address safety check.
Comment on lines
+36
to
+45
| const FORBIDDEN = [ | ||
| { name: "kody.codai.app", pattern: /\bkody\.codai\.app\b/ }, | ||
| { name: "api.kody.codai.app", pattern: /\bapi\.kody\.codai\.app\b/ }, | ||
| { name: "STRIPE_", pattern: /\bprocess\.env\.STRIPE_[A-Z_]+\b/ }, | ||
| { name: "SENDGRID_API_KEY", pattern: /\bprocess\.env\.SENDGRID_API_KEY\b/ }, | ||
| { name: "MAILGUN_API_KEY", pattern: /\bprocess\.env\.MAILGUN_API_KEY\b/ }, | ||
| { name: "POSTMARK_API_KEY", pattern: /\bprocess\.env\.POSTMARK_API_KEY\b/ }, | ||
| { name: "AWS_ACCESS_KEY", pattern: /\bprocess\.env\.AWS_ACCESS_KEY(_ID)?\b/ }, | ||
| { name: "SES_", pattern: /\bprocess\.env\.SES_[A-Z_]+\b/ }, | ||
| ]; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The documented kody-website rule is not enforced.
Lines 92-94 state that kody-website is never allowed anywhere, including README.md. FORBIDDEN contains no pattern for it, so the script accepts the fork name. Add the pattern, or correct the comment.
🐛 Proposed fix
const FORBIDDEN = [
{ name: "kody.codai.app", pattern: /\bkody\.codai\.app\b/ },
{ name: "api.kody.codai.app", pattern: /\bapi\.kody\.codai\.app\b/ },
+ { name: "kody-website", pattern: /\bkody-website\b/ },
{ name: "STRIPE_", pattern: /\bprocess\.env\.STRIPE_[A-Z_]+\b/ },Also applies to: 92-95
🤖 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 `@scripts/check-hosting-leaks.mjs` around lines 36 - 45, Update the FORBIDDEN
list in check-hosting-leaks to add a pattern that rejects the documented
kody-website name everywhere, including README.md. Use the same entry structure
as the existing hostname and secret patterns, while preserving all current
forbidden checks.
Comment on lines
+204
to
+209
| // Run scans. | ||
| const allHits = []; | ||
| for (const abs of candidates) { | ||
| const rel = path.relative(ROOT, abs).split(path.sep).join("/"); | ||
| const ext = path.extname(rel); | ||
| if (!SCAN_EXTENSIONS.has(ext)) continue; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The extension filter silently drops the explicitly listed top-level files.
Line 209 rejects any candidate whose extension is not in SCAN_EXTENSIONS. path.extname("Dockerfile.server") returns ".server", and path.extname(".env.example") returns ".example". Neither value is in SCAN_EXTENSIONS, so both files are never scanned, even though lines 99-105 add them on purpose. Docker and env files are exactly where hosted URLs and vendor keys appear, so the guardrail misses its highest-value targets.
Note also that ".env" in SCAN_EXTENSIONS (line 82) is unreachable. Node treats a leading dot as part of the basename, so path.extname(".env") returns "".
Exempt the explicitly listed files from the extension filter.
🐛 Proposed fix
+const FORCE_SCAN = new Set(
+ [...TOP_LEVEL_FILES, "README.md", "CLAUDE.md", "CONTRIBUTING.md", "LICENSE"],
+);
+
// Run scans.
const allHits = [];
for (const abs of candidates) {
const rel = path.relative(ROOT, abs).split(path.sep).join("/");
const ext = path.extname(rel);
- if (!SCAN_EXTENSIONS.has(ext)) continue;
+ if (!FORCE_SCAN.has(rel) && !SCAN_EXTENSIONS.has(ext)) continue;📝 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
| // Run scans. | |
| const allHits = []; | |
| for (const abs of candidates) { | |
| const rel = path.relative(ROOT, abs).split(path.sep).join("/"); | |
| const ext = path.extname(rel); | |
| if (!SCAN_EXTENSIONS.has(ext)) continue; | |
| const FORCE_SCAN = new Set( | |
| [...TOP_LEVEL_FILES, "README.md", "CLAUDE.md", "CONTRIBUTING.md", "LICENSE"], | |
| ); | |
| // Run scans. | |
| const allHits = []; | |
| for (const abs of candidates) { | |
| const rel = path.relative(ROOT, abs).split(path.sep).join("/"); | |
| const ext = path.extname(rel); | |
| if (!FORCE_SCAN.has(rel) && !SCAN_EXTENSIONS.has(ext)) continue; |
🤖 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 `@scripts/check-hosting-leaks.mjs` around lines 204 - 209, Update the scan loop
around the candidates filter and the existing explicit-file list so explicitly
listed top-level files bypass the SCAN_EXTENSIONS check. Use the same
identifying path or filename set that adds those files, ensuring
Dockerfile.server, .env.example, and .env are scanned while retaining extension
filtering for other candidates.
This was referenced Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@kody/toolsprebuilt library + tool executor tests + admin "Test tool" UIWhy
External tool support is the #1 thing that makes Kody actually useful in
production, but until this PR it was a thin HTTP wrapper: any
{ name, arguments }body, no auth, no retries, no schema validation onthe wire format, no test UI, and zero test coverage on the executor or
agent loop. Anyone wanting "search my CRM" had to write a server.
This PR ships a prebuilt library of opinionated tools, a tool registry
that lets the public repo call the same handlers the toolkit defines
directly, HMAC signing + retry on the executor, and an admin "Test tool"
dialog so site owners can validate their tools without leaving the
dashboard.
What
New package:
packages/tools/A new workspace package that ships prebuilt tool definitions and a
fluent
Toolkitbuilder. Public API only — no hosted assumptions.httpCall,httpGet,httpPost— generic HTTP tools with optionalHMAC signing, retry, JSON-path extraction.
webhook(url, { secret })— POST a signed webhook (HMAC-SHA256 inX-Kody-Signatureheader).slack({ channel, token })— prebuilt Slack tool.linear({ apiKey, teamId })— prebuilt Linear tool (create issue).sendgridEmail({ apiKey, from })— prebuilt transactional email.Toolkit— fluent builder:Each tool uses Zod-validated parameters and an auth context object —
never reads
process.envdirectly except via the toolkit builder.Server changes
packages/shared/src/validators/site-config.ts—customToolSchemagains
secret(optional, HMAC body signing),auth(optional Beareror API-key), and
retry(max attempts, backoff) fields.packages/server/src/services/tools/executor.ts—ToolExecutornow exposes a
ToolRegistryinterface so the toolkit builder canregister HTTP-handler functions directly. Honors
endpoint.secretfor HMAC signing,
endpoint.retryfor transient 5xx/429 retries,returns a structured
ok: booleanon every result. The agent loop inagent.tsnow strictly enforcesmaxToolCalls(the oldwhile (totalToolCalls <= maxCalls)could over-shoot).packages/server/src/routes/admin/tools.ts(new) —POST /api/admin/sites/:siteId/tools/:toolName/testlets adminsexercise a tool without running a chat.
Admin UI
packages/admin/src/pages/SiteEdit.tsx— adds a "Test tool"affordance next to each custom tool entry. Click → opens a modal with
the tool's parameter schema (auto-generated form from the Zod schema),
a JSON editor for advanced users, a "Run" button, and a result viewer
that distinguishes 4xx/5xx vs timeout vs invalid-JSON errors. A
copy-as-curl button lets admins re-run the same call outside the
dashboard.
UX notes
Esc, andexposes a "Copy as curl" button for users who want to script it.
invalid JSON is blue with a "show what was sent" expander. Same
pattern as the rest of the admin.
adding a new tool to the toolkit gives admins the right input UI
without any admin-side work.
Tests
packages/tools/: 25 unit tests (one per prebuilt tool + Toolkitbuilder round-trips)
packages/server/: 13 executor tests (HMAC signing, retry, authresolution, structured error results, unknown tool name), 6 agent
tests (max-calls enforcement, tool-error surfacing, end-to-end loop),
8 admin integration tests (test endpoint, auth, schema validation
on the wire)
Test plan
pnpm test— 405/405 passingpnpm typecheckcleanpnpm buildclean across all 5 packagesJSON validation + curl-copy flow was verified in the agent's report)
Cross-stream notes
tools testcommand (in thefeat/cli-openapibranch)proxies to this PR's
POST /api/admin/sites/:siteId/tools/:toolName/testendpoint. Land this PR first, then the CLI command will work.
fields on
customToolSchemaare optional.Docs
packages/tools/README.mdwith a quick-start, command reference,and a copy-pasteable "wire up a Slack notification on ticket creation"
example.
kody-website/src/app/docs/agent-tools/page.tsxwill need a"Prebuilt tools" section. (This is a follow-up in the hosted repo, not
in this PR.)
Summary by CodeRabbit