Conversation
📝 WalkthroughWalkthroughEnables the @hoprnet scoped npm registry, updates @hoprnet/hopr-sdk version, switches MTU source in SessionsPage to session.hoprMtu, and adjusts getChannelsCorruptedThunk.fulfilled to store the payload array directly instead of expecting a channelIds field. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI as UI
participant Store as Redux Store
participant Thunk as getChannelsCorruptedThunk
participant API as Backend API
UI->>Store: dispatch(getChannelsCorruptedThunk)
Store->>Thunk: invoke async thunk
Thunk->>API: GET /channels/corrupted
API-->>Thunk: [array of channel IDs]
Thunk-->>Store: fulfilled(action.payload = array)
note over Store: Changed: reducer stores payload directly
Store->>Store: state.channels.corrupted.data = payload || []
Store-->>UI: state updated (isFetching=false)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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 |
There was a problem hiding this comment.
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 (4)
src/pages/node/sessions.tsx (1)
169-173: Return path renders the forward path again (user-visible bug).Uses
forwardPathtwice; second should bereturnPath.- {JSON.stringify(session.forwardPath) + {JSON.stringify(session.returnPath) .replace(/{|}|\[|\]|"/g, '') .replace('IntermediatePath:', 'IntermediatePath:\n') .replace(/,/g, ' ')}src/store/slices/node/actionsAsync.ts (3)
914-915: isFetching toggled wrong after isNodeReady success.Fulfilled should set
isFetching = false.- state.nodeIsReady.isFetching = true; + state.nodeIsReady.isFetching = false;
1202-1210: findIndex misuse can overwrite the last token (index -1).
if (tokenExists)is wrong for index checks; use!== -1.- const tokenExists = state.tokens.data?.findIndex((token) => token.id === action.payload?.id); - - if (tokenExists) { - state.tokens.data[tokenExists] = action.payload; - } else { - state.tokens.data.push(action.payload); - } + const idx = state.tokens.data?.findIndex((t) => t.id === action.payload?.id) ?? -1; + if (idx !== -1) { + state.tokens.data[idx] = action.payload; + } else { + state.tokens.data.push(action.payload); + }
1274-1284: Same findIndex bug in pings.Index 0 is falsy; -1 is truthy → incorrect branch.
- if (pingExists) { + if (pingExists !== -1) { state.pings[pingExists] = { latency: action.payload.latency, peerId: action.payload.peerId, }; } else {
🧹 Nitpick comments (3)
src/pages/node/sessions.tsx (2)
217-217: Simplify empty check for export button.
sessionsis an array; use length directly.- disabled={!sessions || Object.keys(sessions).length === 0} + disabled={!sessions || sessions.length === 0}
230-231: Default sort key likely mismatched.
orderByDefault="number"doesn’t match any header key; probably should be"id".- orderByDefault="number" + orderByDefault="id"src/store/slices/node/actionsAsync.ts (1)
983-983: Strip console noise in reducers.
console.log('getChannels', ...)in reducer spam logs.- console.log('getChannels', channels); + // console.log('getChannels', channels);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
.npmrc(1 hunks)package.json(1 hunks)src/pages/node/sessions.tsx(1 hunks)src/store/slices/node/actionsAsync.ts(1 hunks)
🔇 Additional comments (3)
package.json (1)
9-9: Pre-release SDK pin confirmed in lockfile; CI auth still needs verification
- yarn.lock contains @hoprnet/hopr-sdk@3.0.3-pr.156-20250910174548 (yarn.lock lines 722–724); no lingering 3.0.1 refs.
- Verify CI can authenticate to the private registry (europe-west3-npm.pkg.dev) for the @hoprnet scope — check CI npmrc/registry credentials and token.
src/store/slices/node/actionsAsync.ts (1)
1081-1081: Resolved — corrupted channels payload is an array; no downstream breakage found.
Only usage found: src/pages/node/info/index.tsx (line 67) reads store.node.channels.corrupted.data.length > 0 — array access is compatible..npmrc (1)
1-1: Registry confirmed — configure CI-sourced auth token; avoid committing project-level always-auth
- Verified: .npmrc already sets @hoprnet:registry=https://europe-west3-npm.pkg.dev/hoprassociation/npm/ (npm config get returned this URL).
- npm warned: "Unknown project config 'always-auth' (//registry.npmjs.org/:always-auth) … will stop working in the next major version" — do NOT add //europe-west3-npm.pkg.dev/:always-auth=true in the repo .npmrc.
- Action: supply an auth token from CI and inject it at build time (do not commit secrets). Example: set HOPR_NPM_TOKEN in CI and write to .npmrc during the build: //europe-west3-npm.pkg.dev/:_authToken=${HOPR_NPM_TOKEN}.
Summary by CodeRabbit