-
Notifications
You must be signed in to change notification settings - Fork 230
Add serde compliance tooling and improve custom class serialization DX #1552
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
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
e6151c8
Add serde compliance tooling and update skill documentation
TooTallNate 13023fa
Fix --json + --strict: use process.exitCode so JSON output is returne…
TooTallNate dc157b9
Address code review feedback
TooTallNate b56c506
Use .gitignore for validate file discovery; fix changeset wording
TooTallNate File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@workflow/cli": patch | ||
| --- | ||
|
|
||
| Add `workflow transform` command for inspecting SWC transform output with optional serde compliance analysis |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@workflow/cli": patch | ||
| --- | ||
|
|
||
| Implement serde compliance checks in `workflow validate` |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@workflow/builders": patch | ||
| --- | ||
|
|
||
| Add serde compliance checker (`analyzeSerdeCompliance`) and build-time warnings for classes with Node.js imports in workflow bundle |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,200 @@ | ||
| /** | ||
| * Serde compliance checker for workflow custom class serialization. | ||
| * | ||
| * Analyzes source code to determine if classes with WORKFLOW_SERIALIZE / | ||
| * WORKFLOW_DESERIALIZE are correctly set up for the workflow sandbox. | ||
| * | ||
| * Used by: | ||
| * - CLI `validate` command | ||
| * - CLI `transform` command (--check-serde) | ||
| * - SWC playground serde analysis panel | ||
| * - Build-time warnings in BaseBuilder | ||
| */ | ||
|
|
||
| import builtinModules from 'builtin-modules'; | ||
| import type { WorkflowManifest } from './apply-swc-transform.js'; | ||
|
|
||
| // Build a regex that matches Node.js built-in module imports in transformed code. | ||
| // Handles both ESM (`from 'fs'`, `from 'node:fs'`) and CJS (`require('fs')`) | ||
| const nodeBuiltins = builtinModules.join('|'); | ||
|
|
||
| // Regex to extract specific module names from import/require statements | ||
| const nodeImportExtractRegex = new RegExp( | ||
| `(?:from\\s+['"](?:node:)?((?:${nodeBuiltins})(?:/[^'"]*)?)['"]` + | ||
| `|require\\s*\\(\\s*['"](?:node:)?((?:${nodeBuiltins})(?:/[^'"]*)?)['"]\\s*\\))`, | ||
| 'g' | ||
| ); | ||
|
|
||
| // Regex to detect class registration IIFEs generated by the SWC plugin | ||
| const registrationIifeRegex = | ||
| /Symbol\.for\s*\(\s*["']workflow-class-registry["']\s*\)/; | ||
|
|
||
| /** | ||
| * Result of checking a single class for serde compliance. | ||
| */ | ||
| export interface SerdeClassCheckResult { | ||
| /** The class name as detected in the source */ | ||
| className: string; | ||
| /** The classId assigned by the SWC plugin (from the manifest) */ | ||
| classId: string; | ||
| /** Whether the SWC plugin detected serde symbols on this class */ | ||
| detected: boolean; | ||
| /** Whether a registration IIFE was generated in the output */ | ||
| registered: boolean; | ||
| /** | ||
| * Node.js built-in module imports remaining in the workflow-mode output. | ||
| * If non-empty, the class is NOT workflow-sandbox compliant. | ||
| */ | ||
| nodeImports: string[]; | ||
| /** Whether the class passes all compliance checks */ | ||
| compliant: boolean; | ||
| /** Human-readable descriptions of any issues found */ | ||
| issues: string[]; | ||
| } | ||
|
|
||
| /** | ||
| * Full result of serde compliance analysis for a source file. | ||
| */ | ||
| export interface SerdeCheckResult { | ||
| /** Per-class analysis results */ | ||
| classes: SerdeClassCheckResult[]; | ||
| /** All Node.js built-in imports found in the workflow-mode output */ | ||
| globalNodeImports: string[]; | ||
| /** Whether the workflow-mode output contains any serde-related classes */ | ||
| hasSerdeClasses: boolean; | ||
| /** The raw workflow manifest extracted from the SWC transform */ | ||
| manifest: WorkflowManifest; | ||
| } | ||
|
|
||
| /** | ||
| * Lightweight serde compliance checker that works with pre-computed | ||
| * SWC transform results. This avoids re-running the SWC transform | ||
| * when the caller already has the outputs (e.g., the playground or builder). | ||
| */ | ||
| export function analyzeSerdeCompliance(options: { | ||
| /** Source code (used for pattern detection) */ | ||
| sourceCode: string; | ||
| /** Workflow-mode transformed output */ | ||
| workflowCode: string; | ||
| /** Manifest extracted from the SWC transform */ | ||
| manifest: WorkflowManifest; | ||
| }): SerdeCheckResult { | ||
| const { sourceCode, workflowCode, manifest } = options; | ||
|
|
||
| // 1. Extract all Node.js built-in imports from the workflow output | ||
| const globalNodeImports = extractNodeImports(workflowCode); | ||
|
|
||
| // 2. Check if the manifest contains any serde-registered classes | ||
| const classEntries = extractClassEntries(manifest); | ||
| const hasSerdeClasses = classEntries.length > 0; | ||
|
|
||
| // 3. Check if the workflow output contains registration IIFEs | ||
| const hasRegistration = registrationIifeRegex.test(workflowCode); | ||
|
|
||
| // 4. Analyze each class | ||
| const classes: SerdeClassCheckResult[] = classEntries.map((entry) => { | ||
| const issues: string[] = []; | ||
|
|
||
| // Check for Node.js imports (these will fail in the workflow sandbox) | ||
| if (globalNodeImports.length > 0) { | ||
| issues.push( | ||
| `Workflow bundle contains Node.js built-in imports: ${globalNodeImports.join(', ')}. ` + | ||
| `These will fail at runtime in the workflow sandbox. ` + | ||
| `Add "use step" to methods that depend on Node.js APIs so they are stripped from the workflow bundle.` | ||
| ); | ||
| } | ||
|
|
||
| // Check for registration | ||
| if (!hasRegistration) { | ||
| issues.push( | ||
| `No class registration IIFE was generated. ` + | ||
| `Ensure WORKFLOW_SERIALIZE and WORKFLOW_DESERIALIZE are defined as static methods ` + | ||
| `inside the class body using computed property syntax: static [WORKFLOW_SERIALIZE](...) { ... }` | ||
| ); | ||
| } | ||
|
|
||
| return { | ||
| className: entry.className, | ||
| classId: entry.classId, | ||
| detected: true, | ||
| registered: hasRegistration, | ||
| nodeImports: globalNodeImports, | ||
| compliant: globalNodeImports.length === 0 && hasRegistration, | ||
| issues, | ||
| }; | ||
| }); | ||
|
|
||
| // 5. Check for classes that have serde patterns in source but weren't detected by SWC | ||
| const sourceHasSerdePatterns = | ||
| /\[\s*WORKFLOW_(?:SERIALIZE|DESERIALIZE)\s*\]/.test(sourceCode) || | ||
| /Symbol\.for\s*\(\s*['"]workflow-(?:serialize|deserialize)['"]\s*\)/.test( | ||
| sourceCode | ||
| ); | ||
|
|
||
| if (sourceHasSerdePatterns && classEntries.length === 0) { | ||
| classes.push({ | ||
| className: '<unknown>', | ||
| classId: '', | ||
| detected: false, | ||
| registered: false, | ||
| nodeImports: globalNodeImports, | ||
| compliant: false, | ||
| issues: [ | ||
| `Source code contains WORKFLOW_SERIALIZE/WORKFLOW_DESERIALIZE patterns but ` + | ||
| `the SWC plugin did not detect any serde-enabled classes. ` + | ||
| `Ensure the symbols are defined as static methods INSIDE the class body, ` + | ||
| `not assigned externally (e.g., (MyClass as any)[WORKFLOW_SERIALIZE] = ...).`, | ||
| ], | ||
| }); | ||
| } | ||
|
|
||
| return { | ||
| classes, | ||
| globalNodeImports, | ||
| hasSerdeClasses, | ||
| manifest, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Extract Node.js built-in module names from transformed code. | ||
| */ | ||
| function extractNodeImports(code: string): string[] { | ||
| const imports = new Set<string>(); | ||
| // Reset regex state | ||
| nodeImportExtractRegex.lastIndex = 0; | ||
| for ( | ||
| let match = nodeImportExtractRegex.exec(code); | ||
| match !== null; | ||
| match = nodeImportExtractRegex.exec(code) | ||
| ) { | ||
| // match[1] is from the ESM pattern, match[2] is from the CJS pattern | ||
| const moduleName = match[1] || match[2]; | ||
| if (moduleName) { | ||
| // Normalize to base module name (e.g., 'fs/promises' -> 'fs') | ||
| imports.add(moduleName.split('/')[0]); | ||
| } | ||
| } | ||
| return [...imports].sort(); | ||
| } | ||
|
|
||
| /** | ||
| * Extract class entries from a WorkflowManifest. | ||
| */ | ||
| export function extractClassEntries( | ||
| manifest: WorkflowManifest | ||
| ): Array<{ className: string; classId: string; fileName: string }> { | ||
| const entries: Array<{ | ||
| className: string; | ||
| classId: string; | ||
| fileName: string; | ||
| }> = []; | ||
| if (!manifest.classes) return entries; | ||
|
|
||
| for (const [fileName, classes] of Object.entries(manifest.classes)) { | ||
| for (const [className, { classId }] of Object.entries(classes)) { | ||
| entries.push({ className, classId, fileName }); | ||
| } | ||
| } | ||
| return entries; | ||
| } |
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.