diff --git a/apps/mobile/src/app/index.tsx b/apps/mobile/src/app/index.tsx index 9d91d74..3102e70 100644 --- a/apps/mobile/src/app/index.tsx +++ b/apps/mobile/src/app/index.tsx @@ -28,8 +28,7 @@ export default function Index() { secretsManager.connections.query.list, ); - const preferredStoredConnection = - storedConnectionsQuery.data?.firstConnection; + const preferredStoredConnection = storedConnectionsQuery.data?.[0]; const connectionForm = useAppForm({ // https://tanstack.com/form/latest/docs/framework/react/guides/async-initial-values defaultValues: preferredStoredConnection @@ -56,7 +55,7 @@ export default function Index() { value.host, value.port, value.username, - privateKey.privateKey, + privateKey, ); })(); @@ -231,7 +230,7 @@ const KeyPairSection = withFieldGroup({ ) : ( <> - {listPrivateKeysQuery.data?.keys.map((key) => ( + {listPrivateKeysQuery.data?.map((key) => ( ))} @@ -276,9 +275,9 @@ function PreviousConnectionsSection(props: { Loading connections... ) : listConnectionsQuery.isError ? ( Error loading connections - ) : listConnectionsQuery.data?.manifest.connections.length ? ( + ) : listConnectionsQuery.data?.length ? ( - {listConnectionsQuery.data?.manifest.connections.map((conn) => ( + {listConnectionsQuery.data?.map((conn) => ( void; }) { const detailsQuery = useQuery(secretsManager.connections.query.get(props.id)); - const details = detailsQuery.data?.details; + const details = detailsQuery.data; return ( prev + data); }); - // return () => { - // sshConn.client.off('Shell') - // } + return () => { + // // Remove the handler by clearing the internal handler map. + // // The library lacks a public `.off()`, but it routes through a single + // // handler map per event name. Deleting the handler prevents updates. + // // eslint-disable-next-line @typescript-eslint/no-explicit-any + // delete (sshConn.client as any)._handlers?.Shell; + + // Set to no-op + sshConn.client.on('Shell', () => {}); + }; }, [setShellData, sshConn.client]); useEffect(() => { return () => { - setTimeout(() => { - try { - sshConnectionManager.removeAndDisconnectSession({ sessionId }); - console.log('Disconnected from SSH server'); - } catch (error) { - console.error('Error disconnecting from SSH server', error); - } - }, 3_000); + console.log('Clean up shell screen (immediate disconnect)'); + try { + sshConnectionManager.removeAndDisconnectSession({ sessionId }); + console.log('Disconnected from SSH server'); + } catch (error) { + console.error('Error disconnecting from SSH server', error); + } }; }, [sessionId]); diff --git a/apps/mobile/src/lib/secrets-manager.ts b/apps/mobile/src/lib/secrets-manager.ts index ee226b5..09c1995 100644 --- a/apps/mobile/src/lib/secrets-manager.ts +++ b/apps/mobile/src/lib/secrets-manager.ts @@ -4,35 +4,6 @@ import * as SecureStore from 'expo-secure-store'; import * as z from 'zod'; import { queryClient } from './utils'; -const keys = { - storagePrefix: 'privateKey_', - manifestKey: 'privateKeysManifest', - chunkSize: 1800, // Safely under 2048 byte limit -} as const; - -const keyManifestSchema = z.object({ - manifestVersion: z.number().default(1), - keys: z.array( - z.object({ - id: z.string(), - priority: z.number(), - createdAtMs: z.int(), - chunkCount: z.number().default(1), - }), - ), -}); - -async function getKeyManifest() { - const rawManifest = await SecureStore.getItemAsync(keys.manifestKey); - const manifest = rawManifest - ? JSON.parse(rawManifest) - : { - manifestVersion: 1, - keys: [], - }; - return keyManifestSchema.parse(manifest); -} - // Utility functions for chunking large data function splitIntoChunks(data: string, chunkSize: number): string[] { const chunks: string[] = []; @@ -42,96 +13,264 @@ function splitIntoChunks(data: string, chunkSize: number): string[] { return chunks; } -function getChunkKey(keyId: string, chunkIndex: number): string { - return `${keys.storagePrefix}${keyId}_chunk_${chunkIndex}`; +/** + * Secure store does not support: + * - Listing keys + * - Storing more than 2048 bytes + * + * We can bypass both of those by using manifest entries and chunking. + */ +function makeBetterSecureStore(params: { + storagePrefix: string; + extraManifestFieldsSchema?: z.ZodType; +}) { + // const sizeLimit = 2048; + const sizeLimit = 2000; + const rootManifestVersion = 1; + const manifestChunkVersion = 1; + + const rootManifestKey = `${params.storagePrefix}rootManifest`; + const manifestChunkKey = (manifestChunkId: string) => + `${params.storagePrefix}manifestChunk_${manifestChunkId}`; + const entryKey = (entryId: string, chunkIdx: number) => + `${params.storagePrefix}entry_${entryId}_chunk_${chunkIdx}`; + + const rootManifestSchema = z.looseObject({ + manifestVersion: z.number().default(rootManifestVersion), + // We need to chunk the manifest itself + manifestChunksIds: z.array(z.string()), + }); + + const entrySchema = z.object({ + id: z.string(), + chunkCount: z.number().default(1), + metadata: params.extraManifestFieldsSchema ?? z.object({}), + }); + // type Entry = { + // id: string; + // chunkCount: number; + // metadata: T; + // }; + + type Entry = z.infer; + + const manifestChunkSchema = z.object({ + manifestChunkVersion: z.number().default(manifestChunkVersion), + entries: z.array(entrySchema), + }); + + async function getManifest() { + const rawRootManifestString = + await SecureStore.getItemAsync(rootManifestKey); + + console.log( + `Root manifest for ${params.storagePrefix} is ${rawRootManifestString?.length} bytes`, + ); + const unsafedRootManifest = rawRootManifestString + ? JSON.parse(rawRootManifestString) + : { + manifestVersion: rootManifestVersion, + manifestChunksIds: [], + }; + const rootManifest = rootManifestSchema.parse(unsafedRootManifest); + const manifestChunks = await Promise.all( + rootManifest.manifestChunksIds.map(async (manifestChunkId) => { + const rawManifestChunkString = await SecureStore.getItemAsync( + manifestChunkKey(manifestChunkId), + ); + if (!rawManifestChunkString) + throw new Error('Manifest chunk not found'); + console.log( + `Manifest chunk for ${params.storagePrefix} ${manifestChunkId} is ${rawManifestChunkString?.length} bytes`, + ); + const unsafedManifestChunk = JSON.parse(rawManifestChunkString); + return { + manifestChunk: manifestChunkSchema.parse(unsafedManifestChunk), + manifestChunkId, + manifestChunkSize: rawManifestChunkString.length, + }; + }), + ); + return { + rootManifest, + manifestChunks, + }; + } + + async function getEntry(id: string) { + const manifest = await getManifest(); + const manifestEntry = manifest.manifestChunks.reduce( + (_, mChunk) => + mChunk.manifestChunk.entries.find((entry) => entry.id === id), + undefined, + ); + if (!manifestEntry) throw new Error('Entry not found'); + + const rawEntryChunks = await Promise.all( + Array.from({ length: manifestEntry.chunkCount }, async (_, chunkIdx) => { + const rawEntryChunk = await SecureStore.getItemAsync( + entryKey(id, chunkIdx), + ); + console.log( + `Entry chunk for ${params.storagePrefix} ${id} ${chunkIdx} is ${rawEntryChunk?.length} bytes`, + ); + return rawEntryChunk; + }), + ); + const entry = rawEntryChunks.join(''); + return entry; + } + + async function listEntries() { + const manifest = await getManifest(); + return manifest.manifestChunks.flatMap( + (mChunk) => mChunk.manifestChunk.entries, + ); + } + + async function deleteEntry(id: string) { + const manifest = await getManifest(); + const manifestChunkContainingEntry = manifest.manifestChunks.find( + (mChunk) => mChunk.manifestChunk.entries.some((entry) => entry.id === id), + ); + if (!manifestChunkContainingEntry) throw new Error('Entry not found'); + + const manifestEntry = + manifestChunkContainingEntry.manifestChunk.entries.find( + (entry) => entry.id === id, + ); + if (!manifestEntry) throw new Error('Entry not found'); + + const deleteEntryChunksPromise = Array.from( + { length: manifestEntry.chunkCount }, + async (_, chunkIdx) => { + await SecureStore.deleteItemAsync(entryKey(id, chunkIdx)); + }, + ); + + const updateManifestChunkPromise = SecureStore.setItemAsync( + manifestChunkKey(manifestChunkContainingEntry.manifestChunkId), + JSON.stringify({ + ...manifestChunkContainingEntry, + entries: manifestChunkContainingEntry.manifestChunk.entries.filter( + (entry) => entry.id !== id, + ), + }), + ); + + await Promise.all([ + ...deleteEntryChunksPromise, + updateManifestChunkPromise, + ]); + } + + async function upsertEntry(params: { + id: string; + metadata: T; + value: string; + }) { + await deleteEntry(params.id).catch(() => { + console.log(`Entry ${params.id} not found, creating new one`); + }); + + const valueChunks = splitIntoChunks(params.value, sizeLimit); + const newManifestEntry = entrySchema.parse({ + id: params.id, + chunkCount: valueChunks.length, + ...params.metadata, + }); + const newManifestEntrySize = JSON.stringify(newManifestEntry).length; + if (newManifestEntrySize > sizeLimit / 2) + throw new Error('Manifest entry size is too large'); + const manifest = await getManifest(); + + const existingManifestChunkWithRoom = manifest.manifestChunks.find( + (mChunk) => sizeLimit > mChunk.manifestChunkSize + newManifestEntrySize, + ); + const manifestChunkWithRoom = + existingManifestChunkWithRoom ?? + (await (async () => { + const newManifestChunk = { + manifestChunk: { + entries: [], + manifestChunkVersion: manifestChunkVersion, + }, + manifestChunkId: crypto.randomUUID(), + manifestChunkSize: 0, + } satisfies NonNullable<(typeof manifest.manifestChunks)[number]>; + await SecureStore.setItemAsync( + rootManifestKey, + JSON.stringify(manifest.rootManifest), + ); + return newManifestChunk; + })()); + + manifestChunkWithRoom.manifestChunk.entries.push(newManifestEntry); + await Promise.all([ + SecureStore.setItemAsync( + manifestChunkKey(manifestChunkWithRoom.manifestChunkId), + JSON.stringify(manifestChunkWithRoom.manifestChunk), + ), + ...valueChunks.map((vChunk, chunkIdx) => + SecureStore.setItemAsync( + entryKey(newManifestEntry.id, chunkIdx), + vChunk, + ), + ), + ]); + } + + return { + getManifest, + getEntry, + listEntries, + upsertEntry, + deleteEntry, + }; } +const betterKeyStorage = makeBetterSecureStore({ + storagePrefix: 'privateKey_', + extraManifestFieldsSchema: z.object({ + priority: z.number(), + createdAtMs: z.int(), + }), +}); + +const betterConnectionStorage = makeBetterSecureStore({ + storagePrefix: 'connection_', + extraManifestFieldsSchema: z.object({ + priority: z.number(), + createdAtMs: z.int(), + modifiedAtMs: z.int(), + }), +}); + async function savePrivateKey(params: { keyId: string; privateKey: string; priority: number; }) { - const manifest = await getKeyManifest(); - - const existingKey = manifest.keys.find((key) => key.id === params.keyId); - - if (existingKey) throw new Error('Key already exists'); - - // Split the private key into chunks if it's too large - const chunks = splitIntoChunks(params.privateKey, keys.chunkSize); - const chunkCount = chunks.length; - - const newKey = { + await betterKeyStorage.upsertEntry({ id: params.keyId, - priority: params.priority, - createdAtMs: Date.now(), - chunkCount, - }; - - manifest.keys.push(newKey); - - // Save each chunk separately - for (let i = 0; i < chunks.length; i++) { - await SecureStore.setItemAsync(getChunkKey(params.keyId, i), chunks[i]!); - } - - await SecureStore.setItemAsync(keys.manifestKey, JSON.stringify(manifest)); + metadata: { + priority: params.priority, + createdAtMs: Date.now(), + }, + value: params.privateKey, + }); await queryClient.invalidateQueries({ queryKey: [keyQueryKey] }); } async function getPrivateKey(keyId: string) { - const manifest = await getKeyManifest(); - const key = manifest.keys.find((key) => key.id === keyId); - if (!key) throw new Error('Key not found'); - - // Reassemble the private key from chunks - const chunks: string[] = []; - for (let i = 0; i < key.chunkCount; i++) { - const chunk = await SecureStore.getItemAsync(getChunkKey(keyId, i)); - if (!chunk) throw new Error(`Key chunk ${i} not found`); - chunks.push(chunk); - } - - const privateKey = chunks.join(''); - return { - ...key, - privateKey, - }; + return await betterKeyStorage.getEntry(keyId); } async function deletePrivateKey(keyId: string) { - const manifest = await getKeyManifest(); - const key = manifest.keys.find((key) => key.id === keyId); - if (!key) throw new Error('Key not found'); - - manifest.keys = manifest.keys.filter((key) => key.id !== keyId); - - // Delete all chunks for this key - for (let i = 0; i < key.chunkCount; i++) { - await SecureStore.deleteItemAsync(getChunkKey(keyId, i)); - } - - await SecureStore.setItemAsync(keys.manifestKey, JSON.stringify(manifest)); + await betterKeyStorage.deleteEntry(keyId); await queryClient.invalidateQueries({ queryKey: [keyQueryKey] }); } -const connections = { - storagePrefix: 'connection_', - manifestKey: 'connectionsManifest', -} as const; - -const connectionsManifestSchema = z.object({ - manifestVersion: z.number().default(1), - connections: z.array( - z.object({ - id: z.string(), - priority: z.number(), - createdAt: z.date(), - modifiedAt: z.date(), - }), - ), -}); - export const connectionDetailsSchema = z.object({ host: z.string().min(1), port: z.number().min(1), @@ -150,82 +289,32 @@ export const connectionDetailsSchema = z.object({ export type ConnectionDetails = z.infer; -async function getConnectionManifest() { - const rawManifest = await SecureStore.getItemAsync(connections.manifestKey); - const manifest = rawManifest - ? JSON.parse(rawManifest) - : { - manifestVersion: 1, - connections: [], - }; - return connectionsManifestSchema.parse(manifest); -} - async function upsertConnection(params: { id: string; details: ConnectionDetails; priority: number; }) { - const manifest = await getConnectionManifest(); - const existingConnection = manifest.connections.find( - (connection) => connection.id === params.id, - ); - - const newConnection = existingConnection - ? { - ...existingConnection, - priority: params.priority, - modifiedAt: new Date(), - } - : { - id: params.id, - priority: params.priority, - createdAt: new Date(), - modifiedAt: new Date(), - }; - - await SecureStore.setItemAsync( - connections.manifestKey, - JSON.stringify(manifest), - ); - await SecureStore.setItemAsync( - `${connections.storagePrefix}${params.id}`, - JSON.stringify(params.details), - ); + await betterConnectionStorage.upsertEntry({ + id: params.id, + metadata: { + priority: params.priority, + createdAtMs: Date.now(), + modifiedAtMs: Date.now(), + }, + value: JSON.stringify(params.details), + }); await queryClient.invalidateQueries({ queryKey: [connectionQueryKey] }); - return existingConnection ?? newConnection; + return params.details; } async function deleteConnection(id: string) { - const manifest = await getConnectionManifest(); - const connection = manifest.connections.find( - (connection) => connection.id === id, - ); - if (!connection) throw new Error('Connection not found'); - manifest.connections = manifest.connections.filter( - (connection) => connection.id !== id, - ); - await SecureStore.setItemAsync( - connections.manifestKey, - JSON.stringify(manifest), - ); - await SecureStore.deleteItemAsync(`${connections.storagePrefix}${id}`); + await betterConnectionStorage.deleteEntry(id); await queryClient.invalidateQueries({ queryKey: [connectionQueryKey] }); } async function getConnection(id: string) { - const manifest = await getConnectionManifest(); - const connection = manifest.connections.find( - (connection) => connection.id === id, - ); - if (!connection) throw new Error('Connection not found'); - const detailsString = await SecureStore.getItemAsync( - `${connections.storagePrefix}${id}`, - ); - if (!detailsString) throw new Error('Connection details not found'); - const detailsJson = JSON.parse(detailsString); - const details = connectionDetailsSchema.parse(detailsJson); - return { ...connection, details }; + const connDetailsString = await betterConnectionStorage.getEntry(id); + return connectionDetailsSchema.parse(JSON.parse(connDetailsString)); } const connectionQueryKey = 'connections'; @@ -233,16 +322,17 @@ const connectionQueryKey = 'connections'; const listConnectionsQueryOptions = queryOptions({ queryKey: [connectionQueryKey], queryFn: async () => { - const manifest = await getConnectionManifest(); - const firstConnectionMeta = manifest.connections[0]; - const firstConnection = firstConnectionMeta - ? await getConnection(firstConnectionMeta.id) - : null; - - return { - manifest, - firstConnection, - }; + const connManifests = await betterConnectionStorage.listEntries(); + const results = await Promise.all( + connManifests.map(async (connManifest) => { + const details = await getConnection(connManifest.id); + return { + details, + id: connManifest.id, + }; + }), + ); + return results; }, }); @@ -256,7 +346,7 @@ const keyQueryKey = 'keys'; const listKeysQueryOptions = queryOptions({ queryKey: [keyQueryKey], - queryFn: getKeyManifest, + queryFn: async () => await betterKeyStorage.listEntries(), }); // https://github.com/dylankenneally/react-native-ssh-sftp/blob/ea55436d8d40378a8f9dabb95b463739ffb219fa/android/src/main/java/me/keeex/rnssh/RNSshClientModule.java#L101-L119 @@ -279,7 +369,7 @@ async function generateKeyPair(params: { export const secretsManager = { keys: { utils: { - getKeyManifest, + listPrivateKeys: () => betterKeyStorage.listEntries(), savePrivateKey, getPrivateKey, deletePrivateKey, diff --git a/doc/ssh-native-module-fix.md b/doc/ssh-native-module-fix.md new file mode 100644 index 0000000..66988f2 --- /dev/null +++ b/doc/ssh-native-module-fix.md @@ -0,0 +1,245 @@ +# Android SSH Native Module: Crash Root Cause and Fix Plan + +This document explains the dev-only crash observed when navigating back +(unmounting the Shell screen) and disconnecting an SSH session, and details the +native changes required in `@dylankenneally/react-native-ssh-sftp` to resolve it +properly. + +## Summary + +- Symptom: App crashes in development when leaving the Shell screen while an SSH + shell is active and `disconnect()` runs. Release builds do not crash. +- Root cause: The Android native module sometimes invokes the `startShell` + callback more than once and lets the shell read loop terminate via an + exception. In React Native dev mode, invoking a native callback multiple times + can crash the bridge. +- JS-side mitigations we applied in the app: + - Removed the artificial 3s timeout before disconnect; disconnect immediately + on unmount. + - Replaced the Shell event handler with a no-op on unmount to avoid setState + after unmount. +- Proper native fix (recommended upstream): + 1. Ensure `startShell` callback is invoked exactly once (on initial start + only). + 2. Make `closeShell` shut down the loop cleanly and null out resources. + 3. Guard `sendEvent` against emitting when the bridge is inactive. + 4. Optionally remove clients from the `clientPool` after disconnect. + +## Where the issue happens + +File: +`node_modules/@dylankenneally/react-native-ssh-sftp/android/src/main/java/me/keeex/rnssh/RNSshClientModule.java` + +Relevant methods: + +- `startShell(String key, String ptyType, Callback callback)` + - Starts the shell on a background thread, calls `callback.invoke()` when + connected, then loops reading lines and emitting `Shell` events. + - On stream termination, the code currently exits via exceptions + (`IOException`, etc.) and in the catch blocks also calls + `callback.invoke(error.getMessage())` a second time. + +- `closeShell(String key)` + - Closes output stream, input reader, and the channel, but it does not set the + fields to `null`. The `startShell` loop uses + `while (client._bufferedReader != null && ...)`, which relies on `null` to + terminate cleanly. + +- `sendEvent(ReactContext reactContext, String eventName, @Nullable WritableMap params)` + - Emits DeviceEventManager events without checking whether the React bridge is + alive. + +- `disconnect(String key)` + - Calls `closeShell(key)` and `disconnectSFTP(key)` and then disconnects the + session but does not remove the entry from `clientPool`. + +## Why dev-only + +React Native’s dev bridge is stricter: invoking a native callback multiple times +can trigger an error that surfaces as an immediate crash during development. In +release builds, this usually doesn’t bring the app down the same way, which +matches the behavior observed. + +Specifically: + +1. `startShell` calls the callback on success (good) but may call the same + callback again inside a catch block when the shell loop ends with an + exception (bad). That is a “callback invoked twice” scenario. + +2. Because `closeShell` does not null + `_bufferedReader`/`_dataOutputStream`/`_channel`, the read loop + (`while (client._bufferedReader != null && (line = ...))`) doesn’t exit by + the `null` check; it exits by throwing `IOException` when the stream is + closed, sending control flow to the catch block that re-invokes the callback. + +3. Emitting events after the bridge is torn down (e.g., during fast + unmount-navigate) can also cause noise in dev logs and compound timing + problems. + +## Native changes (proposed) + +1. Send the `startShell` callback once only + +- Current pattern: + - On success: `callback.invoke();` + - On exceptions: `callback.invoke(error.getMessage());` (second invocation) + +- Proposed change: + - Keep the success callback invocation. + - Remove all subsequent `callback.invoke(...)` inside the catch blocks of + `startShell`. + - Log the exception with `Log.e(...)` but do not call the callback again. The + callback is for “start shell” completion, not for “shell ended later”. + +2. Make `closeShell` cleanly terminate the read loop + +- After closing resources, set fields to `null`: + - `client._channel = null;` + - `client._dataOutputStream = null;` + - `client._bufferedReader = null;` + +- With this, the read loop conditional `client._bufferedReader != null` becomes + false and exits without throwing. + +3. Harden `sendEvent` + +- Before emitting: + - `if (reactContext == null || !reactContext.hasActiveCatalystInstance()) { return; }` + - Wrap `emit` in a try/catch to prevent rare bridge-state races: + ```java + try { + reactContext + .getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class) + .emit(eventName, params); + } catch (Throwable t) { + Log.w(LOGTAG, "Failed to emit event " + eventName, t); + } + ``` + +4. Optionally remove clients from the pool after disconnect + +- In `disconnect(String key)`, after `client._session.disconnect();`, call + `clientPool.remove(key);` so future lookups cannot access stale references. + +## Example diffs (illustrative) + +Note: Line numbers may vary; these are conceptual patches to apply in the +indicated methods. + +### In `startShell` + +```java +// On success (keep): +callback.invoke(); + +// In catch blocks (change): +} catch (JSchException error) { + Log.e(LOGTAG, "Error starting shell: " + error.getMessage()); + // DO NOT invoke callback again here +} catch (IOException error) { + Log.e(LOGTAG, "Error starting shell: " + error.getMessage()); + // DO NOT invoke callback again here +} catch (Exception error) { + Log.e(LOGTAG, "Error starting shell: " + error.getMessage()); + // DO NOT invoke callback again here +} +``` + +### In `closeShell` + +```java +if (client._channel != null) { + client._channel.disconnect(); +} +if (client._dataOutputStream != null) { + client._dataOutputStream.flush(); + client._dataOutputStream.close(); +} +if (client._bufferedReader != null) { + client._bufferedReader.close(); +} + +// Ensure the shell loop terminates cleanly: +client._channel = null; +client._dataOutputStream = null; +client._bufferedReader = null; +``` + +### In `sendEvent` + +```java +private void sendEvent(ReactContext reactContext, String eventName, @Nullable WritableMap params) { + if (reactContext == null || !reactContext.hasActiveCatalystInstance()) { + return; + } + try { + reactContext + .getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class) + .emit(eventName, params); + } catch (Throwable t) { + Log.w(LOGTAG, "Failed to emit event " + eventName, t); + } +} +``` + +### In `disconnect` + +```java +SSHClient client = clientPool.get(key); +if (client != null) { + client._session.disconnect(); + clientPool.remove(key); // optional but recommended +} +``` + +## Why these changes fix the issue + +- Single-callback guarantee: React Native mandates native callbacks are invoked + once per request. Making `startShell` callback only for the initial start + satisfies this and prevents dev crashes. +- Clean loop termination: Nulling the stream references ensures the shell loop + exits by condition rather than by exception, avoiding the catch path that + previously re-invoked the callback. +- Safe event emission: Avoids emitting into a destroyed bridge during fast + navigation/unmount cycles, reducing flakiness in dev. +- Resource hygiene: Removing client entries and nulling references prevents + accidental reuse of stale state and helps GC. + +## What we changed in app code (JS) + +- In `apps/mobile/src/app/shell.tsx`: + - Removed `setTimeout` and disconnect immediately in the unmount cleanup. + - Replaced the `Shell` event handler with a no-op in the effect cleanup to + avoid setState on an unmounted component while native drains. + - We deliberately did not call `closeShell()` ourselves, since the library’s + `disconnect()` already handles it. + +These app-level changes reduce the timing window and stop React state updates +after unmount. They help, but the true fix is in the native module as outlined +above. + +## Testing + +1. Build a Dev Client (or run a development build) so the native module is + loaded. +2. Connect, navigate to the shell, then press back to unmount while the shell is + active. +3. Capture logs: + - `adb logcat --pid=$(adb shell pidof -s dev.fressh.app) RNSSHClient:V ReactNative:V ReactNativeJS:V AndroidRuntime:E *:S` +4. With the native changes applied, verify: + - No “callback invoked twice” or RN bridge callback violations. + - No crash in dev. + - Events stop cleanly and disconnect completes. + +## Upstreaming + +These fixes are small, safe, and self-contained. Consider opening a PR to +`@dylankenneally/react-native-ssh-sftp` with: + +- Callback discipline in `startShell`. +- Clean resource nulling in `closeShell`. +- Safe `sendEvent`. +- Optional `clientPool.remove(key)` on disconnect. + +Until it’s merged, you can maintain a `patch-package` to keep the project stable +in development.