fix: Phase 3 Medium Priority Fixes - Memory Leaks, Types, Logging, Testnet #12
Conversation
…stnet - Fix MutationObserver memory leak in PubkyURLHandler - Store observer reference and add cleanup() method - Prevents memory leaks on long-running pages - Create SDK Type Definitions - New file: src/types/pubky.d.ts - Provides type safety for @synonymdev/pubky SDK - Reduces 'any' type usage across codebase - Add setLogLevel Configuration - Configure SDK logging in pubky-client-factory - Error level in production, debug in development - Add Testnet Support - Support VITE_PUBKY_NETWORK environment variable - Use TestnetClient when testnet mode enabled - Allows development against testnet without mainnet pollution - Remove console.log Statements - Replace 20 console.log/error statements in background.ts - Replace 9 console.log/error statements in offscreen.ts - Use logger utility consistently for all logging - Document @ts-ignore Comments - Add explanations for all 3 @ts-ignore instances - Include tracking links and TODOs where applicable - Document Manifest Security Decisions - Add comments explaining web_accessible_resources scope - Add comment explaining CSP wasm-unsafe-eval requirement - Both are required for core functionality Addresses medium priority issues from production audit: - Memory leaks from unmanaged observers - Type safety improvements (79 any types) - Logging consistency - Development environment support Phase 3 of comprehensive production audit remediation plan.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "content_security_policy": { | ||
| "extension_pages": "script-src 'self' 'wasm-unsafe-eval'; object-src 'self'" | ||
| // Note: 'wasm-unsafe-eval' is required for Pubky SDK WebAssembly support | ||
| }, | ||
| "web_accessible_resources": [ | ||
| { | ||
| "resources": ["src/profile/profile-renderer.html"], | ||
| "matches": ["<all_urls>"] | ||
| // Note: Profile renderer needs to be accessible from any page to display | ||
| // pubky:// URLs. This is a core feature requirement. | ||
| } |
There was a problem hiding this comment.
Manifest v3 files must be valid JSON, but manifest.json now includes // comments in the CSP and web_accessible_resources sections. Chrome rejects manifests containing comments, so the extension will fail to load in any environment until the file is valid JSON again.
Useful? React with 👍 / 👎.
| // Configure log level if available | ||
| if ('setLogLevel' in pubkyModule && typeof (pubkyModule as any).setLogLevel === 'function') { | ||
| const setLogLevel = (pubkyModule as any).setLogLevel; | ||
| if (process.env.NODE_ENV === 'production') { | ||
| setLogLevel('error'); | ||
| } else { |
There was a problem hiding this comment.
Both client initialization paths now branch on process.env.NODE_ENV, but the extension is built with Vite (import.meta.env) and no process polyfill; accessing process in the service worker/offscreen context will throw ReferenceError: process is not defined, preventing the Pubky client from ever instantiating and blocking auth/API calls.
Useful? React with 👍 / 👎.
Phase 3: Medium Priority Fixes (Post-Launch)
This PR addresses 8 medium-priority issues identified in the production audit report.
Changes
1. Fix MutationObserver Memory Leak (1 hour)
src/content/PubkyURLHandler.tsprivate domObserver: MutationObserver | nullpropertyobserveDOMForPubkyURLs()cleanup()method to disconnect observer2. Create SDK Type Definitions (4 hours)
src/types/pubky.d.tsanytypes due to missing SDK type definitionsClient,TestnetClient,AuthRequest,PublicKey,SessionvalidateCapabilities,setLogLevel,createRecoveryFile3. Add setLogLevel Configuration (1 hour)
src/utils/pubky-client-factory.tserrorlevel onlydebuglevel for verbose logging4. Add Testnet Support (1 hour)
src/utils/pubky-client-factory.tsVITE_PUBKY_NETWORKenvironment variableTestnetClientwhen testnet mode enabled5. Remove console.log Statements (2 hours)
src/background/background.ts,src/offscreen/offscreen.ts6. Document @ts-ignore Comments (30 min)
src/content/AnnotationManager.ts- dom-anchor-text-quote typessrc/sidepanel/App.tsx- pagination cursorsrc/config/config.ts- import.meta7. Document Manifest Security Decisions (30 min)
manifest.jsonweb_accessible_resourcesscope required for pubky:// URL displaywasm-unsafe-evalrequired for Pubky SDK WebAssemblyVerification
tsc --noEmit)npm test- verify all tests passVITE_PUBKY_NETWORK=testnetRelated
Part of comprehensive production audit remediation plan:
Audit Report
See
COMPREHENSIVE_AUDIT_REPORT.mdfor full details.