Conversation
this would have kept the streams from completely running out of control
texuf
requested review from
bas-vk,
erikolsson,
salzbrenner and
sergekh2
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryAdds user inbox group-session de-duplication in node rules, returns newEvents from addEvent through RPC/SDK, and simplifies CI by unifying and fixing multi-node test runs.
Written by Cursor Bugbot for commit 6865f8a. This will update automatically on new commits. Configure here. |
📝 WalkthroughWalkthroughThis PR updates CI workflow job names and stream-metadata ordering, renames and removes various test scripts, adds group-encryption-session discovery and validation in the node, updates SDK client return types to include Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
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. 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/ci.yml (1)
98-107: Avoid YAML alias as astepselement (actionlint flags it as invalid). Inline the upload-artifact step in each job (or use a composite action). This likely breaks “tests actually run in CI” if actionlint is gating.- - *archive_river_node_logs_and_settings + - name: Archive River Node Logs and Settings + if: always() + uses: actions/upload-artifact@v4 + with: + name: 'river-node-${{ github.job }}' + path: | + ./core/run_files/ + !./core/**/bin/**Also applies to: 519-519
packages/sdk/src/client.ts (1)
2810-2856: GuardnewEventsagainstundefinedto keep tests/runtime stable.
Given the server can return a no-op with noNewEvents, ensure the SDK always returns an array.- return { prevMiniblockHash, eventId, newEvents: result.newEvents } + return { prevMiniblockHash, eventId, newEvents: result.newEvents ?? [] }(Then callers like
expect(result.newEvents).toHaveLength(0)won’t explode if the field is omitted.)Also applies to: 2858-2902
core/node/rules/can_add_event.go (1)
425-437: Add basic shape validation (non-empty, unique, bounded) before treating as no-op.
Right now malformed payloads (e.g., emptySessionIds, empty session IDs, emptyCiphertexts, duplicates) silently become(false, nil)no-ops, and the function comment about uniqueness isn’t enforced.Suggested minimal hardening:
func (ru *aeEncryptedGroupSessionRules) validEncryptedGroupSession() (bool, error) { if ru.session == nil { return false, RiverError(Err_INVALID_ARGUMENT, "event is not a group encryption session event") } + if len(ru.session.Ciphertexts) == 0 { + return false, RiverError(Err_INVALID_ARGUMENT, "ciphertexts must not be empty") + } + if len(ru.session.SessionIds) == 0 { + return false, RiverError(Err_INVALID_ARGUMENT, "sessionIds must not be empty") + } + seen := make(map[string]struct{}, len(ru.session.SessionIds)) + for _, sid := range ru.session.SessionIds { + if sid == "" { + return false, RiverError(Err_INVALID_ARGUMENT, "sessionId must not be empty") + } + if _, ok := seen[sid]; ok { + return false, RiverError(Err_INVALID_ARGUMENT, "duplicate sessionId") + } + seen[sid] = struct{}{} + }Optionally also cap sizes (e.g., max sessionIds / max ciphertext devices) to limit worst-case rule CPU.
Also applies to: 2366-2396
🧹 Nitpick comments (1)
core/node/events/stream_viewstate_userinbox.go (1)
24-61: Avoid lint noise + typo in callback params; consider_names.
minibockNum/eventNumare unused (andminibockNumis misspelled). Suggest_placeholders to keepgolangci-linthappy.- updateFn := func(e *ParsedEvent, minibockNum int64, eventNum int64) (bool, error) { + updateFn := func(e *ParsedEvent, _miniblockNum int64, _eventNum int64) (bool, error) {Also, if this is called on a hot path, consider a cached/indexed view-state instead of rescanning from 0 every time.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package.jsonis excluded by!package.json
📒 Files selected for processing (18)
.github/workflows/ci.yml(5 hunks)core/README.md(0 hunks)core/node/events/stream_viewstate_userinbox.go(1 hunks)core/node/rpc/add_event.go(1 hunks)core/node/rules/can_add_event.go(3 hunks)packages/encryption/package.json(1 hunks)packages/proto/package.json(1 hunks)packages/rpc-connector/package.json(1 hunks)packages/sdk/TESTING.md(1 hunks)packages/sdk/package.json(0 hunks)packages/sdk/src/client.ts(6 hunks)packages/sdk/src/tests/multi_ne/client.test.ts(3 hunks)packages/stream-metadata/package.json(1 hunks)packages/stress/package.json(1 hunks)packages/utils/package.json(1 hunks)packages/utils/src/tests/dlog.test.ts(1 hunks)packages/web3/package.json(1 hunks)turbo.json(0 hunks)
💤 Files with no reviewable changes (3)
- packages/sdk/package.json
- turbo.json
- core/README.md
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,yaml,yml,sol,json,md}
📄 CodeRabbit inference engine (AGENTS.md)
All non-Go files (TypeScript, JavaScript, YAML, Solidity) must pass Prettier formatting by running
bun run prettier:fix
Files:
packages/stream-metadata/package.jsonpackages/utils/package.jsonpackages/stress/package.jsonpackages/encryption/package.jsonpackages/proto/package.jsonpackages/rpc-connector/package.jsonpackages/sdk/TESTING.mdpackages/sdk/src/tests/multi_ne/client.test.tspackages/sdk/src/client.tspackages/utils/src/tests/dlog.test.tspackages/web3/package.json
core/node/rpc/**/*.go
📄 CodeRabbit inference engine (core/node/rpc/AGENTS.md)
core/node/rpc/**/*.go: Services run on separate listeners/ports from main River nodes and require separate service initialization with distinct authentication and configuration
Configure service-specific settings likeAppRegistryId,SharedSecretDataEncryptionKey, andAllowInsecureWebhooksin configuration before service initializationImplement gRPC service implementations in
node/rpc/directory using Connect protocol (connectrpc.com)
Files:
core/node/rpc/add_event.go
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Format all Go files using./fmt.shscript from the/coredirectory
Run the linter using./lint.shscript from the/coredirectory for Go code
Use structured logging with zap for Go backend code
Implement proper error handling with RiverError types in Go code
Implement proper context cancellation for graceful shutdowns in Go code
Files:
core/node/rpc/add_event.gocore/node/rules/can_add_event.gocore/node/events/stream_viewstate_userinbox.go
core/**/*.go
📄 CodeRabbit inference engine (core/AGENTS.md)
core/**/*.go: Format all modified Go files by running./fmt.shfrom the/coredirectory before committing
Run the linter via./lint.shfrom the/coredirectory to check code quality using golangci-lint and staticcheck, and fix any issues before committing
UseRiverErrortype fromnode/base/error.gofor structured errors
UseRiverErrorWithBaseto construct a river error derived from an error of arbitrary or unknown type
Wrap blockchain errors with context usingcrypto/utilities
Log errors with structured fields using zap logger
Use structured JSON logging via zap with custom extensions innode/logging/for log output
Use PostgreSQL connection pooling via pgx/v5 for database connections
Files:
core/node/rpc/add_event.gocore/node/rules/can_add_event.gocore/node/events/stream_viewstate_userinbox.go
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/**/*.{ts,tsx}: Use proper TypeScript types, especially for blockchain interactions in SDK code
Implement proper error handling and validation in TypeScript SDK code
Files:
packages/sdk/src/tests/multi_ne/client.test.tspackages/sdk/src/client.tspackages/utils/src/tests/dlog.test.ts
**/*{_test,test,spec}.{go,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*{_test,test,spec}.{go,ts,tsx,js,jsx}: Make tests re-runnable using unique identifiers (e.g., UUIDs) in shared test environments
Test both positive and negative cases explicitly in test files
Files:
packages/sdk/src/tests/multi_ne/client.test.tspackages/utils/src/tests/dlog.test.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: packages/contracts/AGENTS.md:0-0
Timestamp: 2025-12-06T08:15:28.770Z
Learning: Applies to packages/contracts/test/**/*.t.sol : Write comprehensive unit tests for all functionality with fuzz testing for numeric inputs using 4096 test runs minimum
📚 Learning: 2025-12-06T01:22:08.487Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T01:22:08.487Z
Learning: Applies to **/*.{ts,tsx,js,jsx,yaml,yml,sol,json,md} : All non-Go files (TypeScript, JavaScript, YAML, Solidity) must pass Prettier formatting by running `bun run prettier:fix`
Applied to files:
packages/stream-metadata/package.jsonpackages/utils/package.jsonpackages/stress/package.jsonpackages/encryption/package.jsonpackages/proto/package.jsonpackages/rpc-connector/package.jsonpackages/web3/package.json
📚 Learning: 2025-12-06T01:22:08.487Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T01:22:08.487Z
Learning: Applies to **/*{_test,test,spec}.{go,ts,tsx,js,jsx} : Make tests re-runnable using unique identifiers (e.g., UUIDs) in shared test environments
Applied to files:
packages/stream-metadata/package.jsonpackages/proto/package.jsonpackages/utils/src/tests/dlog.test.ts
📚 Learning: 2025-12-04T21:04:03.127Z
Learnt from: shuhuiluo
Repo: towns-protocol/towns PR: 4183
File: packages/utils/package.json:12-12
Timestamp: 2025-12-04T21:04:03.127Z
Learning: In Bun, when running scripts with `bun run <script> <args>`, additional arguments are automatically forwarded to the underlying command. For example, `bun run lint --fix` where `lint` is defined as `eslint --format unix ./src` will execute as `eslint --format unix ./src --fix`. This differs from npm/yarn where you typically need `--` to forward arguments.
Applied to files:
packages/utils/package.jsonpackages/encryption/package.jsonpackages/proto/package.jsonpackages/sdk/TESTING.mdpackages/web3/package.json
📚 Learning: 2025-12-06T01:22:08.487Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T01:22:08.487Z
Learning: All PRs must pass global linting by running `bun run lint` from the root directory
Applied to files:
packages/utils/package.jsonpackages/encryption/package.jsonpackages/proto/package.jsonpackages/sdk/TESTING.mdpackages/web3/package.json
📚 Learning: 2025-12-06T01:22:08.487Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T01:22:08.487Z
Learning: Applies to protocol/**/*.proto : Define protocol changes in Protocol Buffer files under `/protocol/` before implementing backend or frontend changes
Applied to files:
packages/proto/package.json
📚 Learning: 2025-12-06T08:15:28.770Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: packages/contracts/AGENTS.md:0-0
Timestamp: 2025-12-06T08:15:28.770Z
Learning: Applies to packages/contracts/test/**/*.t.sol : Use fork tests to test cross-chain functionality against actual deployed contracts
Applied to files:
packages/sdk/TESTING.md
📚 Learning: 2025-12-06T08:15:28.770Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: packages/contracts/AGENTS.md:0-0
Timestamp: 2025-12-06T08:15:28.770Z
Learning: Use Foundry framework for development, testing, and deployment with support for fuzz testing and fork tests
Applied to files:
packages/sdk/TESTING.md
📚 Learning: 2025-11-25T08:48:08.084Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: core/node/storage/AGENTS.md:0-0
Timestamp: 2025-11-25T08:48:08.084Z
Learning: Applies to core/node/storage/storage/**/*_test.go : Use helper functions to generate test data consistently: `safeAddress(t)` for Ethereum addresses, `testAppMetadataWithName(name string)` for app metadata, `testutils.FakeStreamId(streamType)` for stream IDs
Applied to files:
packages/sdk/src/tests/multi_ne/client.test.ts
📚 Learning: 2025-12-06T08:16:19.499Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: protocol/AGENTS.md:0-0
Timestamp: 2025-12-06T08:16:19.499Z
Learning: Applies to protocol/**/*.proto : Use `bytes` for Ethereum addresses (20 bytes) and stream IDs (32 bytes) in protobuf messages
Applied to files:
packages/sdk/src/tests/multi_ne/client.test.ts
📚 Learning: 2025-12-06T08:16:01.091Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: packages/subgraph/AGENTS.md:0-0
Timestamp: 2025-12-06T08:16:01.091Z
Learning: Applies to packages/subgraph/src/index.ts : Insert records into analyticsEvent table with txHash and logIndex as composite primary key, including eventType, blockTimestamp, and type-safe eventData JSON
Applied to files:
packages/sdk/src/client.ts
📚 Learning: 2025-12-08T20:20:53.994Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: packages/examples/bot-quickstart/AGENTS.md:0-0
Timestamp: 2025-12-08T20:20:53.994Z
Learning: Applies to packages/examples/bot-quickstart/**/*.{ts,tsx} : All event handler functions must use the base payload structure containing userId (hex address 0x...), spaceId, channelId, eventId, and createdAt.
Applied to files:
packages/sdk/src/client.ts
📚 Learning: 2025-12-06T08:15:09.346Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: core/AGENTS.md:0-0
Timestamp: 2025-12-06T08:15:09.346Z
Learning: Events must go through the pipeline: receive via gRPC API (node/rpc/) → check authorization via rule engine (node/rules/) → store in PostgreSQL (node/storage/) → create miniblocks and replicate (node/events/) → update stream views
Applied to files:
packages/sdk/src/client.ts
📚 Learning: 2025-11-25T08:45:13.269Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: packages/contracts/.cursor/rules/contracts.mdc:0-0
Timestamp: 2025-11-25T08:45:13.269Z
Learning: Applies to packages/contracts/**/*.sol : Use events for important state changes
Applied to files:
packages/sdk/src/client.ts
📚 Learning: 2025-12-06T01:22:08.487Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T01:22:08.487Z
Learning: Applies to **/*{_test,test,spec}.{go,ts,tsx,js,jsx} : Test both positive and negative cases explicitly in test files
Applied to files:
packages/utils/src/tests/dlog.test.ts
🧬 Code graph analysis (4)
core/node/rules/can_add_event.go (3)
core/node/protocol/protocol.pb.go (4)
UserInboxPayload_GroupEncryptionSessions(8492-8503)UserInboxPayload_GroupEncryptionSessions(8518-8518)UserInboxPayload_GroupEncryptionSessions(8533-8535)Err_INVALID_ARGUMENT(407-407)core/node/base/error.go (1)
RiverError(74-86)core/node/shared/stream_id.go (2)
StreamIdFromBytes(77-85)StreamId(41-41)
core/node/events/stream_viewstate_userinbox.go (2)
core/node/shared/stream_id.go (1)
StreamId(41-41)core/node/events/stream_view.go (1)
StreamView(238-244)
packages/sdk/src/tests/multi_ne/client.test.ts (3)
packages/sdk/src/tests/testUtils.ts (1)
makeUniqueSpaceStreamId(229-231)packages/sdk/src/types.ts (1)
make_UserInboxPayload_GroupEncryptionSessions(685-697)packages/sdk/src/id.ts (1)
streamIdToBytes(45-45)
packages/sdk/src/client.ts (2)
core/node/protocol/protocol.pb.go (3)
EventRef(3373-3381)EventRef(3396-3396)EventRef(3411-3413)packages/sdk/src/id.ts (1)
streamIdAsBytes(48-48)
🪛 actionlint (1.7.9)
.github/workflows/ci.yml
519-519: element of "steps" section is alias node but mapping node is expected
(syntax-check)
519-519: step must run script with "run" section or run action with "uses" section
(syntax-check)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Multinode Ent Tests (test:ci:multi:ent)
- GitHub Check: Multinode_Ent_Legacy
- GitHub Check: Multinode Tests (test:ci:multi:ne)
- GitHub Check: Common CI (test:unit)
- GitHub Check: Go_Tests
- GitHub Check: Cursor Bugbot
- GitHub Check: XChain_Integration
🔇 Additional comments (11)
packages/stress/package.json (1)
16-16: The filepackages/stress/package.jsondoes not exist in the repository. This review comment cannot be addressed as written.Likely an incorrect or invalid review comment.
packages/web3/package.json (1)
14-16:test:ci:multi:entdelegating totestlooks good (keeps env loading behavior).packages/sdk/TESTING.md (1)
108-111: Docs clarification is an improvement; commands remain unchanged.packages/encryption/package.json (1)
13-16:test:unitrename is consistent and keeps the same execution.packages/proto/package.json (1)
14-16:test:unitrename is consistent and low-risk.packages/utils/package.json (1)
13-16:test:unitrename matches CI + other packages.packages/stream-metadata/package.json (1)
20-22: Script rename totest:ci:multi:neis consistent with the workflow.packages/rpc-connector/package.json (1)
13-15: Addingtest:unitfor CI parity is fine..github/workflows/ci.yml (1)
635-637: Turbo is still orchestrating the task;bun run test:ci:multi:entinvokes the turbo task defined in turbo.json (which has no additional env vars). No environment setup is lost by this simplification—the turbo cache remains configured in the job, and the task definition only depends on the build task, with no special environment configuration. The change is purely in the invocation method (bun instead of direct turbo invocation).core/node/events/stream_viewstate_userinbox.go (1)
10-19: Nice: compile-time assertion keeps rule engine decoupled from concreteStreamView.
This interface +var _ ...assertion is a good seam for testing/mocking.core/node/rpc/add_event.go (1)
132-135: Comment matches behavior (idempotent no-op whencanAddEvent == false).
| - name: Run Stream Metadata Nodes | ||
| run: | | ||
| mkdir -p core/run_files/local_dev/stream-metadata # make a directory for logs so they get saved | ||
| bun run --filter @towns-protocol/stream-metadata dev:local_dev > core/run_files/local_dev/stream-metadata/dev.log 2>&1 & | ||
| bun run wait-on http://localhost:3002/health --timeout=120000 --i=5000 --verbose | ||
|
|
There was a problem hiding this comment.
Fix wait-on option typo (--i=5000) to a supported interval flag. This can hard-fail the Multinode job before tests run.
- bun run wait-on http://localhost:3002/health --timeout=120000 --i=5000 --verbose
+ bun run wait-on http://localhost:3002/health --timeout=120000 -i 5000 --verbose📝 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: Run Stream Metadata Nodes | |
| run: | | |
| mkdir -p core/run_files/local_dev/stream-metadata # make a directory for logs so they get saved | |
| bun run --filter @towns-protocol/stream-metadata dev:local_dev > core/run_files/local_dev/stream-metadata/dev.log 2>&1 & | |
| bun run wait-on http://localhost:3002/health --timeout=120000 --i=5000 --verbose | |
| - name: Run Stream Metadata Nodes | |
| run: | | |
| mkdir -p core/run_files/local_dev/stream-metadata # make a directory for logs so they get saved | |
| bun run --filter @towns-protocol/stream-metadata dev:local_dev > core/run_files/local_dev/stream-metadata/dev.log 2>&1 & | |
| bun run wait-on http://localhost:3002/health --timeout=120000 -i 5000 --verbose |
🤖 Prompt for AI Agents
.github/workflows/ci.yml around lines 503 to 508: the wait-on invocation uses an
unsupported shorthand flag `--i=5000` which can cause the job to fail; replace
that token with the supported interval flag `--interval=5000` (or `-i 5000`) so
the command becomes `bun run wait-on http://localhost:3002/health
--timeout=120000 --interval=5000 --verbose`, keeping the rest of the step
unchanged.
| test('bobCanSendGroupEncryptionSessions', async () => { | ||
| await expect(bobsClient.initializeUser()).resolves.not.toThrow() | ||
| bobsClient.startSync() | ||
| expect(bobsClient.userInboxStreamId).toBeDefined() | ||
|
|
||
| // send it to yourself, works for now... | ||
| const fakeStreamId = makeUniqueSpaceStreamId() | ||
| const deviceId = 'd1' | ||
| const payload = make_UserInboxPayload_GroupEncryptionSessions({ | ||
| streamId: streamIdToBytes(fakeStreamId), | ||
| senderKey: bobsClient.userId, | ||
| sessionIds: ['123'], | ||
| ciphertexts: { | ||
| [deviceId]: 'ciphertext', | ||
| }, | ||
| algorithm: GroupEncryptionAlgorithmId.GroupEncryption, | ||
| }) | ||
|
|
||
| const result1 = await bobsClient.makeEventAndAddToStream( | ||
| bobsClient.userInboxStreamId!, | ||
| payload, | ||
| ) | ||
| // we added one event | ||
| expect(result1.newEvents).toHaveLength(1) | ||
|
|
||
| // sending the same one again should work, but nothing gets added | ||
| const result2 = await bobsClient.makeEventAndAddToStream( | ||
| bobsClient.userInboxStreamId!, | ||
| payload, | ||
| ) | ||
| expect(result2.newEvents).toHaveLength(0) | ||
| // but with another id it should work | ||
| const payload2 = make_UserInboxPayload_GroupEncryptionSessions({ | ||
| streamId: streamIdToBytes(fakeStreamId), | ||
| senderKey: bobsClient.userId, | ||
| sessionIds: ['123', '456'], | ||
| ciphertexts: { | ||
| [deviceId]: 'ciphertext', | ||
| }, | ||
| algorithm: GroupEncryptionAlgorithmId.GroupEncryption, | ||
| }) | ||
|
|
||
| const result3 = await bobsClient.makeEventAndAddToStream( | ||
| bobsClient.userInboxStreamId!, | ||
| payload2, | ||
| ) | ||
| expect(result3.newEvents).toHaveLength(1) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find production send path and senderKey usage
rg -A 5 "senderKey:" packages/sdk/src/client.ts | head -30Repository: towns-protocol/towns
Length of output: 565
🏁 Script executed:
# Check if userDeviceKey method exists in client
rg "userDeviceKey" packages/sdk/src/ -A 3Repository: towns-protocol/towns
Length of output: 21510
🏁 Script executed:
# Examine streamIdToBytes implementation
rg "streamIdToBytes" packages/sdk/src/ -B 2 -A 8 | head -40Repository: towns-protocol/towns
Length of output: 4159
🏁 Script executed:
# Check the test file imports and setup
head -50 packages/sdk/src/tests/multi_ne/client.test.tsRepository: towns-protocol/towns
Length of output: 1376
🏁 Script executed:
# Check for GroupEncryptionSessions payload builder and its senderKey parameter
rg "make_UserInboxPayload_GroupEncryptionSessions" packages/sdk/src/ -B 3 -A 10Repository: towns-protocol/towns
Length of output: 7986
🏁 Script executed:
# Check if there's any server-side validation of senderKey in GroupEncryptionSessions
rg "groupEncryptionSessions" packages/ -A 10 -B 2 | grep -A 10 "senderKey" | head -40Repository: towns-protocol/towns
Length of output: 46
🏁 Script executed:
# Verify the UserDevice type structure to confirm deviceKey field
rg "interface UserDevice\|type UserDevice" packages/ -A 5Repository: towns-protocol/towns
Length of output: 46
Use bobsClient.userDeviceKey().deviceKey for senderKey instead of bobsClient.userId.
The production code sends GroupEncryptionSessions with senderKey: userDevice.deviceKey, so the test should match that pattern to avoid future validation changes breaking CI.
- senderKey: bobsClient.userId,
+ senderKey: bobsClient.userDeviceKey().deviceKey,Update both payload instances (lines 346 and 374).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/sdk/src/tests/multi_ne/client.test.ts around lines 335 to 382, the
test constructs GroupEncryptionSessions payloads using senderKey:
bobsClient.userId but production uses the device key; update both payload
instances (around lines 346 and 374) to set senderKey:
bobsClient.userDeviceKey().deviceKey so the test matches production behavior and
won’t break on validation changes.
| // aellis 12/2025 this test wasn't running, seems like this is bad though | ||
| // debug.enable(ns) | ||
| // log = dlog(ns) | ||
| // expect(log.enabled).toBeTruthy() | ||
|
|
||
| // Disabled explicitly by settings | ||
| debug.enable('-' + ns) | ||
| expect(log.enabled).toBeFalsy() | ||
| // debug.enable('-' + ns) | ||
| // expect(log.enabled).toBeFalsy() | ||
|
|
||
| // Disabled explicitly by settings, default ignored | ||
| log = dlog(ns, { defaultEnabled: true, allowJest: true }) | ||
| expect(log.enabled).toBeFalsy() | ||
| // log = dlog(ns, { defaultEnabled: true, allowJest: true }) | ||
| // expect(log.enabled).toBeFalsy() |
There was a problem hiding this comment.
Commenting out tests reduces coverage without explaining the root cause.
The comment acknowledges this is problematic, but the tests remain disabled. This contradicts the PR objective to "sort out tests" and violates the guideline to test both positive and negative cases explicitly.
Either:
- Fix the underlying issue that prevented these tests from running, or
- Add a TODO/tracking issue explaining why they're disabled and when they'll be restored, or
- Remove them entirely if they're no longer valid
What specifically broke when these tests ran?
🤖 Prompt for AI Agents
In packages/utils/src/tests/dlog.test.ts around lines 202 to 213 the tests were
commented out without explanation which reduces coverage and violates the PR
goal to restore tests; either re-enable and fix the root cause or document why
they remain disabled: reproduce the failing behavior locally, identify and fix
the underlying issue (likely around debug.enable/ns behavior or Jest interaction
with debug namespaces) then re-enable the three test blocks and ensure they
assert log.enabled correctly; if you cannot fix immediately, replace the
commented blocks with a single TODO comment referencing a new issue ID (create a
tracker issue describing the failure, steps to reproduce, and owner) and add a
short skipped test (using test.skip or jest.skip) with a link to that issue so
coverage and intent are explicit; alternatively, if the tests are no longer
valid, remove them and note this removal in the changelog/PR description.
1 participant
No description provided.