Skip to content

chore(deps): update all dependencies, including majors - #534

Open
kibertoad wants to merge 9 commits into
mainfrom
claude/update-dependencies-kc4bdx
Open

chore(deps): update all dependencies, including majors#534
kibertoad wants to merge 9 commits into
mainfrom
claude/update-dependencies-kc4bdx

Conversation

@kibertoad

@kibertoad kibertoad commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Bumps every dependency across the workspace, the sns-sqs example and the kafka load-tests project to its latest version.

Major upgrades

Package From To Scope
typescript 5.9 7.0 all packages
@lokalise/tsconfig 3 (1 in example) 5 all packages
@types/node 25 26 all packages
pnpm (packageManager) 11.1.2 11.18.0 root
@platformatic/kafka 1.31 2.9 kafka/load-tests
@platformatic/dynamic-buffer 0.3 0.4 kafka/load-tests
pino 9 10 example
zod 3 4 example
@message-queue-toolkit/core / schemas / sns / sqs 21 / 6 / 22 / 21 26 / 7 / 26 / 26 example

Minor/patch bumps across the board: @aws-sdk/client-* → 3.1101.0, @lokalise/node-core → 14.8.1, @biomejs/biome → 2.5.6, @lokalise/biome-config → 3.1.1, zod → 4.4.3, rimraf → 6.1.3, turbo → 2.10.8, redis-semaphore → 5.7.0, uuidv7 → 1.2.1, @supercharge/promise-pool → 3.3.0, pg → 8.22.0, @types/pg → 8.20.3.

Every dependency is at the newest version that clears the 7-day cooldown in .github/dependabot.yml and the minimum release age pnpm enforces. Re-checked against the registry on 2026-08-08. The one deliberate exception is @platformatic/kafka — see below.

@platformatic/kafka 2.9.0, and the Node 24 failures

packages/kafka now depends on 2.9.0 (published 2026-08-08), up from the 2.8.0 on main.

The general (24.x, @message-queue-toolkit/kafka) job was red earlier on this branch, failing at a different assertion each run in PermissionConsumer.reconnect.spec.ts while passing on Node 22. Two defects were involved, one upstream and one ours.

Upstream, fixed by 2.9.0. On 2.8.0 the consumer threw out of callback chains nothing could catch:

  • ConnectionPool.#get reported its closed state with a plain Error('Connection pool is closed.'). 2.9.0 raises a NetworkError with canRetry: false instead, with the accompanying comment noting it "has to be a library error: clients classify errors via findBy, which only exists on GenericError subclasses."
  • Consumer.#handleError called findBy as a method on whatever error it was handed, producing TypeError: kafkaError.findBy is not a function. 2.9.0 classifies via the free findErrorBy, which tolerates foreign errors.

vitest fails a run on unhandled errors regardless of assertions, which is what made the failing assertion vary from run to run.

Ours, fixed in AbstractKafkaConsumer. close() closes the consumer stream, which makes the for await loop in handleStream reject with Premature close. That rejection was wired straight into reconnect(), so an ordinary shutdown — including the one the reconnect loop itself performs before re-initialising — started a fresh reconnect cycle against a client on its way out, and that cycle called init() while the previous consumer was still closing. This is what reached the upstream defects in the first place.

Reconnect now runs only for the stream that is still the active one and only outside a deliberate close, and close() waits for an in-flight init() instead of tearing down half-built state underneath it. init() also serialises concurrent callers: doInit() assigns this.consumer before awaiting joinGroup() and consume(), so the previous early return could hand a second caller a resolved init() with no consumer stream yet.

Both fixes are kept. Driving a client after close was a real bug on our side, and a shutdown starting a pointless reconnect cycle is wrong regardless of how the library reports the resulting error.

Release age. 2.9.0 is younger than the minimum release age pnpm enforces, so it carries a recorded minimumReleaseAgeExclude entry in pnpm-workspace.yaml with a comment explaining why. Every other dependency respects the age limit.

Adjustments the upgrades required

@lokalise/tsconfig v5. Two breaking changes in its shared configs:

  • build-app.json now sets "rootDir": "${configDir}/src". Sources live in lib/ here, so every tsconfig.build.json pins "rootDir": "lib". Without this the build fails with TS6059: File '.../lib/index.ts' is not under 'rootDir'.
  • base.json no longer includes node in the default types, so every package tsconfig.json now asks for it explicitly (["node", "vitest/globals"]).

