Conversation
… preExecute functionality - Remove default file_path transformers from StackOneToolSet - Remove addTransformationSourceParameters method - Add experimental preExecute function support for dynamic parameter processing - Add ExperimentalPreExecuteFunction type with "experimental" prefix - Create comprehensive experimental document handling example - Update exports to include experimental types - Update snapshots to reflect removed transformers - Maintain core parameter mapping infrastructure for experimental features This addresses security concerns while providing a flexible, developer-controlled approach to document handling through experimental preExecute functions. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: mattzcarey <mattzcarey@users.noreply.github.com>
- Rename ExperimentalPreExecuteFunction to Experimental_PreExecuteFunction - Rename experimentalPreExecute to experimental_PreExecute - Update all imports, exports, and usages across codebase Co-authored-by: mattzcarey <mattzcarey@users.noreply.github.com>
Add comprehensive example demonstrating experimental_ prefixed transformation API alongside existing ParameterMapper approach. Includes: - Experimental_ParameterMapper class with experimental_ naming - File, user data, and date transformation examples - Composite transformation handlers - Comprehensive test cases for UX comparison Co-authored-by: mattzcarey <mattzcarey@users.noreply.github.com>
Remove old parameter transformation system in favor of new experimental schema override + preExecute API
Remove example that used deprecated ParameterMapper approach in favor of new schema override + preExecute API
Complete implementation of the new experimental two-stage transformation API: - Add experimental_schemaOverride for schema modification at tool creation time - Add experimental_preExecute for parameter transformation at execution time - Remove all old parameter transformation infrastructure (ParameterMapper, etc.) - Update all toolset classes to support new experimental options - Rewrite document handling example to demonstrate new API - Ensure type safety and proper tool creation with experimental options This replaces the old parameter transformation system with a more flexible and developer-friendly approach that separates schema definition from parameter transformation. Co-authored-by: mattzcarey <mattzcarey@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces an experimental document handling mechanism for tools by allowing callers to (a) override a tool’s input JSON schema at creation time and (b) preprocess parameters before execution. In doing so, it also removes the previous parameter transformation/derivation system (including related tests and examples).
Changes:
- Add experimental tool-creation options:
experimental_schemaOverrideandexperimental_preExecute, with support inTools.getTool(...)andToolSet.getTool(...). - Remove the legacy parameter transformer/derivation pipeline from toolsets/tools, along with associated tests and examples.
- Add a new example demonstrating experimental document handling via schema override + preExecute.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Adds experimental type definitions for schema override + preExecute. |
| src/tool.ts | Implements experimental tool cloning + preExecute execution path; updates Tools.getTool. |
| src/toolsets/base.ts | Removes transformers support; adds getTool overload to accept experimental creation options. |
| src/toolsets/openapi.ts | Stops passing transformer configs into tool creation. |
| src/toolsets/stackone.ts | Removes default file-path transformation logic and transformer wiring. |
| src/index.ts | Exposes the new experimental types and stops exporting transformer-related types. |
| examples/experimental-document-handling.ts | New example for schema override + preExecute-based document handling. |
| examples/index.ts | Updates examples index to point to experimental document handling doc. |
| src/tests/transformations.spec.ts | Deletes transformation-related tests (legacy pipeline). |
| src/modules/parameterMapper.ts | Deletes the parameter mapping/derivation module (legacy pipeline). |
| src/modules/tests/parameterMapper.spec.ts | Deletes mapper tests. |
| examples/openapi-transformations.ts | Deletes legacy transformation example. |
| examples/file-uploads.ts | Deletes legacy file upload example dependent on transformations. |
| src/openapi/tests/snapshots/openapi-parser.spec.ts.snap | Snapshot updates reflecting updated OpenAPI parsing output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Convert string params to object | ||
| const params = typeof inputParams === 'string' ? JSON.parse(inputParams) : inputParams || {}; | ||
|
|
||
| // Apply experimental preExecute function (either from tool creation or execution options) | ||
| let processedParams = params; | ||
|
|
||
| if (this.experimental_preExecute) { | ||
| processedParams = await this.experimental_preExecute(params); | ||
| } | ||
|
|
||
| // Execute the request | ||
| return await this.requestBuilder.execute(mappedParams, options); | ||
| // Execute the request directly with processed parameters | ||
| return await this.requestBuilder.execute(processedParams, options); |
There was a problem hiding this comment.
execute() accepts inputParams as object but doesn't ensure it's a plain JSON object. JSON.parse() can produce arrays/primitives and typeof inputParams === 'object' also allows arrays/Date/etc, which will later break RequestBuilder.execute() (uses Object.entries(params)). Consider validating that parsed/received params are non-null, non-array plain objects and throw a StackOneError otherwise.
| // Apply experimental preExecute function (either from tool creation or execution options) | ||
| let processedParams = params; | ||
|
|
||
| if (this.experimental_preExecute) { | ||
| processedParams = await this.experimental_preExecute(params); | ||
| } |
There was a problem hiding this comment.
The comment says the experimental preExecute function can come from "tool creation or execution options", but the implementation only supports the constructor-provided experimental_preExecute (there is no hook in ExecuteOptions). Either update the comment to match current behavior or extend ExecuteOptions to allow an execution-time preExecute override.
| @@ -75,6 +65,23 @@ export interface ExecuteConfig { | |||
| }[]; // this params are the full list of params used to execute. This should come straight from the OpenAPI spec. | |||
| } | |||
|
|
|||
| /** | |||
| * EXPERIMENTAL: Options for creating tools with schema overrides and preExecute functions | |||
| */ | |||
| export interface Experimental_ToolCreationOptions { | |||
| /** | |||
| * EXPERIMENTAL: Function to override the tool schema at creation time | |||
| * Takes the original schema and returns a new schema for the tool | |||
| */ | |||
| experimental_schemaOverride?: Experimental_SchemaOverride; | |||
|
|
|||
| /** | |||
| * EXPERIMENTAL: Function to preprocess parameters before execution | |||
| * Transforms parameters from override schema format back to original API format | |||
| */ | |||
| experimental_preExecute?: Experimental_PreExecuteFunction; | |||
| } | |||
There was a problem hiding this comment.
New exported type names like Experimental_SchemaOverride / Experimental_PreExecuteFunction / Experimental_ToolCreationOptions introduce underscores, which is inconsistent with the existing exported type naming (e.g. ExecuteConfig, ToolDefinition). Consider renaming to ExperimentalSchemaOverride, ExperimentalPreExecuteFunction, ExperimentalToolCreationOptions (or similar) to keep the public API consistent.
| // Security check: only allow certain paths | ||
| const isAllowed = allowedPaths.some((allowedPath) => doc_id.startsWith(allowedPath)); | ||
|
|
||
| if (!isAllowed) { | ||
| throw new Error(`Document path not allowed: ${doc_id}`); | ||
| } |
There was a problem hiding this comment.
allowedPaths.some((allowedPath) => doc_id.startsWith(allowedPath)) is not a safe containment check (e.g., /allowed also matches /allowed_evil, and symlink/.. traversal issues remain). For a safer example, resolve/normalize the path and verify it is within an allowed directory (e.g., using path.resolve + path.relative checks) before reading the file.
| getTool(name: string, options?: Experimental_ToolCreationOptions): BaseTool | undefined { | ||
| const originalTool = this.tools.find((tool) => tool.name === name); | ||
| if (!originalTool) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // If no experimental options provided, return original tool | ||
| if (!options?.experimental_schemaOverride && !options?.experimental_preExecute) { | ||
| return originalTool; | ||
| } | ||
|
|
||
| // Create a new tool with experimental schema override and preExecute | ||
| let parameters = originalTool.parameters; | ||
|
|
||
| // Apply schema override if provided | ||
| if (options.experimental_schemaOverride) { | ||
| parameters = options.experimental_schemaOverride(originalTool.parameters); | ||
| } | ||
|
|
||
| // Create new tool instance with modified schema and preExecute function | ||
| if (originalTool instanceof StackOneTool) { | ||
| const newTool = new StackOneTool( | ||
| originalTool.name, | ||
| originalTool.description, | ||
| parameters, | ||
| originalTool.executeConfig, | ||
| originalTool.getHeaders(), | ||
| options.experimental_preExecute | ||
| ); | ||
| return newTool; | ||
| } | ||
| const newTool = new BaseTool( | ||
| originalTool.name, | ||
| originalTool.description, | ||
| parameters, | ||
| originalTool.executeConfig, | ||
| originalTool.getHeaders(), | ||
| options.experimental_preExecute | ||
| ); | ||
| return newTool; |
There was a problem hiding this comment.
The new experimental tool creation flow (schemaOverride / preExecute) adds non-trivial behavior (tool cloning + schema mutation + pre-execution param transformation), but there are no unit tests covering it. Since this repo already has tool-level tests, please add coverage for: (1) Tools.getTool(name, options) returning a tool with overridden schema, (2) experimental_preExecute being invoked and its output reaching RequestBuilder.execute() (including dryRun), and (3) error paths when preExecute throws/returns invalid params.
| authentication?: AuthenticationConfig; | ||
| headers?: Record<string, string>; | ||
| transformers?: ParameterTransformerMap; | ||
| _oasUrl?: string; |
There was a problem hiding this comment.
BaseToolSetConfig no longer exposes the previous transformers configuration and the old parameter transformation pipeline has been removed across toolsets. If this SDK is semvered, this is a breaking public API change; please ensure it’s called out clearly in release notes / migration guidance (and versioning) so downstream users aren’t surprised.
| _oasUrl?: string; | |
| _oasUrl?: string; | |
| /** | |
| * @deprecated This configuration option is retained for backward compatibility. | |
| * The legacy parameter transformation pipeline has been removed and | |
| * this field is currently ignored by the implementation. | |
| */ | |
| transformers?: unknown; |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19e8383cf2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Execute the request directly with processed parameters | ||
| return await this.requestBuilder.execute(processedParams, options); |
There was a problem hiding this comment.
BaseTool.execute now forwards processedParams directly to RequestBuilder without running any derivation/mapping step, so inputs that previously relied on derived fields (for example the documented file_path and custom transformer flows) are no longer expanded into API parameters before dispatch. In practice this sends incomplete payloads to endpoints that expect transformed fields and breaks existing integrations that were valid before this commit.
Useful? React with 👍 / 👎.
| if (options.experimental_schemaOverride) { | ||
| parameters = options.experimental_schemaOverride(originalTool.parameters); |
There was a problem hiding this comment.
experimental_schemaOverride receives originalTool.parameters by reference, so an override that mutates its input (e.g., deleting or rewriting properties in place) will permanently alter the shared base tool schema for later calls to getTool, including calls without experimental options. This leaks state across tool instances and makes behavior order-dependent; the override should operate on a defensive copy.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 15 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="examples/experimental-document-handling.ts">
<violation number="1" location="examples/experimental-document-handling.ts:86">
P0: Custom agent: **Flag Security Vulnerabilities**
The allowlist check uses `startsWith` on raw `doc_id`, which can be bypassed and enables arbitrary local file reads. Resolve and normalize the path, then verify it is within an allowed directory before reading. This violates the security-vulnerability rule for path traversal/local file exposure.</violation>
</file>
<file name="src/toolsets/stackone.ts">
<violation number="1" location="src/toolsets/stackone.ts:134">
P1: This change removes StackOne’s built-in file upload parameter derivation, so `file_path`-only calls will no longer be transformed into API-required file fields.</violation>
</file>
<file name="src/toolsets/openapi.ts">
<violation number="1" location="src/toolsets/openapi.ts:90">
P1: OpenAPI tools are now created without derived-parameter preprocessing, which breaks documented `transformers` behavior for parameter transformations.</violation>
</file>
<file name="src/toolsets/base.ts">
<violation number="1" location="src/toolsets/base.ts:206">
P2: The `headersOrOptions` type guard is too loose and can misclassify headers as experimental options, leading to runtime failures when non-function values are used as `experimental_*` fields.</violation>
<violation number="2" location="src/toolsets/base.ts:222">
P2: The experimental `getTool` branch overwrites cloned tool headers with only toolset defaults, which can silently drop tool-specific headers.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| // Security check: only allow certain paths | ||
| const isAllowed = allowedPaths.some((allowedPath) => doc_id.startsWith(allowedPath)); |
There was a problem hiding this comment.
P0: Custom agent: Flag Security Vulnerabilities
The allowlist check uses startsWith on raw doc_id, which can be bypassed and enables arbitrary local file reads. Resolve and normalize the path, then verify it is within an allowed directory before reading. This violates the security-vulnerability rule for path traversal/local file exposure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/experimental-document-handling.ts, line 86:
<comment>The allowlist check uses `startsWith` on raw `doc_id`, which can be bypassed and enables arbitrary local file reads. Resolve and normalize the path, then verify it is within an allowed directory before reading. This violates the security-vulnerability rule for path traversal/local file exposure.</comment>
<file context>
@@ -0,0 +1,387 @@
+ }
+
+ // Security check: only allow certain paths
+ const isAllowed = allowedPaths.some((allowedPath) => doc_id.startsWith(allowedPath));
+
+ if (!isAllowed) {
</file context>
| this.headers, | ||
| this.transformers | ||
| toolDef.description, | ||
| toolDef.parameters, |
There was a problem hiding this comment.
P1: This change removes StackOne’s built-in file upload parameter derivation, so file_path-only calls will no longer be transformed into API-required file fields.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/toolsets/stackone.ts, line 134:
<comment>This change removes StackOne’s built-in file upload parameter derivation, so `file_path`-only calls will no longer be transformed into API-required file fields.</comment>
<file context>
@@ -173,25 +122,18 @@ export class StackOneToolSet extends ToolSet {
- this.headers,
- this.transformers
+ toolDef.description,
+ toolDef.parameters,
+ toolDef.execute,
+ this.headers
</file context>
|
|
||
| // Create tool | ||
| const tool = this.createTool(toolName, processedDef); | ||
| const tool = this.createTool(toolName, toolDef); |
There was a problem hiding this comment.
P1: OpenAPI tools are now created without derived-parameter preprocessing, which breaks documented transformers behavior for parameter transformations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/toolsets/openapi.ts, line 90:
<comment>OpenAPI tools are now created without derived-parameter preprocessing, which breaks documented `transformers` behavior for parameter transformations.</comment>
<file context>
@@ -88,11 +86,8 @@ export class OpenAPIToolSet extends ToolSet {
-
// Create tool
- const tool = this.createTool(toolName, processedDef);
+ const tool = this.createTool(toolName, toolDef);
// Add tool to the list
</file context>
|
|
||
| // Apply instance headers to the tool | ||
| if (this.headers && experimentalTool.setHeaders) { | ||
| experimentalTool.setHeaders(this.headers); |
There was a problem hiding this comment.
P2: The experimental getTool branch overwrites cloned tool headers with only toolset defaults, which can silently drop tool-specific headers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/toolsets/base.ts, line 222:
<comment>The experimental `getTool` branch overwrites cloned tool headers with only toolset defaults, which can silently drop tool-specific headers.</comment>
<file context>
@@ -206,74 +189,48 @@ export abstract class ToolSet {
+
+ // Apply instance headers to the tool
+ if (this.headers && experimentalTool.setHeaders) {
+ experimentalTool.setHeaders(this.headers);
}
+
</file context>
| ('experimental_schemaOverride' in headersOrOptions || | ||
| 'experimental_preExecute' in headersOrOptions); |
There was a problem hiding this comment.
P2: The headersOrOptions type guard is too loose and can misclassify headers as experimental options, leading to runtime failures when non-function values are used as experimental_* fields.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/toolsets/base.ts, line 206:
<comment>The `headersOrOptions` type guard is too loose and can misclassify headers as experimental options, leading to runtime failures when non-function values are used as `experimental_*` fields.</comment>
<file context>
@@ -206,74 +189,48 @@ export abstract class ToolSet {
+ // Determine if the second parameter is headers or experimental options
+ const isExperimentalOptions =
+ headersOrOptions &&
+ ('experimental_schemaOverride' in headersOrOptions ||
+ 'experimental_preExecute' in headersOrOptions);
</file context>
| ('experimental_schemaOverride' in headersOrOptions || | |
| 'experimental_preExecute' in headersOrOptions); | |
| (typeof (headersOrOptions as Experimental_ToolCreationOptions).experimental_schemaOverride === 'function' || | |
| typeof (headersOrOptions as Experimental_ToolCreationOptions).experimental_preExecute === 'function'); |
|
Acknowledged. (Benchmark test, please ignore.) |
This pull request introduces experimental document handling capabilities to improve document processing in the StackOne AI Node SDK.
Summary by cubic
Adds experimental document handling with a two-stage API (schema override + preExecute) and removes the old parameter transformation system. This enables flexible tool input schemas and secure, developer-controlled parameter processing.
New Features
examples/experimental-document-handling.ts(covers files, URLs, DBs).Experimental_SchemaOverride,Experimental_PreExecuteFunction,Experimental_ToolCreationOptions.Migration
ParameterMapper,ParameterTransformer*, and thetransformerstoolset option.file_pathand other derived-parameter flows with preExecute logic and a schema override.file-uploadsandopenapi-transformationsexamples were removed.Written for commit 19e8383. Summary will update on new commits.