release: ship tiny-asl-machine 1.0 with stronger AWS parity #33
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Prepares the tiny-asl-machine 1.0.0 release by expanding AWS-backed conformance coverage, tightening AWS parity semantics (notably around query-language-aware template evaluation), and streamlining contributor/user guidance.
Changes:
- Added many AWS Step Functions snapshot fixtures to broaden dual-environment conformance coverage for Choice operators and related suites.
- Updated template parsing/evaluation utilities (including JSONata wrapper handling and improved parser error reporting).
- Added/adjusted tooling and docs for release workflows (new scripts, agent instructions, lint/format config, and a new “skill” doc).
Reviewed changes
Copilot reviewed 198 out of 1938 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/conformance/cases/.snapshots/aws/* | Adds AWS snapshot fixtures for expanded conformance/parity validation. |
| src/utils/replacePathTemplateFields.ts | Makes template replacement query-language aware and supports JSONata template field evaluation. |
| src/utils/parseStringTemplate.ts | Improves template parser error handling for unmatched braces. |
| src/utils/parseTemplate.spec.ts | Adds tests for unmatched brace error cases. |
| src/utils/parseIntrinsicFunction.ts | Adds intrinsic function signature metadata and tightens argument parsing errors. |
| src/utils/jsonataTemplate.ts | Introduces helpers for detecting/extracting JSONata wrapper templates. |
| src/utils/jsonataTemplate.spec.ts | Adds unit tests for JSONata template helper behavior. |
| src/choices/operators.spec.ts | Aligns Choice-state “no match + no Default” error expectations with AWS behavior. |
| src/utils/runtime.spec.ts | Formatting-only change for UUID regex assertion readability. |
| src/index.ts | Re-exports additional JSONPath/JSONata-specific public types. |
| skills/write-local-state-machine-tests/SKILL.md | Adds concise guidance for writing local state machine tests with mocked resources. |
| AGENTS.md | Adds contributor workflow/testing command guidance. |
| scripts/package.json | Marks scripts/ as ESM to support .ts/.mjs tooling. |
| scripts/build-package-exports.ts | Generates an ESM entrypoint shim for package exports. |
| scripts/aws-remove-stack.ts | Adds a helper script to remove AWS harness stack/resources cleanly. |
| scripts/aws-create-deployment-config.ts | Adds a helper script to create local AWS deployment config scaffolding. |
| scripts/aws-lambda/index.mjs | Adds a Lambda handler that executes harness scripts with AWS SDK clients in scope. |
| .oxlintrc.json | Adds oxlint configuration for repo linting. |
| .oxfmtrc.json | Adds oxfmt configuration including ignore patterns for snapshots/artifacts. |
| pnpm-workspace.yaml | Updates pnpm build allowlist configuration. |
| .github/dependabot.yml | Normalizes quoting/formatting for Dependabot config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (typeof value !== 'string') { | ||
| throw new TypeError(`Expected JSONPath string for template field ${key}`); | ||
| } | ||
|
|
There was a problem hiding this comment.
In JSONata query-language mode, this still evaluates .<field>.$ entries using selectPath(...) (JSONPath). That conflicts with the intent of “query-language-aware template replacement” and can silently apply JSONPath semantics when queryLanguage === 'JSONata'. Consider guarding this branch (e.g., reject .$ fields in JSONata mode with a clear error, or route to the correct evaluator) so the behavior is strictly query-language aware.
| if (queryLanguage === 'JSONata') { | |
| throw new TypeError( | |
| `Template field ${key} uses JSONPath-only '.$' syntax, which is not supported when QueryLanguage is JSONata` | |
| ); | |
| } |
There was a problem hiding this comment.
Still evaluating this one under the AWS-first rule. I have not changed .$ handling in JSONata mode yet because I want an AWS-backed reproduction before either rejecting the shape locally or routing it differently. Leaving this thread open for now instead of making an unproven semantic change.
![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: 147719294d
ℹ️ 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".
| const fields = line.split(delimiter); | ||
| return Object.fromEntries(headers.map((header, index) => [header, fields[index] ?? ''])); |
There was a problem hiding this comment.
Splitting each CSV line with line.split(delimiter) breaks valid CSV records that contain quoted delimiters (for example "last, first") and can shift values into the wrong headers. Since ItemReader advertises CSV support, this causes incorrect map-item payloads for common real-world CSV inputs; row parsing needs quote-aware CSV logic instead of raw string splitting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I agree this is a plausible limitation in the current local CSV materializer, but I am leaving the thread open until I add an AWS-backed reproduction for quoted-field behavior. Per the repo rule for parity work, I do not want to change the parser semantics here without first pinning the symptom with a focused conformance case.
1477192 to
7696aac
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7696aac6fc
ℹ️ 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".
| const rfc3339TimestampPattern = | ||
| /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+-]\d{2}:\d{2})$/; |
There was a problem hiding this comment.
The new IsTimestamp check now relies on a shape-only regex, so invalid timestamps such as 2026-99-99T99:99:99Z are treated as valid and can incorrectly match IsTimestamp: true branches. The prior implementation validated calendar and time ranges; dropping that validation regresses choice accuracy for malformed-but-pattern-matching inputs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I inspected this one again before changing anything. The current code still uses a shape regex for IsTimestamp, and the existing conformance catalog explicitly treats broader timestamp semantics as not yet finalized (see Choice.IsTimestamp case 004 notes). I am leaving this thread open until I can reproduce an AWS-backed mismatch with a focused conformance case, rather than guessing stricter semantics locally.
00d1543 to
5b87fc1
Compare
gabrielmoreira
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b87fc1978
ℹ️ 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".
| isRecord(objectPayload) && 'Body' in objectPayload ? objectPayload.Body : objectPayload; | ||
| const objectString = | ||
| typeof objectBody === 'string' ? objectBody : JSON.stringify(objectBody); | ||
| const parsed = JSON.parse(objectString) as unknown; |
There was a problem hiding this comment.
The LOAD_AND_FLATTEN branch unconditionally does JSON.parse(objectString), so valid configurations with ReaderConfig.InputType set to CSV or JSONL fail at runtime (or are mis-materialized) instead of producing map items. This breaks distributed-map fan-out for non-JSON object collections even though InputType is already read and only PARQUET is explicitly rejected; the parsing logic here needs to branch by InputType (e.g., CSV/JSONL parsers) rather than always assuming JSON.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This looks plausible from the current code path, but I have not changed it yet because I want the same reproduce-first treatment as the other ItemReader issues: add a focused case, then only fix what I can verify without drifting from AWS behavior. Leaving the thread open while I scope that reproduction.
|
|
||
| const payload = await context.Resources?.invoke(itemReader.Resource, parameters); | ||
| const body = isRecord(payload) && 'Body' in payload ? payload.Body : payload; | ||
| const bodyString = typeof body === 'string' ? body : JSON.stringify(body); |
There was a problem hiding this comment.
When s3:getObject returns a non-string body (for example Buffer/Uint8Array from common S3 adapters), this path uses JSON.stringify(body) instead of decoding bytes to text, so JSON/CSV/JSONL parsing operates on serialized metadata rather than file contents. In those environments, ItemReader will fail or produce incorrect rows even with valid objects; the body should be normalized via byte-to-UTF8 decoding instead of JSON stringification.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Also plausible, especially for local adapters that return byte bodies instead of strings, but I have not reproduced it in the current harness yet. I am leaving this open until I can pin it with a focused test and then decide the narrowest safe normalization path.
5b87fc1 to
d8c416b
Compare
…er docs - expand AWS-backed conformance coverage and promote more cases to dual-environment validation - harden query-language-aware JSONata/JSONPath behavior without diverging from observed AWS semantics - simplify public and contributor documentation for the 1.0 release - publish a migration guide from the real npm baseline (0.0.10) and bump package version to 1.0.0 - adopt the repaired Vite+ tooling layer while keeping pnpm scripts as the normal project interface - switch the Vitest launcher to pnpm exec vp test and update related tooling config - aggressively upgrade key tooling and AWS SDK dependencies - fix package publish artifacts by including root types/ and excluding emitted spec files - verify format, lint, typecheck, unit tests, local+AWS conformance, build, and packed artifact shape
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8c416bdab
ℹ️ 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".
| runUntilFinished( | ||
| definition, | ||
| createChildScopeContext(context), | ||
| inputData, |
There was a problem hiding this comment.
This branch execution reuses the parent context without applying the child StateDefinition query-language default, so nested definitions that set QueryLanguage: 'JSONata' at the definition level are still evaluated as JSONPath unless every inner state repeats QueryLanguage. That breaks valid mixed-language workflows (for example JSONata Output/Condition fields in branch states being treated as plain strings). Please derive the child context with the nested definition’s query language before calling runUntilFinished (and mirror this for map iterator/item-processor execution too).
Useful? React with 👍 / 👎.
| const timestamp = | ||
| getQueryLanguage(context, state) === 'JSONata' | ||
| ? String( | ||
| await replacePathTemplateFields( | ||
| state.Timestamp, |
There was a problem hiding this comment.
The JSONata Wait.Timestamp path coerces the evaluated value with String(...) before parsing, so non-string results are silently accepted and fed into Date.parse. Inputs like numeric/object JSONata results can therefore become unintended dates or NaN, leading to wrong delay behavior instead of a clear runtime/query-evaluation error. This should validate the evaluated type as string first, then parse.
Useful? React with 👍 / 👎.
Highlights:
This release is intentionally more AWS-aligned and less permissive than the old published 0.x line in several areas. Users upgrading should review MIGRATION_GUIDE.md and rerun parity-sensitive workflows against AWS when exact behavior matters.