Lockfile dedupe. Bumping the AWS SDK dev dependencies left two @aws-sdk/client-s3 (3.1048.0 / 3.1101.0) and two @smithy/types (4.14.2 / 4.16.1) copies installed, which made the SNS/SQS payload-store test types mutually unassignable. pnpm dedupe collapses them to one version each.

Formatting. biome 2.5.6 reformats the it.each(...) blocks in three spec files. No behavior change.

Example (examples/sns-sqs). With the toolkit packages on zod 4, the example no longer compiles against the zod/v3 compatibility entrypoint — switched to zod v4 imports. Also stopped annotating the shared AWS client config as SNSClientConfig, since the SNS/SQS/STS config types are no longer mutually assignable and it is passed to all three clients.

Review follow-ups. packages/amqp declared engines.node: ">=18" while the sibling adapters declare >=22.0.0 and CI only exercises 22/24 — aligned to >=22.0.0. The SQS 2-consumer heartbeat test waited on consumer2Counter > 0 unconditionally, then asserted the counter is 0 when heartbeat is enabled; guarded it the way the SNS counterpart already is.

Consumer lifecycle test coverage. The new guards added branches nothing exercised, and suppressing the spurious reconnect removed the only thing that reached the isReconnecting re-entry guard, dropping branch coverage to 84.16% against the 85% gate. Four tests now cover the guards directly — a stream failing once it is no longer active, a stream failing during a deliberate close, concurrent init() calls, and close() landing mid-init(). Branches are back to 86.66%.

Held back

ioredis stays at ^5. redis-semaphore — a runtime dependency of @message-queue-toolkit/redis-message-deduplication-store — declares "ioredis": "^4.1.0 || ^5" and has no v6-compatible release yet, so moving the dev dependencies in sns/sqs/gcp-pubsub to v6 would install a version its own peer range rejects. Worth revisiting once redis-semaphore ships ioredis 6 support.

@aws-sdk/core stays at the 3.977.4 the lockfile resolves. It carries a deprecation notice, but 3.977.5/3.977.6 are both inside the age-limit window.

Verification

Run locally against real containers (kafka, rabbitmq, redis, the GCS and Pub/Sub emulators) and fauxqs, on both Node 22 and Node 24:

  • pnpm run lint (biome + tsc over lib and tests) and pnpm run build pass for all 12 packages; pnpm install --frozen-lockfile is clean.
  • Full suites green: kafka (84), core (221), sqs (212), sns (128 + 2 skipped), gcp-pubsub (98), amqp (63), metrics (29), schemas (22), gcs-payload-store (11), s3-payload-store (10), redis-message-deduplication-store (9), outbox-core (5).
  • Against 2.9.0 specifically: full kafka suite green on both Node versions with no unhandled errors, coverage thresholds met, and kafka/load-tests typechecks clean.
  • CI was fully green (32/32) on the preceding commit, including general (24.x, @message-queue-toolkit/kafka).

Also merged main into the branch to clear the conflicts in package.json, packages/sns/package.json, packages/sqs/package.json and pnpm-lock.yaml (all of them this branch's higher versions against the dependabot bumps that landed since).

Summary by CodeRabbit

  • Bug Fixes

    • Improved Kafka consumer reliability during initialization, shutdown, and reconnection.
    • Prevented unnecessary reconnects caused by stale or intentional stream failures.
    • Ensured closing waits for startup to complete safely.
    • Refined SNS and SQS visibility-timeout test coverage.
  • Chores

    • Updated supported runtime tooling and package dependencies across messaging, storage, and schema packages.
    • Node.js 22 or newer is now required for the AMQP package.
    • Improved build configuration and TypeScript compatibility across packages.

Bumps every dependency across the workspace, the sns-sqs example and the
kafka load-tests project to its latest version.

Major upgrades:
- typescript 5.9 -> 7.0
- @lokalise/tsconfig 3 -> 5 (1 -> 5 in the example)
- @types/node 25 -> 26
- pnpm 11.1.2 -> 11.18.0 (packageManager)
- @platformatic/kafka 1.31 -> 2.8 and @platformatic/dynamic-buffer 0.3 -> 0.4
  (load-tests)
- example only: pino 9 -> 10, zod 3 -> 4, @message-queue-toolkit/core 21 -> 26,
  /schemas 6 -> 7, /sns 22 -> 26, /sqs 21 -> 26

Adjustments required by the upgrades:
- @lokalise/tsconfig v5 sets rootDir to `src` in build-app and drops `node`
  from the default `types`. Sources live in `lib` here, so each
  tsconfig.build.json now pins `rootDir: lib` and each tsconfig.json asks for
  the `node` types explicitly.
- Deduped the lockfile so a single @aws-sdk/client-s3 / @smithy/types version
  is installed; the mixed versions made the SNS/SQS payload-store test types
  mutually unassignable.
- Reformatted three spec files per biome 2.5.6.
- Example: switched from the `zod/v3` compatibility entrypoint to zod v4, and
  stopped typing the shared AWS client config as SNSClientConfig since the
  SNS/SQS/STS config types are no longer mutually assignable.

ioredis is deliberately held at ^5: redis-semaphore, a runtime dependency of
@message-queue-toolkit/redis-message-deduplication-store, declares an
`ioredis: ^4.1.0 || ^5` peer range and has no v6-compatible release yet.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LNF5yvgsnmw6fiGZsyB3HB
@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@kibertoad, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 29 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6bf85f8c-fde2-47b5-8901-4d6a85f8bf9a

📥 Commits

Reviewing files that changed from the base of the PR and between 2ecdb3f and c7c3fcc.

📒 Files selected for processing (2)
  • packages/kafka/lib/AbstractKafkaConsumer.ts
  • packages/kafka/test/consumer/PermissionConsumer.reconnect.spec.ts
📝 Walkthrough

Walkthrough

The change refreshes workspace dependencies and TypeScript settings. It updates SNS-SQS example imports and configuration typing. It reformats selected tests and changes Kafka consumer initialization, stream reconnection, and shutdown handling.

Changes

Workspace alignment

Layer / File(s) Summary
Kafka consumer lifecycle
packages/kafka/lib/AbstractKafkaConsumer.ts, packages/kafka/test/consumer/PermissionConsumer.reconnect.spec.ts
Tracks initialization and closing state. Restricts reconnection to the active stream and prevents reconnection during shutdown. Adds coverage for concurrent initialization and closing during initialization.
Dependency version refresh
package.json, pnpm-workspace.yaml, examples/sns-sqs/package.json, packages/*/package.json
Updates workspace, runtime, peer, and development dependency versions. Raises the AMQP minimum Node.js version to 22. Adds exact release-age exclusions.
TypeScript build configuration
examples/sns-sqs/tsconfig.json, packages/*/tsconfig*.json
Adds Node.js types and sets package build rootDir values to lib.
Example compatibility and test updates
examples/sns-sqs/lib/**/*, packages/kafka/test/publisher/*, packages/sns/test/consumers/*, packages/sqs/test/consumers/*
Updates TypeScript and Zod import paths, shared AWS configuration typing, parameterized test formatting, and SQS heartbeat polling behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested reviewers: carlosgamero, kjamrog

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the broad dependency updates, including major version upgrades, which are the primary changes in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/update-dependencies-kc4bdx

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@package.json`:
- Line 19: Update the `engines.node` declaration in `packages/amqp/package.json`
to `>=22.0.0` so it matches the pnpm 11 requirement and CI-supported runtime
range.

In `@packages/sqs/test/consumers/SqsPermissionConsumer.spec.ts`:
- Around line 961-965: Update the waitAndRetry condition in the heartbeat
consumer test so it always waits for consumer1Counter > 0, but only requires
consumer2Counter > 0 when heartbeatEnabled is false. Preserve the existing
expectations for both counters.
🪄 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 Plus

Run ID: efae0e6c-3791-4e4a-9c6f-63dcda7185f1

📥 Commits

Reviewing files that changed from the base of the PR and between db1451e and 23cb12c.

⛔ Files ignored due to path filters (2)
  • packages/kafka/load-tests/package-lock.json is excluded by !**/package-lock.json
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (51)
  • examples/sns-sqs/lib/01-publish-message.spec.ts
  • examples/sns-sqs/lib/02-configurable-init.spec.ts
  • examples/sns-sqs/lib/common/Dependencies.ts
  • examples/sns-sqs/lib/common/TestMessages.ts
  • examples/sns-sqs/lib/common/TestPublisherManager.ts
  • examples/sns-sqs/lib/common/UserConsumer.ts
  • examples/sns-sqs/lib/common/handlers/UserCreatedHandler.ts
  • examples/sns-sqs/lib/common/handlers/UserUpdatedHandler.ts
  • examples/sns-sqs/package.json
  • examples/sns-sqs/tsconfig.json
  • package.json
  • packages/amqp/package.json
  • packages/amqp/tsconfig.build.json
  • packages/amqp/tsconfig.json
  • packages/core/package.json
  • packages/core/tsconfig.build.json
  • packages/core/tsconfig.json
  • packages/gcp-pubsub/package.json
  • packages/gcp-pubsub/tsconfig.build.json
  • packages/gcp-pubsub/tsconfig.json
  • packages/gcs-payload-store/package.json
  • packages/gcs-payload-store/tsconfig.build.json
  • packages/gcs-payload-store/tsconfig.json
  • packages/kafka/load-tests/package.json
  • packages/kafka/package.json
  • packages/kafka/test/publisher/PermissionPublisher.spec.ts
  • packages/kafka/tsconfig.build.json
  • packages/kafka/tsconfig.json
  • packages/metrics/package.json
  • packages/metrics/tsconfig.build.json
  • packages/metrics/tsconfig.json
  • packages/outbox-core/package.json
  • packages/outbox-core/tsconfig.build.json
  • packages/outbox-core/tsconfig.json
  • packages/redis-message-deduplication-store/package.json
  • packages/redis-message-deduplication-store/tsconfig.build.json
  • packages/redis-message-deduplication-store/tsconfig.json
  • packages/s3-payload-store/package.json
  • packages/s3-payload-store/tsconfig.build.json
  • packages/s3-payload-store/tsconfig.json
  • packages/schemas/package.json
  • packages/schemas/tsconfig.build.json
  • packages/schemas/tsconfig.json
  • packages/sns/package.json
  • packages/sns/test/consumers/SnsSqsPermissionConsumer.spec.ts
  • packages/sns/tsconfig.build.json
  • packages/sns/tsconfig.json
  • packages/sqs/package.json
  • packages/sqs/test/consumers/SqsPermissionConsumer.spec.ts
  • packages/sqs/tsconfig.build.json
  • packages/sqs/tsconfig.json

Comment thread package.json
Comment thread packages/sqs/test/consumers/SqsPermissionConsumer.spec.ts Outdated
- packages/amqp declared `engines.node: ">=18"` while the sibling adapters
  declare `>=22.0.0`, CI only exercises Node 22/24, and the repo now builds
  against @types/node 26. Aligned it to `>=22.0.0`.
- The SQS 2-consumer heartbeat test waited on `consumer2Counter > 0`
  unconditionally, even in the heartbeat-enabled case where it then asserts the
  counter is 0. That made the wait burn its full retry budget and gave no real
  synchronization. Guarded it the same way the SNS counterpart already is.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LNF5yvgsnmw6fiGZsyB3HB
@kibertoad kibertoad added the major label Aug 2, 2026
claude added 2 commits August 8, 2026 10:49
…ncies-kc4bdx

# Conflicts:
#	package.json
#	packages/sns/package.json
#	packages/sqs/package.json
#	pnpm-lock.yaml
`close()` closes the consumer stream, which makes the `for await` loop in
`handleStream` reject with `Premature close`. That rejection was wired
straight into `reconnect()`, so an ordinary shutdown - including the one the
reconnect loop itself performs before re-initialising - started a fresh
reconnect cycle against a client that was on its way out.

The spurious cycle then called `init()` while the previous consumer was still
closing, and both failure modes seen in CI follow from that:

- `consume()` had already handed back a `MessagesStream` whose `_construct`
  refreshes offsets. Running that against a closed client throws
  `Error: Client is closed.` with no owner, so vitest records it as an
  uncaught exception.
- the connection pool reports its closed state with a plain `Error`
  ("Connection pool is closed."), while `Consumer.#handleError` assumes every
  error it sees is one of its own error classes and calls `findBy` on it. That
  throws `TypeError: kafkaError.findBy is not a function` out of a callback
  chain nothing can catch.

Both surfaced only under the timing of Node 24, which is why the same commit
passed on 22 and failed on 24, with a different assertion each run.

Reconnect now runs only for the stream that is still the active one and only
outside of a deliberate close, and `close()` waits for an in-flight `init()`
instead of tearing down half-built state underneath it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbY7k4gHTXhbMjiBZy4W6S

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/kafka/lib/AbstractKafkaConsumer.ts`:
- Around line 164-171: Update init() to check this.initPromise before checking
this.consumer, awaiting and reusing the existing initialization promise for
concurrent callers. Ensure all callers receive completion only after doInit()
finishes joinGroup() and consume(), while preserving the existing cleanup that
clears this.initPromise after initialization settles.
🪄 Autofix

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 Plus

Run ID: f620130f-fc38-4a05-8fac-04bb9399ddca

📥 Commits

Reviewing files that changed from the base of the PR and between 23cb12c and ad926c6.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (3)
  • packages/amqp/package.json
  • packages/kafka/lib/AbstractKafkaConsumer.ts
  • packages/sqs/test/consumers/SqsPermissionConsumer.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/amqp/package.json

Comment thread packages/kafka/lib/AbstractKafkaConsumer.ts
claude added 3 commits August 8, 2026 10:54
`doInit()` assigns `this.consumer` before it has awaited `joinGroup()` and
`consume()`, so the early return on `this.consumer` handed a concurrent caller a
resolved `init()` while `consumerStream` was still undefined. Check the in-flight
promise first and let every caller await the same one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbY7k4gHTXhbMjiBZy4W6S
The new guards in `init()`, `close()` and the reconnect trigger added branches
nothing exercised, and suppressing the spurious reconnect also took away the
only thing that reached the `isReconnecting` re-entry guard. Branch coverage
landed at 84.16% against the 85% threshold.

Covers what the guards are actually for: a stream failing once it is no longer
the active one, a stream failing during a deliberate close, concurrent `init()`
calls sharing one initialization, and `close()` landing while `init()` is still
in flight. Branches back to 86.66%.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbY7k4gHTXhbMjiBZy4W6S
2.9.0 fixes both upstream defects behind the Node 24 consumer failures, which
2.8.0 could only be worked around from our side:

- `ConnectionPool.#get` reported its closed state with a plain
  `Error('Connection pool is closed.')`. It now raises a `NetworkError` carrying
  `canRetry: false`, so clients can classify it.
- `Consumer.#handleError` called `findBy` as a method on whatever error it was
  handed, which threw `TypeError: kafkaError.findBy is not a function` out of a
  callback chain nothing could catch. It now classifies via the free
  `findErrorBy`, which tolerates foreign errors.

The consumer-side reconnect fix stays: driving a client after close was our bug,
and a shutdown starting a pointless reconnect cycle is wrong regardless of how
the library reports the resulting error.

2.9.0 was published today, so it is inside the minimum release age pnpm enforces
and needs the recorded `minimumReleaseAgeExclude` carve-out. Every other
dependency still respects the age limit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbY7k4gHTXhbMjiBZy4W6S
Moves `@aws-sdk/client-s3`, `client-sns`, `client-sqs` and `client-sts` from
3.1101.0 to 3.1106.0 across `packages/sns`, `packages/sqs` and the `sns-sqs`
example, and pulls the transitive `@aws-sdk/core` from the deprecated 3.977.4
up to 3.977.6.

`pnpm update` alone left two `@aws-sdk/core` copies installed; `pnpm dedupe`
collapses them, so the workspace resolves a single `@aws-sdk/core@3.977.6`,
`@aws-sdk/client-s3@3.1106.0` and `@smithy/types@4.16.1`. Mixed copies are what
previously made the SNS/SQS payload-store test types mutually unassignable.

3.1106.0 is inside the minimum release age, so each client carries its own
`minimumReleaseAgeExclude` entry pinned to that exact version rather than a bare
package name — a later release is gated again instead of inheriting the waiver.
The transitive `@aws-sdk/core` needs no entry of its own; `--frozen-lockfile`
is clean.

Peer dependency floors are left alone: raising them would force consumers onto
a newer SDK for no reason.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbY7k4gHTXhbMjiBZy4W6S

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/kafka/lib/AbstractKafkaConsumer.ts (1)

266-300: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Make a deliberate close cancel an in-progress reconnect.

During reconnect, reconnect() first calls close(), which clears consumer and resets isClosing. If application code then calls close() during the backoff delay, line 274 returns without recording shutdown intent. The reconnect loop then calls init() and recreates the consumer after the caller closed it.

Track persistent shutdown intent separately from reconnect teardown, or cancel the reconnect operation. Add a regression test that calls close() during reconnect backoff and verifies that no new consumer starts.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/kafka/lib/AbstractKafkaConsumer.ts` around lines 266 - 300, Update
AbstractKafkaConsumer.close() and the reconnect lifecycle so an
application-triggered close records persistent shutdown intent even when
consumer is absent during reconnect backoff, while internal reconnect teardown
does not. Make reconnect() check that intent before calling init() and avoid
recreating the consumer; add a regression test covering close() during backoff
and asserting no new consumer starts.
🧹 Nitpick comments (1)
packages/kafka/test/consumer/PermissionConsumer.reconnect.spec.ts (1)

121-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that every concurrent init() call stays pending.

Promise.all() waits for the first initialization. The assertions pass even if the second and third calls resolve before consumerStream exists.

Block joinGroup() or consume() with a deferred promise. Assert that each init() promise remains unsettled until setup completes. This will detect a regression that checks this.consumer before this.initPromise.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/kafka/test/consumer/PermissionConsumer.reconnect.spec.ts` around
lines 121 - 126, Update the concurrent init test around consumer.init() to defer
joinGroup() or consume() until setup is explicitly released, retain each
returned init promise, and assert all remain unsettled before releasing the
deferred operation. After setup completes, await all promises and preserve the
existing single-call, stream, and connection assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/kafka/lib/AbstractKafkaConsumer.ts`:
- Around line 165-175: Update the initialization error path around doInit() and
its finally block to clean up partial state when initialization rejects: close
and clear this.consumer, stream, and batch stream before rethrowing the original
error. Keep successful initialization behavior unchanged, and ensure initPromise
is still cleared so a later init() can retry safely.

---

Outside diff comments:
In `@packages/kafka/lib/AbstractKafkaConsumer.ts`:
- Around line 266-300: Update AbstractKafkaConsumer.close() and the reconnect
lifecycle so an application-triggered close records persistent shutdown intent
even when consumer is absent during reconnect backoff, while internal reconnect
teardown does not. Make reconnect() check that intent before calling init() and
avoid recreating the consumer; add a regression test covering close() during
backoff and asserting no new consumer starts.

---

Nitpick comments:
In `@packages/kafka/test/consumer/PermissionConsumer.reconnect.spec.ts`:
- Around line 121-126: Update the concurrent init test around consumer.init() to
defer joinGroup() or consume() until setup is explicitly released, retain each
returned init promise, and assert all remain unsettled before releasing the
deferred operation. After setup completes, await all promises and preserve the
existing single-call, stream, and connection assertions.
🪄 Autofix

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 Plus

Run ID: e5028e2c-bb11-4de8-a41b-1c6e4fad89fa

📥 Commits

Reviewing files that changed from the base of the PR and between ad926c6 and 2ecdb3f.

⛔ Files ignored due to path filters (2)
  • packages/kafka/load-tests/package-lock.json is excluded by !**/package-lock.json
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (8)
  • examples/sns-sqs/package.json
  • packages/kafka/lib/AbstractKafkaConsumer.ts
  • packages/kafka/load-tests/package.json
  • packages/kafka/package.json
  • packages/kafka/test/consumer/PermissionConsumer.reconnect.spec.ts
  • packages/sns/package.json
  • packages/sqs/package.json
  • pnpm-workspace.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
  • packages/sns/package.json
  • examples/sns-sqs/package.json
  • packages/kafka/load-tests/package.json
  • packages/kafka/package.json

Comment thread packages/kafka/lib/AbstractKafkaConsumer.ts
Both are pre-existing, and both were surfaced by review of the reconnect fix.

**A failed init left a half-built consumer behind.** `doInit()` creates the
consumer before `joinGroup()` and `consume()` can fail, and nothing dropped it
when they did. `init()`'s early return then handed the next caller a resolved
init() for a consumer that never got a stream — a silent dead consumer. The
references are now cleared in `doInit()`'s catch so a retry starts from scratch.
Closing the abandoned client is best effort and deliberately not awaited: one
that never joined its group can take seconds to close, and the two
`autocreateTopics: false` specs time out at 5s if that blocks the rejection.

**A close during reconnect backoff was ignored.** `close()` returned early when
there was no consumer to tear down, which is exactly the state the reconnect
loop leaves behind while it waits out its backoff. The loop then called `init()`
and handed back a running consumer the caller believed it had shut down.
Shutdown intent is now recorded before that early return, and the loop checks it
after the backoff.

Recording intent means separating the application's close from the loop's own
teardown, so `close()` is now intent plus `teardown()`, and the reconnect loop
calls `teardown()` directly. The two specs that asserted on the `close` spy
assert on `teardown` instead — they were always asserting the loop's internal
teardown, not a public close.

Also strengthens the concurrent-init test, which could not fail: it gates
`joinGroup` so initialization is held open at the point where `this.consumer`
exists but the stream does not, then asserts no caller settles in that window.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbY7k4gHTXhbMjiBZy4W6S
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants