Skip to content

Conversation

@MohamedBassem
Copy link
Collaborator

No description provided.

claude and others added 3 commits November 21, 2025 00:13
This commit integrates single-file-core (v1.5.61) into the browser
extension, allowing users to capture complete web page archives when
saving bookmarks.

Changes:
- Integrated single-file-core library (AGPL-3.0-or-later licensed)
- Added `useSingleFile` setting to extension options
- Created content script to capture pages using SingleFile
- Implemented asset upload functionality for captured archives
- Updated SavePage component to use SingleFile when enabled
- Added UI toggle in OptionsPage to enable/disable SingleFile
- Updated manifest.json with required permissions and content scripts

The SingleFile option can be toggled in the extension's options page.
When enabled, bookmarks will be saved with a full page archive that
includes all assets embedded inline as a single HTML file.

Technical details:
- Content script captures page using single-file-core
- Captured HTML is uploaded to /api/v1/bookmarks/singlefile endpoint
- Archive is stored as a precrawled asset and linked to the bookmark
- Source is marked as "singlefile" for bookmarks created this way

License note: The single-file-core library is licensed under AGPL-3.0.
Attribution and license information is included in the singlefile directory.
@chatgpt-codex-connector
Copy link

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 30, 2025

Walkthrough

Integrates SingleFile library into the browser extension by adding manifest permissions, a content script for page capture, TypeScript type declarations, utility functions for capturing and uploading pages, a new settings toggle, and modifications to the SavePage component to enable SingleFile-based bookmark creation.

Changes

Cohort / File(s) Summary
Configuration & Manifest
apps/browser-extension/manifest.json, apps/browser-extension/package.json
Extended manifest permissions to include activeTab, scripting, and <all_urls> host permissions. Added single-file-core@^1.5.68 dependency and registered content script singlefile-content-script.ts to run on all URLs at document idle.
Type Declarations & Library Integration
apps/browser-extension/src/single-file-core.d.ts
New TypeScript declarations for single-file-core library, exposing SingleFileOptions interface with feature toggles, PageData interface for capture output, and functions init() and getPageData().
Settings & Utilities
apps/browser-extension/src/utils/settings.ts, apps/browser-extension/src/utils/singlefile.ts
Extended settings schema with useSingleFile boolean field (default false). Added utility functions capturePageWithSingleFile() and uploadSingleFileAndCreateBookmark() with error handling, file sanitization, and custom headers support.
Content Script
apps/browser-extension/src/content-scripts/singlefile-content-script.ts
New content script that listens for CAPTURE_PAGE messages via chrome.runtime, invokes SingleFile's getPageData() with comprehensive content-cleaning options, and returns captured HTML or structured error.
UI Components
apps/browser-extension/src/OptionsPage.tsx, apps/browser-extension/src/SavePage.tsx
Added Settings UI toggle for "Use SingleFile" in OptionsPage. Modified SavePage to conditionally execute SingleFile capture-and-upload flow when settings.useSingleFile is enabled, tracking async state with isSaving and savedBookmarkId, and navigating to created bookmark on success.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • SavePage.tsx: Verify the conditional control flow logic and state transitions when SingleFile is enabled vs. disabled; confirm error handling and loading UI state management.
  • singlefile-content-script.ts: Validate message listener setup and error propagation between content script and background script; confirm SingleFile options are appropriate.
  • singlefile.ts utility functions: Review error handling paths, API endpoint correctness, FormData construction, and custom headers integration.
  • Type safety: Verify single-file-core.d.ts declarations match actual library API to prevent runtime type mismatches.
  • Settings persistence: Confirm useSingleFile setting is properly persisted and synchronized across components.

Pre-merge checks

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive No pull request description was provided by the author. Add a description explaining the changes, rationale, and any relevant context for reviewers.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding SingleFile support to the browser extension.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
apps/browser-extension/manifest.json (1)

47-56: Re‑check whether <all_urls> host permissions and matches scope are strictly needed

The new "host_permissions": ["<all_urls>"] plus a content script that matches "<all_urls>" is a large permission surface. If SingleFile doesn’t need access to non‑HTTP(S) schemes or programmatic injection outside your existing flows, consider tightening this to just the schemes you actually support for bookmarking (e.g. ["http://*/*", "https://*/*"]) and/or dropping host_permissions if content‑script matches alone suffice.

