-
Notifications
You must be signed in to change notification settings - Fork 800
Wip triggers #717
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Wip triggers #717
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
| let triggeredData: any = null | ||
|
|
||
| // Mock the callStepFile function to capture triggers | ||
| const originalCallStepFile = require('../call-step-file').callStepFile |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
The best way to address this problem is to remove the unused variable declaration on line 131. Specifically, delete the line:
const originalCallStepFile = require('../call-step-file').callStepFileNo further code changes are needed, since this variable is not referenced anywhere afterwards. There are no dependencies to add, and this change does not affect logic or test integrity.
| @@ -128,7 +128,6 @@ | ||
| let triggeredData: any = null | ||
|
|
||
| // Mock the callStepFile function to capture triggers | ||
| const originalCallStepFile = require('../call-step-file').callStepFile | ||
| jest.spyOn(require('../call-step-file'), 'callStepFile').mockImplementation(async (options: any) => { | ||
| triggeredStep = options.step.config.name | ||
| triggeredData = options.data |
| @@ -0,0 +1,442 @@ | |||
| import { Step, StepConfig } from '../types' | |||
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix this problem, remove the unused StepConfig import from the import statement on line 1. The best way to do this is to edit the import statement so that it imports only Step from '../types', eliminating StepConfig from the list. This change is isolated to the first line of the file and will not alter any existing functionality, as StepConfig is not referenced elsewhere in the shown code.
-
Copy modified line R1
| @@ -1,4 +1,4 @@ | ||
| import { Step, StepConfig } from '../types' | ||
| import { Step } from '../types' | ||
| import { hasApiTrigger, hasEventTrigger, hasCronTrigger, hasStateTrigger, getTriggersByType } from '../guards' | ||
| import { validateStep } from '../step-validator' | ||
|
|
| // Helper functions | ||
| import { Emit, Step } from 'src/types' | ||
| import { isApiStep, isCronStep, isEventStep, isNoopStep } from '../guards' | ||
| import { Emit, Step, Trigger } from 'src/types' |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix this issue, simply remove Trigger from the named imports on line 2 of packages/core/src/helper/flows-helper.ts. No other code changes are required because Trigger is not referenced elsewhere in the file. Leave the rest of the import statement intact so Emit and Step remain imported.
-
Copy modified line R2
| @@ -1,5 +1,5 @@ | ||
| // Helper functions | ||
| import { Emit, Step, Trigger } from 'src/types' | ||
| import { Emit, Step } from 'src/types' | ||
| import { hasApiTrigger, hasEventTrigger, hasCronTrigger, getTriggersByType } from '../guards' | ||
| import { FlowEdge, FlowResponse, FlowStepResponse } from '../types/flows-types' | ||
| import { getStepLanguage } from '../get-step-language' |
| import { ValidationError } from './step-validator' | ||
| import { Step } from './types' | ||
| import { isApiStep, isCronStep, isEventStep, isNoopStep } from './guards' | ||
| import { hasApiTrigger, hasEventTrigger, hasCronTrigger } from './guards' |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix the issue, the unused imports hasApiTrigger, hasEventTrigger, and hasCronTrigger should be removed from the import statement on line 5 of packages/core/src/printer.ts. As none of these are used elsewhere in this code snippet, their removal will not alter the existing functionality of the codebase and will simply make the code cleaner and easier to maintain. No other code changes or new imports are needed.
-
Copy modified line R5
| @@ -2,7 +2,7 @@ | ||
| import path from 'path' | ||
| import { ValidationError } from './step-validator' | ||
| import { Step } from './types' | ||
| import { hasApiTrigger, hasEventTrigger, hasCronTrigger } from './guards' | ||
| // import { hasApiTrigger, hasEventTrigger, hasCronTrigger } from './guards' | ||
| import { Stream } from './types-stream' | ||
|
|
||
| const stepTag = colors.bold(colors.magenta('Step')) |
| import { hasStateTrigger, getTriggersByType } from './guards' | ||
| import { globalLogger } from './logger' | ||
| import { Motia } from './motia' | ||
| import { Event, Step, StateTrigger, InternalStateManager } from './types' |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To resolve the unused import, simply remove Event from the import statement on line 5. Leave the other imported types in place if they are still used. This change should be made within packages/core/src/state-trigger-handler.ts, editing only the relevant import line. No other changes are required, and there is no risk to program behavior, as removing an unused type import has no effect at runtime or for type checking.
-
Copy modified line R5
| @@ -2,7 +2,7 @@ | ||
| import { hasStateTrigger, getTriggersByType } from './guards' | ||
| import { globalLogger } from './logger' | ||
| import { Motia } from './motia' | ||
| import { Event, Step, StateTrigger, InternalStateManager } from './types' | ||
| import { Step, StateTrigger, InternalStateManager } from './types' | ||
|
|
||
| export type MotiaStateManager = { | ||
| createStateHandler: (step: Step) => void |
| } | ||
|
|
||
| const triggerStep = async (step: Step, data: { key: string; value: any; traceId: string; depth: number }) => { | ||
| const { config, filePath } = step |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix this problem, the unused variable should be removed from the destructuring assignment in the triggerStep function. Specifically, on line 242, filePath is destructured along with config, but it is never used. The function only uses config and its name property. The best fix is to change line 242 so that only the config property is destructured from step, leaving out filePath.
No further changes are required elsewhere, since nothing depends on the filePath variable in this context.
-
Copy modified line R242
| @@ -239,7 +239,7 @@ | ||
| } | ||
|
|
||
| const triggerStep = async (step: Step, data: { key: string; value: any; traceId: string; depth: number }) => { | ||
| const { config, filePath } = step | ||
| const { config } = step | ||
| const { name } = config | ||
|
|
||
| globalLogger.debug('[state handler] triggering step', { step: name, key: data.key, depth: data.depth }) |
|
|
||
| try { | ||
| // Generate user ID | ||
| const userId = `user_${Date.now()}_${Math.random().toString(36).substr(2, 9)}` |
Check failure
Code scanning / CodeQL
Insecure randomness High
Math.random()
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix the problem, we need to replace the use of Math.random() with a cryptographically secure random number generator. In Node.js, the recommended way is to use crypto.randomBytes from the built-in crypto module. We'll import crypto, generate a random set of bytes, and convert them to a string format suitable for use in a userId. The rest of the identifier can remain the same, using Date.now() for consistency, but the "random" part should come from cryptographically secure random bytes. Edit only the lines creating the userId (line 40) and add any necessary imports—do not change any other existing logic or interfaces.
-
Copy modified line R3 -
Copy modified lines R40-R41
| @@ -1,6 +1,6 @@ | ||
| import { StepConfig, Handlers } from 'motia' | ||
| import { z } from 'zod' | ||
|
|
||
| import * as crypto from 'crypto'; | ||
| export const config: StepConfig = { | ||
| name: 'UserRegistration', | ||
| description: 'API endpoint to register a new user with initial profile data', | ||
| @@ -37,7 +37,8 @@ | ||
|
|
||
| try { | ||
| // Generate user ID | ||
| const userId = `user_${Date.now()}_${Math.random().toString(36).substr(2, 9)}` | ||
| const randomSuffix = crypto.randomBytes(6).toString('base64').replace(/[^a-zA-Z0-9]/g, '').substr(0, 9); | ||
| const userId = `user_${Date.now()}_${randomSuffix}` | ||
|
|
||
|
|
||
| // Set initial user state - this will trigger multiple state monitors |
| return this.update(traceId, key, (current: T | null) => { | ||
| const obj = current || ({} as T) | ||
| const { [field as keyof T]: _, ...rest } = obj | ||
| return rest as T |
Check failure
Code scanning / CodeQL
Remote property injection High
user-provided value
| const deleteFieldKey = this._makeKey(traceId, operation.key) | ||
| const deleteFieldCurrent = data[deleteFieldKey] ? JSON.parse(data[deleteFieldKey]) as T : ({} as T) | ||
| const { [operation.field as keyof T]: _, ...deleteFieldRest } = deleteFieldCurrent | ||
| data[deleteFieldKey] = JSON.stringify(deleteFieldRest) |
Check failure
Code scanning / CodeQL
Remote property injection High
user-provided value
| async setField<T>(traceId: string, key: string, field: string, value: any): Promise<T> { | ||
| return this.update(traceId, key, (current: T | null) => { | ||
| const obj = current || ({} as T) | ||
| return { ...obj, [field]: value } |
Check failure
Code scanning / CodeQL
Remote property injection High
user-provided value
| case 'setField': | ||
| if (!operation.field) throw new Error('Field required for setField operation') | ||
| const currentObj = (this.state[fullKey] as T) || ({} as T) | ||
| const newObj = { ...currentObj, [operation.field]: operation.value } |
Check failure
Code scanning / CodeQL
Remote property injection High
user-provided value
| }) | ||
|
|
||
| it('should handle missing user gracefully', async () => { | ||
| const consoleSpy = jest.spyOn(console, 'log').mockImplementation() |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix the issue, the unused variable declaration const consoleSpy = jest.spyOn(console, 'log').mockImplementation() on line 779 should be removed from the file playground/integration-tests/complexStateTriggers.spec.ts. This deletion is safe and will not affect test behavior, as its value is never used, nor its effects (mocking, restoring) invoked.
| @@ -776,7 +776,6 @@ | ||
| }) | ||
|
|
||
| it('should handle missing user gracefully', async () => { | ||
| const consoleSpy = jest.spyOn(console, 'log').mockImplementation() | ||
|
|
||
| try { | ||
| // Try to update score for non-existent user |
| } | ||
|
|
||
| export const handler: Handlers['NotificationCleaner'] = async (input, { logger, state }) => { | ||
| const { key, value, traceId } = input |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix the problem, simply remove the unused variable key from destructuring on line 19. Destructure just the needed properties from input — in this case value and traceId. No other code needs to change because the rest of the logic does not use key. The change is restricted to the destructuring line within the handler in playground/steps/complex-state-triggers/notification-cleaner.step.ts.
-
Copy modified line R19
| @@ -16,7 +16,7 @@ | ||
| } | ||
|
|
||
| export const handler: Handlers['NotificationCleaner'] = async (input, { logger, state }) => { | ||
| const { key, value, traceId } = input | ||
| const { value, traceId } = input | ||
|
|
||
| // Extract userId from the traceId (format: user_123_abc) | ||
| const userId = traceId |
| }] : []) | ||
| ] | ||
|
|
||
| const cleanupResult = await state.batch(userId, cleanupOperations) |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix the problem, simply remove the unused variable cleanupResult by either omitting the assignment altogether or, if the side effect of calling state.batch(...) is still required, call it as an expression and await its completion without assigning its result to any variable. Because only the assignment is flagged, and the function must still perform the atomic batch operation for notifications cleanup, the best fix is to call await state.batch(userId, cleanupOperations) without assigning its result to any variable. Only modify line 66 of playground/steps/complex-state-triggers/notification-cleaner.step.ts; no additional imports or definitions are required.
-
Copy modified line R66
| @@ -63,7 +63,7 @@ | ||
| }] : []) | ||
| ] | ||
|
|
||
| const cleanupResult = await state.batch(userId, cleanupOperations) | ||
| await state.batch(userId, cleanupOperations) | ||
|
|
||
| // Reduced logging for test performance | ||
| // logger.info('Notifications cleaned up atomically', { |
| // Clean up temp file if it exists | ||
| try { | ||
| fs.unlinkSync(tempPath) | ||
| } catch { |
Check failure
Code scanning / CodeQL
Insecure temporary file High
the os temp dir
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
The most robust way to solve this issue is to use the tmp library to securely create a unique temporary file with proper permissions and guaranteed non-existence before creation. Replace the manual .tmp approach in _writeFile with tmp.fileSync() to generate the temp file for atomic writes. This change is localized to the _writeFile method in packages/core/src/state/adapters/default-state-adapter.ts. You need to:
- Add an import for the
tmppackage at the top of the file. - Replace the logic in
_writeFileso that it creates a secure temp file withtmp.fileSync(), writes the JSON data to that pathname, and then performs the atomic rename. - Remember to clean up the temp file handle as well as the path if an error is thrown.
- No changes to tests are required.
-
Copy modified line R3 -
Copy modified lines R410-R411 -
Copy modified lines R413-R414 -
Copy modified lines R416-R417 -
Copy modified lines R421-R423
| @@ -1,5 +1,6 @@ | ||
| import fs from 'fs' | ||
| import * as path from 'path' | ||
| import tmp from 'tmp' | ||
| import { StateAdapter, StateItem, StateItemsInput } from '../state-adapter' | ||
| import { StateOperation, BatchOperation, TransactionResult, BatchResult } from '../../types' | ||
| import { filterItem, inferType } from './utils' | ||
| @@ -404,19 +405,22 @@ | ||
| } | ||
|
|
||
| private _writeFile(data: unknown) { | ||
| const tempPath = this.filePath + '.tmp' | ||
| const jsonData = JSON.stringify(data, null, 2) | ||
|
|
||
| // Use tmp for secure temp file creation | ||
| let tempFile | ||
| try { | ||
| // Write to temporary file first | ||
| fs.writeFileSync(tempPath, jsonData, 'utf-8') | ||
|
|
||
| tempFile = tmp.fileSync({ dir: path.dirname(this.filePath), prefix: path.basename(this.filePath) + '-', postfix: '.tmp' }) | ||
| fs.writeFileSync(tempFile.name, jsonData, 'utf-8') | ||
| // Atomically rename temp file to final location (atomic on POSIX systems) | ||
| fs.renameSync(tempPath, this.filePath) | ||
| fs.renameSync(tempFile.name, this.filePath) | ||
| tempFile.removeCallback() | ||
| } catch (error) { | ||
| // Clean up temp file if it exists | ||
| try { | ||
| fs.unlinkSync(tempPath) | ||
| if (tempFile && tempFile.removeCallback) { | ||
| tempFile.removeCallback() | ||
| } | ||
| } catch { | ||
| // Ignore cleanup errors | ||
| } |
-
Copy modified lines R30-R31
| @@ -27,7 +27,8 @@ | ||
| "uuid": "^11.1.0", | ||
| "ws": "^8.18.2", | ||
| "zod": "^3.24.1", | ||
| "zod-to-json-schema": "^3.24.1" | ||
| "zod-to-json-schema": "^3.24.1", | ||
| "tmp": "^0.2.5" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/body-parser": "^1.19.5", |
| Package | Version | Security advisories |
| tmp (npm) | 0.2.5 | None |
Check failure
Code scanning / CodeQL
Insecure temporary file High
the os temp dir
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix the problem, the file adapter should never create files or temporary files in the OS temp directory or in any location with a custom mechanism, unless steps are taken to guarantee atomicity, uniqueness, and proper permissions. Standard practice for securely writing temporary files is to use a well-tested library such as tmp. This library ensures temporary files are created with secure permissions, are unique, and are inaccessible to other users on the system.
To fix the current problem:
- Import the
tmplibrary indefault-state-adapter.ts. - In the
_writeFilemethod, instead of constructingtempPathmanually, usetmp.fileSync()to get a securely created temporary file, and write your data to that. - Atomically move/replace the file at the desired path.
- Remove the manual temp naming logic.
Only replace relevant code and add the minimum imports required, staying inside the code you've been shown.
-
Copy modified line R3 -
Copy modified lines R411-R413 -
Copy modified line R416
| @@ -1,5 +1,6 @@ | ||
| import fs from 'fs' | ||
| import * as path from 'path' | ||
| import tmp from 'tmp' | ||
| import { StateAdapter, StateItem, StateItemsInput } from '../state-adapter' | ||
| import { StateOperation, BatchOperation, TransactionResult, BatchResult } from '../../types' | ||
| import { filterItem, inferType } from './utils' | ||
| @@ -404,23 +405,16 @@ | ||
| } | ||
|
|
||
| private _writeFile(data: unknown) { | ||
| const tempPath = this.filePath + '.tmp' | ||
| const jsonData = JSON.stringify(data, null, 2) | ||
|
|
||
| try { | ||
| // Write to temporary file first | ||
| fs.writeFileSync(tempPath, jsonData, 'utf-8') | ||
| // Securely create temp file, write data into it | ||
| const tempFile = tmp.fileSync({ prefix: 'state-adapter-', postfix: '.tmp' }) | ||
| fs.writeFileSync(tempFile.name, jsonData, 'utf-8') | ||
|
|
||
| // Atomically rename temp file to final location (atomic on POSIX systems) | ||
| fs.renameSync(tempPath, this.filePath) | ||
| fs.renameSync(tempFile.name, this.filePath) | ||
| } catch (error) { | ||
| // Clean up temp file if it exists | ||
| try { | ||
| fs.unlinkSync(tempPath) | ||
| } catch { | ||
| // Ignore cleanup errors | ||
| } | ||
|
|
||
| // Fallback: try to initialize and write directly | ||
| this.init() | ||
| fs.writeFileSync(this.filePath, jsonData, 'utf-8') |
-
Copy modified lines R30-R31
| @@ -27,7 +27,8 @@ | ||
| "uuid": "^11.1.0", | ||
| "ws": "^8.18.2", | ||
| "zod": "^3.24.1", | ||
| "zod-to-json-schema": "^3.24.1" | ||
| "zod-to-json-schema": "^3.24.1", | ||
| "tmp": "^0.2.5" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/body-parser": "^1.19.5", |
| Package | Version | Security advisories |
| tmp (npm) | 0.2.5 | None |
No description provided.