feat!: bump to v2 - require Node.js >= 20, upgrade all dependencies#67
feat!: bump to v2 - require Node.js >= 20, upgrade all dependencies#67tinovyatkin merged 1 commit intomainfrom
Conversation
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. CodeRabbit does not review PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughUpgrades CI/tooling and developer toolchain: Node → lts (>=20.9.0), TypeScript → ~6.0, ES target → ES2024; swaps several Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 1
🧹 Nitpick comments (1)
package.json (1)
96-97: Simplify redundant Jest glob.Line 96 and Line 97 use
@(src|src), which is equivalent to justsrc.Proposed cleanup
- "<rootDir>/@(src|src)/**/*(*.)@(spec|test).ts?(x)", - "<rootDir>/@(src|src)/**/__tests__/**/*.ts?(x)", + "<rootDir>/src/**/*(*.)@(spec|test).ts?(x)", + "<rootDir>/src/**/__tests__/**/*.ts?(x)",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 96 - 97, The two Jest glob patterns using the redundant group "@(src|src)" should be simplified to "src" to avoid needless complexity; update the two entries "\"<rootDir>/@(src|src)/**/*(*.)@(spec|test).ts?(x)\"" and "\"<rootDir>/@(src|src)/**/__tests__/**/*.ts?(x)\"" in package.json to use "\"<rootDir>/src/...\"" equivalents (preserve the rest of each pattern like "/**/*(*.)@(spec|test).ts?(x)" and "/**/__tests__/**/*.ts?(x)").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pull-request-lint.yml:
- Line 22: Update the workflow step that references "uses:
amannn/action-semantic-pull-request@v6" to a specific, Node-24–compatible
release (e.g., `@v6.1.1`) or roll back to the last Node-16/18-compatible major
(e.g., `@v5`) after verifying your runner environment; explicitly test on the
intended GitHub Actions runner to confirm it provides Node.js 24 runtime or
adjust the action version accordingly and update the workflow comment to
document the Node.js runtime requirement.
---
Nitpick comments:
In `@package.json`:
- Around line 96-97: The two Jest glob patterns using the redundant group
"@(src|src)" should be simplified to "src" to avoid needless complexity; update
the two entries "\"<rootDir>/@(src|src)/**/*(*.)@(spec|test).ts?(x)\"" and
"\"<rootDir>/@(src|src)/**/__tests__/**/*.ts?(x)\"" in package.json to use
"\"<rootDir>/src/...\"" equivalents (preserve the rest of each pattern like
"/**/*(*.)@(spec|test).ts?(x)" and "/**/__tests__/**/*.ts?(x)").
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 98db5a68-004b-445c-a75e-1c944fca4f43
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
.eslintrc.yml.gitattributes.github/workflows/build.yml.github/workflows/pull-request-lint.yml.github/workflows/release.yml.github/workflows/upgrade-main.yml.gitignore.mergify.yml.npmignore.projen/deps.json.projen/tasks.json.projenrc.tsLICENSEpackage.jsonsrc/index.test.tssrc/index.tstsconfig.json
tinovyatkin
force-pushed
the
chore/major-version-bump-node20
branch
2 times, most recently
from
2b009de to
a484187
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package.json (1)
96-99: Redundant pattern in testMatch globs.The pattern
@(src|src)is redundant - it matches "src" or "src", which is just "src". This appears to be a projen generation artifact and doesn't affect functionality, but is unnecessarily verbose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 96 - 99, The testMatch globs in package.json include a redundant alternation "@(src|src)"; update those patterns to use just "src" (e.g., replace "@(src|src)" with "src") so the four entries referencing "@(src|src)" become simpler and equivalent; locate the testMatch array entries in package.json and update the string literals accordingly to remove the duplicate alternation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@package.json`:
- Around line 96-99: The testMatch globs in package.json include a redundant
alternation "@(src|src)"; update those patterns to use just "src" (e.g., replace
"@(src|src)" with "src") so the four entries referencing "@(src|src)" become
simpler and equivalent; locate the testMatch array entries in package.json and
update the string literals accordingly to remove the duplicate alternation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 54053ffb-8de5-4393-b167-5ed0188eee2a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
.eslintrc.yml.gitattributes.github/workflows/build.yml.github/workflows/pull-request-lint.yml.github/workflows/release.yml.github/workflows/upgrade-main.yml.gitignore.mergify.yml.npmignore.projen/deps.json.projen/tasks.json.projenrc.tsLICENSEpackage.jsonsrc/index.test.tssrc/index.tstsconfig.json
✅ Files skipped from review due to trivial changes (8)
- LICENSE
- .npmignore
- .gitignore
- tsconfig.json
- .gitattributes
- src/index.ts
- .github/workflows/build.yml
- .eslintrc.yml
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/workflows/pull-request-lint.yml
- .mergify.yml
- src/index.test.ts
- .projen/deps.json
- .github/workflows/release.yml
tinovyatkin
force-pushed
the
chore/major-version-bump-node20
branch
from
a484187 to
e4eb197
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.github/workflows/release.yml (1)
68-72:release_githubnow depends onrelease_npmcompletion.Adding
release_npmas a dependency ensures the GitHub release is only created after successful npm publication. This provides a more reliable release sequence—if npm publish fails, no GitHub release is created.However, this creates a strict ordering that may not be desired. If npm publish fails, you won't get a GitHub release even though the build artifacts are valid. Consider whether this dependency is intentional.
If independent releases are preferred (GitHub release even if npm fails):
needs: - release - - release_npm🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 68 - 72, The job dependency was changed so release_github now waits for release_npm; decide desired behavior and update the release_github job: if you want GitHub releases only after npm succeeds, keep needs: [release, release_npm] (no change); if you want GitHub releases to run regardless of npm result, remove release_npm from the needs array or keep the needs but add an if: always() condition on the release_github job so it runs even when release_npm fails; reference the release_github job and release_npm job names when making the change.package.json (1)
54-54:ts-jestappears unused—consider removing.The Jest transform configuration uses
@swc/jest, butts-jest@^29remains indevDependencies. Ifts-jestis no longer used, it should be removed to reduce dependency footprint.♻️ Remove unused ts-jest dependency
In
.projenrc.ts, removets-jestfrom devDeps if it's not needed elsewhere:devDeps: [ "@aws-amplify/amplify-appsync-simulator@^2.16.18", "@swc/core", "@swc/jest", "graphql", "prettier-plugin-organize-imports@^4", "prettier-plugin-organize-attributes", - // ts-jest is implicitly added by projen but no longer used ],Note: If projen automatically adds
ts-jest, you may need to explicitly exclude it or this may require projen configuration changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 54, Remove the unused "ts-jest" devDependency from package.json (the entry "ts-jest": "^29") since the Jest transform uses "@swc/jest"; update deps by running your package manager to refresh lockfile. Also check .projenrc.ts (if present) for any code that adds "ts-jest" to devDeps and remove or disable that emission (adjust Projen config so it no longer reintroduces ts-jest). Finally, run install and a test run to confirm no breakage and commit the updated package.json and lockfile.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pull-request-lint.yml:
- Around line 13-20: The workflow declares merge_group but the validate job's if
condition excludes merge-queue events so merge_group is a no-op; either remove
the merge_group key or update the validate job's if expression to include merge
queue events (adjust the if on the job named "validate") so merge_group actually
triggers; locate the merge_group declaration and the validate job's if condition
in the file and either delete merge_group or change the if expression on
validate to something that includes the merge-queue event (so merge_group is
effective).
- Line 22: Replace the floating tag in the GitHub Action usage to a pinned
commit SHA: find the uses string "amannn/action-semantic-pull-request@v6" and
update it to the full commit SHA for that action (e.g.,
"amannn/action-semantic-pull-request@<full-commit-sha>"); ensure this change is
made in the pull request lint workflow that uses the pull_request_target trigger
so the action reference is deterministic and not a floating tag.
In `@package.json`:
- Around line 35-36: The pipeline failed to find `@swc/jest` despite entries in
package.json; run npm install (or the team's package manager command) to
regenerate package-lock.json after updating .projenrc.ts, verify `@swc/jest` and
`@swc/core` appear in devDependencies in package.json and in package-lock.json,
resolve any version conflicts (check for npm audit or peer/resolution issues),
commit the updated package-lock.json, and re-run the pipeline; if your repo uses
yarn/PNPM, perform the equivalent lockfile regeneration and commit instead.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 68-72: The job dependency was changed so release_github now waits
for release_npm; decide desired behavior and update the release_github job: if
you want GitHub releases only after npm succeeds, keep needs: [release,
release_npm] (no change); if you want GitHub releases to run regardless of npm
result, remove release_npm from the needs array or keep the needs but add an if:
always() condition on the release_github job so it runs even when release_npm
fails; reference the release_github job and release_npm job names when making
the change.
In `@package.json`:
- Line 54: Remove the unused "ts-jest" devDependency from package.json (the
entry "ts-jest": "^29") since the Jest transform uses "@swc/jest"; update deps
by running your package manager to refresh lockfile. Also check .projenrc.ts (if
present) for any code that adds "ts-jest" to devDeps and remove or disable that
emission (adjust Projen config so it no longer reintroduces ts-jest). Finally,
run install and a test run to confirm no breakage and commit the updated
package.json and lockfile.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 96ffcc81-c3cf-4832-9feb-3c7eba4b4a1c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
.eslintrc.yml.gitattributes.github/workflows/build.yml.github/workflows/pull-request-lint.yml.github/workflows/release.yml.github/workflows/upgrade-main.yml.gitignore.mergify.yml.npmignore.projen/deps.json.projen/tasks.json.projenrc.tsLICENSEpackage.jsonsrc/index.test.tssrc/index.tstsconfig.json
✅ Files skipped from review due to trivial changes (7)
- LICENSE
- .gitignore
- .npmignore
- .gitattributes
- tsconfig.json
- src/index.ts
- .github/workflows/build.yml
🚧 Files skipped from review as they are similar to previous changes (4)
- .eslintrc.yml
- .mergify.yml
- src/index.test.ts
- .github/workflows/upgrade-main.yml
| if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') | ||
| steps: | ||
| - uses: amannn/action-semantic-pull-request@v5.0.2 | ||
| - uses: amannn/action-semantic-pull-request@v6 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify current reference is floating (non-SHA)
rg -n 'uses:\s*amannn/action-semantic-pull-request@' .github/workflows/pull-request-lint.yml
# Resolve the current v6 tag target so you can pin explicitly
gh api repos/amannn/action-semantic-pull-request/git/matching-refs/tags/v6 --jq '.[].ref + " -> " + .object.sha'Repository: tinovyatkin/appsync-client-node
Length of output: 291
🏁 Script executed:
cat -n .github/workflows/pull-request-lint.ymlRepository: tinovyatkin/appsync-client-node
Length of output: 1003
🏁 Script executed:
# Check all actions in workflow files to see if pinning is standard
rg 'uses:\s+' .github/workflows/ -A 0
# Try to find the commit SHA from a public source (without auth)
git ls-remote --tags https://github.com/amannn/action-semantic-pull-request.git v6 2>/dev/null || echo "Cannot resolve via git ls-remote"Repository: tinovyatkin/appsync-client-node
Length of output: 1613
Pin the third-party action to a commit SHA.
Line 22 uses a floating major tag (@v6) in a pull_request_target workflow, which allows silent upstream changes. The pull_request_target trigger (line 5) runs untrusted PR code with access to GITHUB_TOKEN secrets, making this a supply chain attack surface. Pin to the full commit SHA for deterministic and safer CI behavior.
Suggested adjustment
- - uses: amannn/action-semantic-pull-request@v6
+ - uses: amannn/action-semantic-pull-request@48f256284bd46cdaab1048c3721360e808335d50🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pull-request-lint.yml at line 22, Replace the floating tag
in the GitHub Action usage to a pinned commit SHA: find the uses string
"amannn/action-semantic-pull-request@v6" and update it to the full commit SHA
for that action (e.g., "amannn/action-semantic-pull-request@<full-commit-sha>");
ensure this change is made in the pull request lint workflow that uses the
pull_request_target trigger so the action reference is deterministic and not a
floating tag.
e4eb197 to
d587143
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.eslintrc.yml (1)
12-12: UpdateecmaVersionto match ES2024 target.The
ecmaVersionis set to2018, but the PR upgrades the TypeScript target to ES2024. While@typescript-eslint/parserhandles modern syntax, updatingecmaVersionto2024ensures ESLint recognizes newer JavaScript features consistently.Additionally, upgrade
eslint-plugin-importfrom ^2.29.1 to ^2.32.0+, which declares full ESLint 9 compatibility. The current version technically works with ESLint 9, but the newer version provides explicit support for the major version upgrade.The rule configurations in
.eslintrc.ymlare correct—@typescript-eslint/no-shadowand@typescript-eslint/return-awaitare properly enabled with their base rules disabled, andimport/no-duplicatesis compatible with the installed versions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.eslintrc.yml at line 12, Update .eslintrc.yml's ecmaVersion from 2018 to 2024 so ESLint recognizes ES2024 syntax and aligns with the TypeScript target; additionally, bump the eslint-plugin-import dependency in package.json from ^2.29.1 to ^2.32.0+ to get explicit ESLint 9 compatibility. Locate the ecmaVersion setting in .eslintrc.yml and change its value to 2024, then update the eslint-plugin-import entry in package.json and run npm/yarn install to refresh lockfile; keep existing rule configurations for `@typescript-eslint/no-shadow`, `@typescript-eslint/return-await`, and import/no-duplicates unchanged.package.json (1)
97-102: Redundant extglob patterns intestMatch.
@(src|src)(lines 98-99) is redundant — it matches "src" or "src". Similarly,@(projenrc)(lines 100-101) has only one option. Since this file is projen-generated, this may be an upstream quirk. Not a functional issue, just verbose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 97 - 102, The testMatch entries contain redundant extglob alternatives like "@(src|src)" and "@(projenrc)" — update the patterns in the testMatch array to use the single directory names directly (e.g. replace "<rootDir>/@(src|src)/**/*(*.)@(spec|test).ts?(x)" with "<rootDir>/src/**/*(*.)@(spec|test).ts?(x)" and similarly replace the projenrc patterns to "<rootDir>/projenrc/**/*(*.)@(spec|test).ts?(x)" and the corresponding "__tests__" patterns) so the patterns in package.json are simplified while preserving the same matching behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 70-76: The current workflow dead-ends reruns because
release_github depends on release_npm while both use
needs.release.outputs.tag_exists; update the job dependencies so release_github
is not gated by release_npm (remove release_npm from release_github's needs) or
make release_npm idempotent by adding a pre-check that skips npm publish when
the package/version is already published (e.g., check registry for the version
or add a needs.release.outputs.tag_exists check inside the release_npm job);
ensure the condition using needs.release.outputs.tag_exists and
needs.release.outputs.latest_commit remains the sole gate for release_github (or
replicate that idempotency check in release_github) so a partial npm publish
won't block the GitHub release job.
- Around line 45-50: The step check_tag_exists masks real failures by swallowing
git ls-remote errors and treating them as exists=false; instead enable strict
shell mode (set -euo pipefail) at the top of the job/step, explicitly validate
TAG from dist/releasetag.txt is non-empty, run git ls-remote --tags origin
"$TAG" and let its non-zero exit code fail the step (don’t wrap it into a
conditional that converts failures to exists=false), and only set exists=true
when git ls-remote succeeds; also avoid unprotected pipes (use temporary
variables or check command exit statuses) so a failed git or pipe will fail the
workflow rather than silently defaulting.
In @.github/workflows/upgrade-main.yml:
- Around line 61-62: The current "Apply patch" step uses '[ -s ${{ runner.temp
}}/repo.patch ] && git apply ${{ runner.temp }}/repo.patch || echo "Empty patch.
Skipping."' which masks real git apply errors as "Empty patch". Replace that
chained command so that you first test whether runner.temp/repo.patch is
non-empty and only then run git apply, but do not attach a fallback echo to the
same conditional that would swallow git apply failures; let git apply exit
non‑zero and fail the job if it errors, and only print "Empty patch. Skipping."
when the patch file is truly empty. Ensure the step references git apply and the
runner.temp/repo.patch path so the new conditional logic replaces the current
combined &&/|| chain.
In `@package.json`:
- Around line 54-56: Remove the unused "ts-jest" dependency from package.json
and update the Jest testMatch patterns to remove the redundant alternation:
delete the "ts-jest" entry (since the Jest transform uses "@swc/jest") and
replace occurrences of the pattern using "@(src|src)" with just "src" in the
"testMatch" array so the patterns read against "src" only; this ensures the
package.json dependencies and Jest configuration (transform using "@swc/jest"
and testMatch entries) are consistent and simplified.
---
Nitpick comments:
In @.eslintrc.yml:
- Line 12: Update .eslintrc.yml's ecmaVersion from 2018 to 2024 so ESLint
recognizes ES2024 syntax and aligns with the TypeScript target; additionally,
bump the eslint-plugin-import dependency in package.json from ^2.29.1 to
^2.32.0+ to get explicit ESLint 9 compatibility. Locate the ecmaVersion setting
in .eslintrc.yml and change its value to 2024, then update the
eslint-plugin-import entry in package.json and run npm/yarn install to refresh
lockfile; keep existing rule configurations for `@typescript-eslint/no-shadow`,
`@typescript-eslint/return-await`, and import/no-duplicates unchanged.
In `@package.json`:
- Around line 97-102: The testMatch entries contain redundant extglob
alternatives like "@(src|src)" and "@(projenrc)" — update the patterns in the
testMatch array to use the single directory names directly (e.g. replace
"<rootDir>/@(src|src)/**/*(*.)@(spec|test).ts?(x)" with
"<rootDir>/src/**/*(*.)@(spec|test).ts?(x)" and similarly replace the projenrc
patterns to "<rootDir>/projenrc/**/*(*.)@(spec|test).ts?(x)" and the
corresponding "__tests__" patterns) so the patterns in package.json are
simplified while preserving the same matching behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 75cfb935-501f-425c-8be5-1f96d2b6b122
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
.eslintrc.yml.gitattributes.github/workflows/build.yml.github/workflows/pull-request-lint.yml.github/workflows/release.yml.github/workflows/upgrade-main.yml.gitignore.mergify.yml.npmignore.projen/deps.json.projen/tasks.json.projenrc.tsLICENSEpackage.jsonsrc/index.test.tssrc/index.tstsconfig.json
✅ Files skipped from review due to trivial changes (4)
- LICENSE
- .gitignore
- .github/workflows/build.yml
- .npmignore
🚧 Files skipped from review as they are similar to previous changes (9)
- .mergify.yml
- .github/workflows/pull-request-lint.yml
- .gitattributes
- src/index.test.ts
- tsconfig.json
- .projen/tasks.json
- .projenrc.ts
- .projen/deps.json
- src/index.ts
| - name: Check if version has already been tagged | ||
| id: check_tag_exists | ||
| run: |- | ||
| TAG=$(cat dist/releasetag.txt) | ||
| ([ ! -z "$TAG" ] && git ls-remote -q --exit-code --tags origin $TAG && (echo "exists=true" >> $GITHUB_OUTPUT)) || (echo "exists=false" >> $GITHUB_OUTPUT) | ||
| cat $GITHUB_OUTPUT |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "# Current workflow lines"
nl -ba .github/workflows/release.yml | sed -n '45,56p'
echo
echo "# Line 49 pattern: any non-zero status falls through to exists=false"
bash -lc 'set +e; out=$(mktemp); (false && echo "exists=true" >> "$out") || echo "exists=false" >> "$out"; cat "$out"'
echo
echo "# Line 54 pattern: without pipefail, an upstream failure still yields status 0"
bash -lc 'set +e; value="$(false | cut -f1)"; status=$?; printf "status=%s value=<%s>\n" "$status" "$value"'Repository: tinovyatkin/appsync-client-node
Length of output: 159
🏁 Script executed:
cat -n .github/workflows/release.yml | sed -n '45,60p'Repository: tinovyatkin/appsync-client-node
Length of output: 956
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Testing Line 49 pattern: conditional chain with || ==="
# Simulating: ([ ! -z "$TAG" ] && git ls-remote ...) || (echo "exists=false" >> ...)
# When git command fails, does it fall through to exists=false?
echo "Test 1: What happens when git ls-remote fails?"
bash -c 'set +e; out=$(mktemp); ([ -n "tag" ] && false) || echo "exists=false" >> "$out"; cat "$out"'
echo ""
echo "Test 2: Status code after failed git ls-remote with ||"
bash -c 'set +e; ([ -n "tag" ] && false) || true; echo "exit status: $?"'
echo ""
echo "=== Testing Line 54 pattern: command substitution without pipefail ==="
# Simulating: echo "latest_commit=$(git ls-remote ... | cut -f1)"
# Without pipefail, pipe failures are hidden
echo "Test 3: Pipe failure without pipefail"
bash -c 'set +e; value=$(false | cut -f1); echo "status=$?"; echo "value=<$value>"'
echo ""
echo "Test 4: With pipefail enabled"
bash -c 'set -o pipefail; set +e; value=$(false | cut -f1) 2>&1; echo "status=$?"; echo "value=<$value>"'Repository: tinovyatkin/appsync-client-node
Length of output: 441
Fail the workflow when remote checks break instead of silently defaulting to false.
Line 49's conditional chain masks git ls-remote failures as exists=false, conflating "tag does not exist" with "command failed"; a transient auth or network error will silently proceed with retry logic rather than alerting you to the failure. Line 54 hides pipe failures entirely—without pipefail, a failed git ls-remote yields an empty latest_commit without error.
Harden both steps: use set -euo pipefail, explicitly check for empty inputs, and allow real lookup failures to fail the job instead of masking them in conditional chains or pipes.
Suggested hardening
- name: Check if version has already been tagged
id: check_tag_exists
run: |-
+ set -euo pipefail
- TAG=$(cat dist/releasetag.txt)
- ([ ! -z "$TAG" ] && git ls-remote -q --exit-code --tags origin $TAG && (echo "exists=true" >> $GITHUB_OUTPUT)) || (echo "exists=false" >> $GITHUB_OUTPUT)
- cat $GITHUB_OUTPUT
+ TAG=$(<dist/releasetag.txt)
+ if [ -z "$TAG" ]; then
+ echo "::error::dist/releasetag.txt is empty"
+ exit 1
+ fi
+ tag_refs="$(git ls-remote --tags origin "refs/tags/$TAG")"
+ if [ -n "$tag_refs" ]; then
+ echo "exists=true" >> "$GITHUB_OUTPUT"
+ else
+ echo "exists=false" >> "$GITHUB_OUTPUT"
+ fi
- name: Check for new commits
id: git_remote
run: |-
+ set -euo pipefail
- echo "latest_commit=$(git ls-remote origin -h ${{ github.ref }} | cut -f1)" >> $GITHUB_OUTPUT
- cat $GITHUB_OUTPUT
+ latest_commit="$(git ls-remote origin -h "${{ github.ref }}" | cut -f1)"
+ echo "latest_commit=$latest_commit" >> "$GITHUB_OUTPUT"
shell: bash🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml around lines 45 - 50, The step
check_tag_exists masks real failures by swallowing git ls-remote errors and
treating them as exists=false; instead enable strict shell mode (set -euo
pipefail) at the top of the job/step, explicitly validate TAG from
dist/releasetag.txt is non-empty, run git ls-remote --tags origin "$TAG" and let
its non-zero exit code fail the step (don’t wrap it into a conditional that
converts failures to exists=false), and only set exists=true when git ls-remote
succeeds; also avoid unprotected pipes (use temporary variables or check command
exit statuses) so a failed git or pipe will fail the workflow rather than
silently defaulting.
| - name: Apply patch | ||
| run: '[ -s ${{ runner.temp }}/.repo.patch ] && git apply ${{ runner.temp }}/.repo.patch || echo "Empty patch. Skipping."' | ||
| run: '[ -s ${{ runner.temp }}/repo.patch ] && git apply ${{ runner.temp }}/repo.patch || echo "Empty patch. Skipping."' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "# Current workflow lines"
nl -ba .github/workflows/upgrade-main.yml | sed -n '61,62p'
patch_file="$(mktemp)"
printf 'x\n' > "$patch_file"
echo
echo "# Reproducing the current shell pattern: the fallback also runs when the apply command fails"
bash -lc 'set +e; patch="$1"; [ -s "$patch" ] && false || echo "Empty patch. Skipping."' _ "$patch_file"Repository: tinovyatkin/appsync-client-node
Length of output: 159
🏁 Script executed:
#!/bin/bash
# Read the workflow file to confirm lines 61-62
echo "=== Lines 61-62 of upgrade-main.yml ==="
sed -n '61,62p' .github/workflows/upgrade-main.yml
# Test the problematic shell pattern
echo ""
echo "=== Testing the problematic pattern ==="
echo "Test 1: Empty patch file (test should succeed)"
patch_file="$(mktemp)"
# Empty file
bash -c 'set +e; patch="$1"; [ -s "$patch" ] && false || echo "Empty patch. Skipping."' _ "$patch_file"
echo ""
echo "Test 2: Non-empty patch, but git apply fails (simulated)"
patch_file="$(mktemp)"
echo "dummy" > "$patch_file"
bash -c 'set +e; patch="$1"; [ -s "$patch" ] && false || echo "Empty patch. Skipping."' _ "$patch_file"
echo "Exit code from bash: $?"Repository: tinovyatkin/appsync-client-node
Length of output: 483
Don't turn git apply failures into a fake "empty patch".
Line 62's && ... || ... chain masks git apply failures as empty patches. When git apply fails with a corrupt or stale patch, the workflow silently outputs "Empty patch. Skipping." instead of failing, hiding the real error and dropping the upgrade run.
Suggested fix
- name: Apply patch
- run: '[ -s ${{ runner.temp }}/repo.patch ] && git apply ${{ runner.temp }}/repo.patch || echo "Empty patch. Skipping."'
+ run: |-
+ if [ -s "${{ runner.temp }}/repo.patch" ]; then
+ git apply "${{ runner.temp }}/repo.patch"
+ else
+ echo "Empty patch. Skipping."
+ fi📝 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.
| - name: Apply patch | |
| run: '[ -s ${{ runner.temp }}/.repo.patch ] && git apply ${{ runner.temp }}/.repo.patch || echo "Empty patch. Skipping."' | |
| run: '[ -s ${{ runner.temp }}/repo.patch ] && git apply ${{ runner.temp }}/repo.patch || echo "Empty patch. Skipping."' | |
| - name: Apply patch | |
| run: |- | |
| if [ -s "${{ runner.temp }}/repo.patch" ]; then | |
| git apply "${{ runner.temp }}/repo.patch" | |
| else | |
| echo "Empty patch. Skipping." | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/upgrade-main.yml around lines 61 - 62, The current "Apply
patch" step uses '[ -s ${{ runner.temp }}/repo.patch ] && git apply ${{
runner.temp }}/repo.patch || echo "Empty patch. Skipping."' which masks real git
apply errors as "Empty patch". Replace that chained command so that you first
test whether runner.temp/repo.patch is non-empty and only then run git apply,
but do not attach a fallback echo to the same conditional that would swallow git
apply failures; let git apply exit non‑zero and fail the job if it errors, and
only print "Empty patch. Skipping." when the patch file is truly empty. Ensure
the step references git apply and the runner.temp/repo.patch path so the new
conditional logic replaces the current combined &&/|| chain.
| "ts-jest": "^29", | ||
| "ts-node": "^10.9.2", | ||
| "typescript": "~5.2.2" | ||
| "typescript": "~6.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest stable version of TypeScript as of April 2026?
💡 Result:
As of April 2026, the latest stable TypeScript release is TypeScript 6.0. [1][2]
Source: The official TypeScript download page notes that npm install -g typescript installs the latest version, “currently 6.0.” [1]
Release announcement: Microsoft’s TypeScript blog also announces the availability of TypeScript 6.0. [2]
References:
[1] https://www.typescriptlang.org/download/
[2] https://devblogs.microsoft.com/typescript/announcing-typescript-6-0/
🌐 Web query:
Is Jest 30 released and available as of April 2026?
💡 Result:
Yes. Jest 30 has been released and is available on npm as of April 2026. The Jest project’s release notes show v30.0.0 released on June 10, 2025, and the official docs list 30.0 as the current stable major version. [1] [2]
Sources: [1] Jest GitHub releases (v30.0.0) [2] Jest “Versions” page (current stable = 30.0)
🏁 Script executed:
# Check the Jest configuration in package.json to see if ts-jest is used
cd /dev/null || true # safety
find . -name "package.json" -type f | head -5Repository: tinovyatkin/appsync-client-node
Length of output: 156
🏁 Script executed:
# Read the Jest configuration section from package.json to check transform usage
cat -n package.json | grep -A 30 '"jest"'Repository: tinovyatkin/appsync-client-node
Length of output: 1958
🏁 Script executed:
# Read the complete transform configuration from Jest config
cat -n package.json | sed -n '125,145p'Repository: tinovyatkin/appsync-client-node
Length of output: 598
Remove unused ts-jest dependency and simplify testMatch patterns.
TypeScript ~6.0 and Jest ^30 are both released and stable as of April 2026. However, ts-jest (line 54) is no longer used — the Jest transform at line 126 is configured to use @swc/jest exclusively. Remove this dependency.
Additionally, the testMatch patterns (lines 97–101) contain a redundant alternation: @(src|src) should be simplified to just src in both patterns.
- "ts-jest": "^29",- "<rootDir>/@(src|src)/**/*(*.)@(spec|test).ts?(x)",
- "<rootDir>/@(src|src)/**/__tests__/**/*.ts?(x)",
+ "<rootDir>/src/**/*(*.)@(spec|test).ts?(x)",
+ "<rootDir>/src/**/__tests__/**/*.ts?(x)",📝 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.
| "ts-jest": "^29", | |
| "ts-node": "^10.9.2", | |
| "typescript": "~5.2.2" | |
| "typescript": "~6.0" | |
| "ts-node": "^10.9.2", | |
| "typescript": "~6.0" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 54 - 56, Remove the unused "ts-jest" dependency
from package.json and update the Jest testMatch patterns to remove the redundant
alternation: delete the "ts-jest" entry (since the Jest transform uses
"@swc/jest") and replace occurrences of the pattern using "@(src|src)" with just
"src" in the "testMatch" array so the patterns read against "src" only; this
ensures the package.json dependencies and Jest configuration (transform using
"@swc/jest" and testMatch entries) are consistent and simplified.
BREAKING CHANGE: Minimum Node.js version raised from 16 to 20 LTS. - Migrate from deprecated @aws-sdk/hash-node, @aws-sdk/protocol-http, @aws-sdk/signature-v4, @aws-sdk/types to @smithy/* equivalents - Upgrade TypeScript 5.2 → 6.0, target/lib es2022 → es2024 - Upgrade moduleResolution from "node" (deprecated in TS6) to "bundler" - Upgrade Jest 29 → 30, projen 0.79 → 0.99, ESLint 8 → 9, @typescript-eslint 6 → 8, prettier-plugin-organize-imports 3 → 4 - Fix previously-skipped TimeoutError test by isolating socket timeout from AbortSignal race condition - Fix IAM auth test for new credential-provider-node behavior that prefers AWS_PROFILE over explicit env vars - Replace deprecated Jest matcher toBeCalledTimes → toHaveBeenCalledTimes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tinovyatkin
force-pushed
the
chore/major-version-bump-node20
branch
from
d587143 to
68b265d
Compare
1 participant
Summary
Major version bump (v1 → v2) with comprehensive dependency modernization:
moduleResolution: "bundler"(TS6 deprecates"node") andtarget/lib→ ES2024@aws-sdk/hash-node,@aws-sdk/protocol-http,@aws-sdk/signature-v4,@aws-sdk/typesto@smithy/*equivalentstoBeCalledTimes→toHaveBeenCalledTimes)@typescript-eslint/*6 → 8Housekeeping fixes
AbortSignal.timeout()and socket timeout — both fire attimeoutMs, butAbortSignalwins and throwsAbortErrorinstead ofTimeoutError. Fixed by passing a non-aborting signal in the test to isolate the socket timeout path.@aws-sdk/credential-provider-nodeprefersAWS_PROFILEover explicitAWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEYenv vars, causing the simulator to see the wrong identity. Fixed by clearingAWS_PROFILEin test setup.Not upgraded (intentionally)
graphqlstays at ^15 —@aws-amplify/amplify-appsync-simulatordepends ongraphql@^15.5.0eslintstays at ^9 — ESLint 10 requires flat config, which projen doesn't generate yet@types/nodestays at ^20 — matchesminNodeVersionto prevent using APIs unavailable in Node 20Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
CI/CD
Quality
Tests