Also, since the script path is a .ts entry, please confirm the crx/vite build is configured to compile src/content-scripts/singlefile-content-script.ts into the JS file referenced from the final manifest; otherwise Chrome will fail to load the content script at runtime.

apps/browser-extension/src/content-scripts/singlefile-content-script.ts (1)

1-57: Async CAPTURE_PAGE handler looks solid; consider making capture options configurable later

The content script correctly:

  • Initializes SingleFile once.
  • Uses chrome.runtime.onMessage.addListener with return true for async sendResponse.
  • Wraps getPageData in captureCurrentPage and returns pageData.content, with errors logged and surfaced as strings.

Functionally this looks good. If you anticipate tweaking capture behavior (e.g., maxResourceSize, whether scripts are removed) based on user settings, consider centralizing the options object or wiring it to settings in a follow‑up change; for a first pass, hard‑coding is fine.

apps/browser-extension/src/utils/singlefile.ts (1)

10-80: Consider clarifying headers type annotation and update endpoint comment

Functionally this utility looks solid:

  • capturePageWithSingleFile cleanly wraps the CAPTURE_PAGE messaging and surfaces failure reasons.
  • uploadSingleFileAndCreateBookmark constructs the File and FormData, includes API key and optional custom headers, and throws with detailed server error text when the upload fails.
  • sanitizeFilename is reasonable for generating safe, bounded filenames.

Minor improvements:

The headers variable is typed as HeadersInit (a union type) but then used with index assignment (headers[key] = value). While TypeScript currently allows this, it's clearer to type it as Record<string, string> since that reflects the actual usage:

-  const headers: HeadersInit = {
+  const headers: Record<string, string> = {
     Authorization: `Bearer ${settings.apiKey}`,
   };

Additionally, the comment on line 44 says "Upload to the /v1/singlefile endpoint" but the actual endpoint is /api/v1/bookmarks/singlefile (line 45). Update the comment for accuracy.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c6b8e9 and 7a9cdb9.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (8)
  • apps/browser-extension/manifest.json (1 hunks)
  • apps/browser-extension/package.json (1 hunks)
  • apps/browser-extension/src/OptionsPage.tsx (1 hunks)
  • apps/browser-extension/src/SavePage.tsx (3 hunks)
  • apps/browser-extension/src/content-scripts/singlefile-content-script.ts (1 hunks)
  • apps/browser-extension/src/single-file-core.d.ts (1 hunks)
  • apps/browser-extension/src/utils/settings.ts (2 hunks)
  • apps/browser-extension/src/utils/singlefile.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,json,css,md}

📄 CodeRabbit inference engine (AGENTS.md)

Format code using Prettier according to project standards

Files:

  • apps/browser-extension/manifest.json
  • apps/browser-extension/src/utils/singlefile.ts
  • apps/browser-extension/src/utils/settings.ts
  • apps/browser-extension/src/single-file-core.d.ts
  • apps/browser-extension/src/SavePage.tsx
  • apps/browser-extension/src/OptionsPage.tsx
  • apps/browser-extension/package.json
  • apps/browser-extension/src/content-scripts/singlefile-content-script.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript for type safety in all source files

Files:

  • apps/browser-extension/src/utils/singlefile.ts
  • apps/browser-extension/src/utils/settings.ts
  • apps/browser-extension/src/single-file-core.d.ts
  • apps/browser-extension/src/SavePage.tsx
  • apps/browser-extension/src/OptionsPage.tsx
  • apps/browser-extension/src/content-scripts/singlefile-content-script.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Lint code using oxlint and fix issues with pnpm lint:fix

Files:

  • apps/browser-extension/src/utils/singlefile.ts
  • apps/browser-extension/src/utils/settings.ts
  • apps/browser-extension/src/single-file-core.d.ts
  • apps/browser-extension/src/SavePage.tsx
  • apps/browser-extension/src/OptionsPage.tsx
  • apps/browser-extension/src/content-scripts/singlefile-content-script.ts
🧠 Learnings (1)
📚 Learning: 2025-11-27T16:02:47.432Z
Learnt from: CR
Repo: karakeep-app/karakeep PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T16:02:47.432Z
Learning: Applies to apps/web/**/*.{ts,tsx} : Use Tailwind CSS for styling in the web application

Applied to files:

  • apps/browser-extension/package.json
🧬 Code graph analysis (3)
apps/browser-extension/src/utils/singlefile.ts (1)
apps/browser-extension/src/utils/settings.ts (1)
  • getPluginSettings (74-83)
