DCON-4311 add ability to show getDataSource information for a connection

[JoshuaASmith]

[gecko-security]

Gecko PR Review

No vulnerabilities found, LGTM.

Summary

This PR updates the bwell-sdk-ts dependency from a pre-release version (2.0.0-beta-rc.1765486931) to a stable beta (2.0.0-beta.5) in package.json and package-lock.json, along with minor README formatting fixes. The dependency upgrade in package-lock.json shows cascading updates to @babel/* packages and other transitive dependencies, which may introduce behavioral changes or vulnerabilities in the build toolchain and SDK integration.

Important Files Changed

4 important files out of 5 changed

File Path	Why it matters
bwell-typescript-react/package.json	Direct dependency version change for @icanbwell/bwell-sdk-ts from pre-release to 2.0.0-beta.5 requires verification of breaking changes and API compatibility with Connections.tsx and connectionSlice.ts.
bwell-typescript-react/package-lock.json	Transitive dependency updates (especially @babel/* from 7.24.6 to 7.29.7) may affect code transpilation, runtime behavior, and security posture; requires audit for known CVEs in updated versions.
bwell-typescript-react/src/store/connectionSlice.ts	Redux slice managing connection state likely depends on bwell-sdk-ts API surface; must verify no breaking changes in SDK types or methods used for connection management.
bwell-typescript-react/src/pages/Connections.tsx	React component consuming connectionSlice and SDK may rely on API contracts that changed between SDK versions; regression testing required for connection workflow.

Omitted 1 lower-signal changed file.

Sequence Diagram

sequenceDiagram
  participant Reviewer
  participant PR as Changed Files
  Reviewer->>PR: Inspect changed files and control flow
  Reviewer->>Reviewer: Validate security assumptions

Loading

_{Edit Gecko PR Settings | Reviewed commit: 660e8fd}

[claude]

LGTM — the flagged issues are minor nits in a sample/test app and don't block merging.

Extended reasoning...

Overview

This PR adds a "View Data Source" feature to the Connections page of the bwell-typescript-react sample app: a new getDataSource thunk in connectionSlice.ts, a button/render block in Connections.tsx, and a bump of @icanbwell/bwell-sdk-ts to a pinned 2.0.0-beta.5 (which regenerated package-lock.json). README also got minor markdown-linting touch-ups (bullet style, whitespace).

Security risks

None of note. This is a reference/sample TypeScript app (not the production SDK or a production service) exercising a new SDK method (connection.getDataSource) with no new auth, crypto, or permission logic. The dependency bump pulls a pinned pre-release version of the internal SDK; no new attack surface is introduced.

Level of scrutiny

Low-to-moderate. This is sample/testing code (bwell-typescript-react), self-contained to one feature (Connections page), and follows the same Redux Toolkit thunk/slice pattern already used elsewhere in the app (e.g., memberConnections). The three findings surfaced by the bug-hunting pass are legitimate but appropriately scoped as nits: an unintended npm-registry switch in the lockfile, a race condition where out-of-order responses across two quick "View Data Source" clicks could show the wrong connection's data in a debug JSON panel, and stale dataSource state persisting across reload/navigation/delete because the mount effect doesn't call the existing resetState() action. None of these cause data loss, security exposure, or affect any production/critical path — they affect a debug display in a sample app.

Other factors

The repository's blanket CODEOWNERS rule (* @icanbwell/enterprise-architecture @icanbwell/product-and-delivery) applies uniformly to all files, so it doesn't single out this change as unusually sensitive. No CI workflow in this repo runs npm ci/install against bwell-typescript-react, so the lockfile registry nit has no demonstrated build-breaking effect today. Given the small size, sample-app context, and existing in-repo patterns being followed correctly, I don't see a need for mandatory human re-review beyond what the inline comments already surface.

[claude]

🟡 The bwell-typescript-react/package-lock.json was regenerated resolving all 383 packages via the public https://registry.npmjs.org instead of the internal Artifactory mirror (artifacts.bwell.com) used previously and still used by the sibling bwell-react-native/package-lock.json. This looks like an unintended side effect of running npm install without the corporate .npmrc while bumping @icanbwell/bwell-sdk-ts to 2.0.0-beta.5, rather than an intentional change — worth confirming and regenerating against the internal mirror if so.

Extended reasoning...

What changed: Before this PR, every resolved dependency in bwell-typescript-react/package-lock.json pointed at https://artifacts.bwell.com/artifactory/api/npm/virtual-npm/... (visible on the @esbuild/*, @rollup/*, and @swc/* entries in the diff). After this PR, all 383 resolved entries in the file point directly at https://registry.npmjs.org, and there are zero remaining references to artifacts.bwell.com. The sibling bwell-react-native/package-lock.json in the same monorepo still resolves exclusively through the internal Artifactory mirror (1028 references), which strongly suggests the mirror is this org's established convention rather than something being deliberately phased out.

Likely cause: The PR's core change is bumping @icanbwell/bwell-sdk-ts from ^2.0.0-beta-rc.1765486931 to a pinned 2.0.0-beta.5. That triggered npm install, which regenerated the whole lockfile. If that install ran on a machine without the corporate .npmrc (which is not committed to the repo — there's no .npmrc anywhere in this tree) or with a registry override pointing at the public registry, npm would re-resolve every dependency (not just the SDK) against registry.npmjs.org, which is exactly the diff we see.

Why I'm not confident this needs to block the PR: After checking the current state of the repository, two mitigating factors stand out. First, there is no .npmrc committed anywhere in this repo, so the "internal mirror" convention isn't actually enforced by the codebase — it's only an artifact of whichever machine last ran npm install. Second, the only GitHub Actions workflows in this repo (generate_apk.yml, instrumented_tests.yml, release.yml) build the Android/React Native app; none of them run npm ci/npm install against bwell-typescript-react. So the concrete "this breaks network-restricted CI" scenario isn't demonstrable today in this repository, and this is a public example/reference repo (icanbwell/bwell-sdk-example) whose primary audience is external developers who likely cannot reach artifacts.bwell.com anyway — for them, public-registry URLs are arguably more correct, not a regression.

Proof of the underlying fact (not necessarily the impact):

1. grep -c artifacts.bwell.com bwell-typescript-react/package-lock.json → 0 (post-PR)
2. grep -c registry.npmjs.org bwell-typescript-react/package-lock.json → 383 (post-PR)
3. grep -c artifacts.bwell.com bwell-react-native/package-lock.json → 1028 (unchanged sibling lockfile)
4. Diff hunks such as @esbuild/aix-ppc64 and @rollup/rollup-android-arm-eabi show resolved URLs literally rewritten from artifacts.bwell.com/artifactory/... to registry.npmjs.org/... for the exact same or bumped version.

Recommendation: If the internal Artifactory mirror is required for any real install path (a local dev machine's .npmrc, a not-yet-added CI step, or a future one), it'd be worth confirming this was intentional and, if not, regenerating the lockfile with the corporate registry configured. But since no committed config or existing CI depends on the mirror for this package today, and the repo is public-facing, this doesn't look like something that should block the merge — flagging it mainly so the author can confirm the registry switch was intended.

[JoshuaASmith]

Confirmed — this was an unintended side effect of a full node_modules/package-lock.json reset I ran to fix an unrelated corrupted lockfile entry, done without the internal Artifactory registry configured. I looked into pointing the default registry at artifacts.bwell.com/artifactory/api/npm/virtual-npm/ to match the sibling app, but that repo key returns 404 for public packages from this machine (likely wrong repo key or missing auth) — will need to confirm the correct internal registry config with the platform team separately rather than guess. Leaving this on the public registry for now since, as noted, no CI step in this repo depends on the internal mirror and it's a public example app.

[JoshuaASmith]

Update: resolved properly. The 404 was missing auth — virtual-npm is the correct JFrog repo (matches the convention in bwell's Root.io/JFrog migration for Node.js, e.g. api-gateway's .yarnrc.yml). Added the registry + auth token to .npmrc and regenerated the lockfile; it now resolves all 382 packages via artifacts.bwell.com, matching bwell-react-native's lockfile again.

[claude]

🟡 getDataSource results are stored in a single shared state.dataSource field with no requestId/connectionId correlation, so if a user clicks 'View Data Source' on one connection then quickly clicks it on another before the first request resolves, an out-of-order response can leave the panel showing the wrong connection's data. Since the panel never displays which connectionId the shown data belongs to, this mismatch would be silent and hard to detect; consider tracking the requesting connectionId alongside the result (or ignoring stale responses) and labeling the panel with it.

Extended reasoning...

What the bug is

connectionSlice.ts stores the result of the getDataSource thunk in a single shared state.dataSource field (lines 62-77). There is no requestId or connectionId stored alongside the result to correlate a response with the click that triggered it. getDataSource.pending resets state.dataSource = null, and getDataSource.fulfilled unconditionally overwrites state.dataSource = action.payload?.data ?? null whenever any in-flight getDataSource thunk resolves — regardless of which row/button click started it.

How it manifests

In Connections.tsx, each row renders a "View Data Source" button that dispatches getDataSource(params.row.id) (line ~34-42). createAsyncThunk from Redux Toolkit does not cancel or dedupe previous in-flight requests by default — each dispatch spins up an independent thunk lifecycle. If a user clicks "View Data Source" for connection A, and then — before A's request resolves — clicks it for connection B, both requests race independently. Whichever network response settles last wins the final write to state.dataSource, irrespective of click order. If B's backend responds faster than A's (a very plausible scenario, e.g. different downstream data source latencies), A's late-arriving response silently overwrites B's data, even though B was the user's most recent action.

Why nothing already prevents this

The pending handlers state.dataSource = nullmight look like a safeguard, but it only clears state at request start — it does not tag the eventualfulfilled/rejectedresult with which request produced it, so it cannot detect or discard an out-of-order response. RTK'screateAsyncThunkdoes exposeaction.meta.requestIdand supports cancellation viaabort()`, but neither is used here.

Impact

The rendered "Data Source" panel (Connections.tsx, ~line 79-84) is a raw <pre>{JSON.stringify(dataSource, null, 2)}</pre> block with no connectionId label. So when the wrong connection's data is displayed, there is no visual cue distinguishing it from the correct result — a developer/QA user relying on this panel to inspect a specific connection's data source would be silently misled.

Step-by-step proof

1. User clicks "View Data Source" for connection A. getDataSource("A") thunk starts; state.dataSource is set to null.
2. Before A's network call resolves, user clicks "View Data Source" for connection B. getDataSource("B") thunk starts; state.dataSource is set to null again (no-op here since it was already null).
3. Connection B's backend responds quickly; getDataSource.fulfilled fires for B, setting state.dataSource = B's data. The panel now correctly shows B's data source.
4. Connection A's backend, being slower (e.g., higher latency source), responds afterward; getDataSource.fulfilled fires for A, overwriting state.dataSource = A's data.
5. The panel now displays connection A's data source, even though the user's last / current intent was to view connection B's data — and there is no connectionId shown anywhere to indicate the mismatch.

Suggested fix

Store the requesting connectionId alongside the result (e.g., { connectionId, data }), and either (a) ignore a fulfilled action if its action.meta.arg (the connectionId) no longer matches the currently-selected/most-recently-clicked connection, or (b) simply always display the connectionId next to the rendered data so a stale result is at least visible to the user. This is a narrow, low-likelihood race (requires rapid clicks on two different rows plus out-of-order network responses) in a sample/testing app, so it is not merge-blocking, but is worth addressing given how easy it would be to add the correlation.

[JoshuaASmith]

Fixed in the latest commit: added dataSourceConnectionId to connectionSlice state, set on getDataSource.pending. fulfilled/rejected now bail out if action.meta.arg no longer matches the current dataSourceConnectionId, so an out-of-order response from a prior click can't overwrite a newer one. The panel header in Connections.tsx now also displays the connectionId the shown data belongs to, so any residual staleness is visible rather than silent.

[claude]

🟡 The new dataSource state introduced by this PR is never cleared on mount, so a stale/orphaned "View Data Source" JSON payload can persist across a page reload, navigating away and back, or even after the underlying connection is deleted (the Delete button only re-dispatches getMemberConnections, not a reset). This diverges from the established pattern in Labs.tsx/Medications.tsx, which dispatch resetState() on mount to clear stale detail-panel data; the fix is a one-line dispatch(connectionSlice.actions.resetState()) in the Connections mount effect.

Extended reasoning...

This PR adds a getDataSource thunk and a dataSource field to connectionSlice.ts, along with a "View Data Source" button and a render block in Connections.tsx that displays dataSource as raw JSON whenever it is truthy. The problem is that nothing on the Connections page ever resets dataSource except the getDataSource thunk itself (on .pending, right when the button is clicked). Because store.ts wraps the entire combineReducers output in persistReducer with persistConfig = { key: 'root', storage } and no whitelist/blacklist, the whole connections slice — including dataSource — is persisted to localStorage and rehydrated on every session start.

The codebase already has an established convention for handling exactly this kind of stale-detail-panel problem: both Labs.tsx and Medications.tsx dispatch <slice>.actions.resetState() inside a mount useEffect specifically to clear out a previously-fetched detail/knowledge payload before the page renders. Connections.tsx's mount effect, by contrast, only dispatches getMemberConnections() — it never calls connectionSlice.actions.resetState(), even though that reducer already exists in connectionSlice.ts and is simply unused.

Step-by-step reproduction:

1. User navigates to the Connections page and clicks "View Data Source" for connection A. getDataSource.fulfilled sets state.dataSource = <connection A's data>.
2. User reloads the page (or navigates to another page and back). redux-persist rehydrates the entire connections slice from localStorage, including the stale dataSource for connection A.
3. Connections.tsx's mount effect only fires getMemberConnections(), which resets memberConnections but does not touch dataSource (see getMemberConnections.pending in connectionSlice.ts).
4. The page renders with {dataSource && ...} still true, showing connection A's stale JSON — with no indication that it is stale, and no re-fetch was performed.
5. Worse, if the user now clicks "Delete" on connection A, the Delete handler awaits deleteConnectionById and re-dispatches getMemberConnections() only. The now-deleted connection's dataSource remains displayed as if it were still valid.

The impact is limited to a confusing/misleading debug-style JSON panel in what is a sample/QA app for exercising the SDK — there is no crash, data loss, or corruption of persisted state beyond the display itself. Given that, and that the fix is a trivial one-liner that mirrors an existing in-repo pattern (dispatch(connectionSlice.actions.resetState()) in the mount useEffect, matching Labs.tsx/Medications.tsx), this is best treated as a nit rather than a merge blocker.

[JoshuaASmith]

Fixed in the latest commit: added dispatch(connectionSlice.actions.resetState()) to the Connections mount effect, matching the Labs.tsx/Medications.tsx convention. This clears any stale dataSource from a prior session (redux-persist rehydrates the whole slice) before the page renders.