apps/browser-extension/src/SavePage.tsx (2)
apps/browser-extension/src/utils/settings.ts (1)
  • usePluginSettings (34-72)
apps/browser-extension/src/utils/singlefile.ts (2)
  • capturePageWithSingleFile (10-22)
  • uploadSingleFileAndCreateBookmark (27-70)
apps/browser-extension/src/OptionsPage.tsx (1)
apps/browser-extension/src/components/ui/switch.tsx (1)
  • Switch (27-27)
⏰ 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). (3)
  • GitHub Check: typecheck
  • GitHub Check: tests
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/browser-extension/src/OptionsPage.tsx (1)

118-131: SingleFile toggle wiring is correct and consistent with existing settings

The new “Use SingleFile” row correctly:

  • Binds checked to settings.useSingleFile.
  • Uses the same setSettings pattern as other toggles.
  • Provides brief explanatory text.

No functional issues from this addition.

apps/browser-extension/src/utils/settings.ts (1)

16-27: useSingleFile setting is correctly added and defaulted

The new useSingleFile flag is wired into both the Zod schema and DEFAULT_SETTINGS with a default of false, which keeps existing users on the old behavior unless they opt in. This integrates cleanly with Settings and consumers like OptionsPage / SavePage.

"react": "^19.1.0",
"react-dom": "^19.1.0",
"react-router-dom": "^6.22.0",
"single-file-core": "^1.5.68",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

single-file-core npm package version 1.5.68 license size

💡 Result:

I can't find a published [email protected] — the latest npm release appears to be 1.5.48. For 1.5.48: license = AGPL-3.0-or-later; unpacked size ≈ 1.32 MB (npm) — UNPKG shows the package file breakdown (~1.27–1.32 MB total). [1][2]

Sources:

  • npm package page (single-file-core). [1]
  • UNPKG file listing for v1.5.48. [2]

single-file-core@^1.5.68 does not exist in npm registry

Version 1.5.68 is not published; the latest release is 1.5.48. This version constraint will fail to resolve during installation. Additionally, verify:

  • Whether 1.5.48 (or the intended version) is what you tested with.
  • License compatibility: single-file-core uses AGPL-3.0-or-later, a strong copyleft license with potential implications for browser extension distribution and code you link against.
  • Bundle impact: ~1.32 MB unpacked size is significant for an extension dependency.
🤖 Prompt for AI Agents
In apps/browser-extension/package.json around line 38, the dependency
"single-file-core": "^1.5.68" references a non-existent npm version; update the
version to a published release (e.g., "^1.5.48" or the exact version you
tested), run npm install to confirm resolution, and update any lockfile. Also
verify the dependency version you used in testing, evaluate AGPL-3.0-or-later
license compatibility with this extension (replace, obtain permission, or vendor
a permitted alternative if needed), and reassess bundle impact (consider smaller
alternative, lazy-load, or bundle exclusion) before committing.

Comment on lines +12 to 17
import usePluginSettings from "./utils/settings";
import {
capturePageWithSingleFile,
uploadSingleFileAndCreateBookmark,
} from "./utils/singlefile";
import { api } from "./utils/trpc";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard runSave so it doesn’t execute twice when settings finish loading

The SingleFile flow itself (capture → upload → set savedBookmarkId → refresh badge) looks good, and the new UI states isSaving / savedBookmarkId are wired correctly. The main concern is when runSave runs:

  • usePluginSettings() initializes settings to DEFAULT_SETTINGS (where useSingleFile is false) and then asynchronously replaces them with the real stored settings.
  • Your useEffect has [createBookmark, settings.useSingleFile] as deps, so:
    • It runs once on mount with settings.useSingleFile === false (default) and calls runSave().
    • After getPluginSettings() resolves and settings.useSingleFile is updated (potentially to true), the effect runs again and calls runSave() a second time.

Depending on whether there was a pre-seeded NEW_BOOKMARK_REQUEST_KEY_NAME, this can lead to double saves: once via the normal createBookmark path, and again via the current‑tab/SingleFile branch.

A minimal fix is to gate runSave on settings being initialized, using the existing isInit flag from usePluginSettings:

- import usePluginSettings from "./utils/settings";
+ import usePluginSettings from "./utils/settings";

 export default function SavePage() {
   const [error, setError] = useState<string | undefined>(undefined);
   const [isSaving, setIsSaving] = useState(false);
   const [savedBookmarkId, setSavedBookmarkId] = useState<string | undefined>(
     undefined,
   );
-  const { settings } = usePluginSettings();
+  const { settings, isPending: settingsInitialized } = usePluginSettings();

   ...
-  useEffect(() => {
+  useEffect(() => {
+    if (!settingsInitialized) {
+      return;
+    }
+
     async function getNewBookmarkRequestFromBackgroundScriptIfAny(): Promise<ZNewBookmarkRequest | null> {
       ...
     }

     async function runSave() {
       ...
     }

     runSave();
-  }, [createBookmark, settings.useSingleFile]);
+  }, [createBookmark, settingsInitialized, settings.useSingleFile]);

This way, runSave executes only after settings are loaded from storage, avoiding the “default‑then‑real” double invocation while still picking up useSingleFile correctly.

Also applies to: 23-28, 81-107, 124-143

🤖 Prompt for AI Agents
In apps/browser-extension/src/SavePage.tsx around lines 12-17 (and also apply
same change to 23-28, 81-107, 124-143), the effect that calls runSave can fire
twice because settings start as DEFAULT_SETTINGS and are replaced
asynchronously; update each useEffect to depend on settings.isInit (from
usePluginSettings) and early-return unless isInit is true so runSave only
executes after real settings are loaded, ensuring you keep the existing
dependency on settings.useSingleFile and createBookmark but gate execution with
if (!settings.isInit) return; to avoid duplicate saves.

Comment on lines +1 to +37
/**
* Type definitions for single-file-core
*/

declare module "single-file-core/single-file.js" {
export interface SingleFileOptions {
removeHiddenElements?: boolean;
removeUnusedStyles?: boolean;
removeUnusedFonts?: boolean;
compressHTML?: boolean;
removeImports?: boolean;
removeScripts?: boolean;
removeAudioSrc?: boolean;
removeVideoSrc?: boolean;
removeAlternativeFonts?: boolean;
removeAlternativeMedias?: boolean;
removeAlternativeImages?: boolean;
groupDuplicateImages?: boolean;
maxResourceSizeEnabled?: boolean;
maxResourceSize?: number;
}

export interface PageData {
content: string;
title?: string;
url?: string;
}

export function init(options?: Record<string, unknown>): void;

export function getPageData(
options?: SingleFileOptions,
initOptions?: Record<string, unknown>,
doc?: Document,
win?: Window,
): Promise<PageData>;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

single-file-core npm package API documentation getPageData init options

💡 Result:

Quick summary — how to call getPageData (SingleFile API) and the main init/options:

Usage (from an injected content script)
const { content, title, filename } = await extension.getPageData(options);

Options (examples — pass as an object):

  • removeHiddenElements: boolean
  • removeUnusedStyles: boolean
  • removeUnusedFonts: boolean
  • removeImports: boolean
  • blockScripts: boolean
  • blockAudios: boolean
  • blockVideos: boolean
  • compressHTML: boolean
  • removeAlternativeFonts: boolean
  • removeAlternativeMedias: boolean
  • removeAlternativeImages: boolean
  • groupDuplicateImages: boolean
    (You can pass any other SingleFile options as documented in the project.)

Return value (Promise resolves to an object):

  • content: HTML content of the page
  • title: page title
  • filename: filename (if filenameTemplate option defined)

Sources:

  • SingleFile wiki: "How to integrate the API of SingleFile into an extension" (getPageData usage and options). [1]
  • npm package page for single-file-core. [2]

🏁 Script executed:

# Search for usage of single-file-core in the codebase
rg "single-file-core" --type ts --type tsx --type js --type json -A 3 -B 1

Repository: karakeep-app/karakeep

Length of output: 92


🏁 Script executed:

# Check if package.json references single-file-core and find the version
rg "single-file-core" package.json -A 1 -B 1

Repository: karakeep-app/karakeep

Length of output: 47


🏁 Script executed:

# Look for content script files that might use the imported functions
fd "content" --type f -e ts -e tsx -e js | head -20

Repository: karakeep-app/karakeep

Length of output: 390


🏁 Script executed:

cat -n apps/browser-extension/src/content-scripts/singlefile-content-script.ts

Repository: karakeep-app/karakeep

Length of output: 2024


🏁 Script executed:

# Check pnpm typecheck as suggested in learnings
cd apps/browser-extension && pnpm typecheck 2>&1 | head -100

Repository: karakeep-app/karakeep

Length of output: 306


🏁 Script executed:

# Find single-file-core package to check actual types/version
fd "single-file-core" node_modules -type d | head -5

Repository: karakeep-app/karakeep

Length of output: 235


🏁 Script executed:

# Check if pnpm-lock.yaml or package.json has single-file-core info
rg "single-file-core" pnpm-lock.yaml -A 5 -B 1

Repository: karakeep-app/karakeep

Length of output: 721


🏁 Script executed:

# Look at package.json files in browser-extension
cat -n apps/browser-extension/package.json | head -50

Repository: karakeep-app/karakeep

Length of output: 2041


🌐 Web query:

single-file-core 1.5.68 TypeScript types getPageData API options blockScripts removeScripts

💡 Result:

Short answer: getPageData supports blockScripts (to block/disable scripts) but there is no documented removeScripts option in the SingleFile API/types — use blockScripts. Sources: SingleFile API docs and repo. [1][2]

Sources:

  • SingleFile wiki — "How to integrate the API ...", example options include blockScripts. [1]
  • single-file-core GitHub repository (repo with API/implementation). [2]

🌐 Web query:

single-file-core npm package 1.5.68 github source code single-file.js export

💡 Result:

I couldn't find a published single-file-core version 1.5.68. The package on npm and the GitHub repo show latest published v1.5.48, and the repository contains single-file.js (the module file) in its root. See:

  • GitHub repository (single-file.js present in repo root). [1]
  • npm package page (latest published version 1.5.48). [2]
  • jsDelivr listing showing available package files (includes single-file.js). [3]

If you want, I can fetch the single-file.js source from the repo (or from a specific published version) and show how it exports its API. Which exact version or file would you like me to open?


🌐 Web query:

single-file-core 1.5.48 getPageData options API documentation blockScripts blockAudios blockVideos

💡 Result:

Yes — in SingleFile (core v1.5.48) the extension API method getPageData accepts boolean options blockScripts, blockAudios and blockVideos. When set true they tell SingleFile to block page scripts, audio and video elements during capture (so they are not loaded/executed/embedded). Example usage from the SingleFile integration docs:

const { content, title, filename } = await extension.getPageData({
...,
blockScripts: true,
blockAudios: true,
blockVideos: true,
...
});

Docs / examples: [1][2]

Sources:

  • SingleFile "How to integrate the API..." (getPageData example showing blockScripts / blockAudios / blockVideos). [1]
  • single-file-core repository (package / codebase). [2]

🌐 Web query:

"single-file-core" github "getPageData" options filename content title actual API

💡 Result:

The SingleFile extension API method extension.getPageData(options) accepts a SingleFile options object and returns a Promise resolving to an object with these properties:

  • content — the captured HTML content of the page. [1]
  • title — the page title. [1]
  • filename — the generated filename (present when a filenameTemplate option is defined). [1]

Pass any SingleFile capture options (e.g., removeHiddenElements, blockScripts, compressHTML, filenameTemplate, etc.) in the options object; getPageData forwards them to SingleFile. [1][2]

Sources: [SingleFile API/integration docs], [SingleFile hidden/options list].


Fix type definitions to match single-file-core API: use blockScripts, blockAudios, blockVideos instead of removeScripts, removeAudioSrc, removeVideoSrc, and change PageData.url to PageData.filename

The ambient module declaration contains multiple type mismatches with the actual single-file-core v1.5.48 API:

  • removeScripts → should be blockScripts
  • removeAudioSrc → should be blockAudios
  • removeVideoSrc → should be blockVideos
  • PageData.url → should be PageData.filename

Additionally, maxResourceSizeEnabled and maxResourceSize are not part of the documented single-file-core API and should be removed or verified against the actual runtime.

The content script will pass incorrect option names to getPageData(), and consumers accessing pageData.url will receive undefined instead of the actual filename property.

🤖 Prompt for AI Agents
In apps/browser-extension/src/single-file-core.d.ts around lines 1 to 37, the
ambient module types don't match single-file-core v1.5.48: replace removeScripts
with blockScripts, removeAudioSrc with blockAudios, and removeVideoSrc with
blockVideos in SingleFileOptions; rename PageData.url to PageData.filename; and
remove (or verify and drop) maxResourceSizeEnabled and maxResourceSize from
SingleFileOptions since they are not part of the documented API. Update the
option names and the PageData property accordingly so callers pass the correct
keys and read filename instead of url.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants