Skip to content

feat(SUP-23): implement session storage for pending capture state restoration #104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
arjun-veer wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat(SUP-23): implement session storage for pending capture state restoration #104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b6b2adc
feat: implement session storage for pending capture state restoration
arjun-veer Feb 9, 2026
36b798f
fix: improve error handling and state initialization in capture compo…
arjun-veer Feb 9, 2026
7b008f9
feat: refactor capture memory manager to use extension-isolated stora…
arjun-veer Feb 9, 2026
9701a4b
Merge branch 'main' into feat/persist-auto-capture
mikr13 Feb 10, 2026
76b400d
feat: enhance capture memory manager integration and improve state ha…
arjun-veer Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/components/features/setting/capture-settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ const logger = createLogger("capture-settings");

export const CaptureSettings = () => {
const captureEnabledId = useId();
const [captureEnabled, setCaptureEnabled] = useState<boolean | undefined>(
undefined,
);
const [captureEnabled, setCaptureEnabled] = useState<boolean>(false);
const [neverAskSites, setNeverAskSites] = useState<string[]>([]);

useEffect(() => {
Expand Down
219 changes: 186 additions & 33 deletions src/entrypoints/content/components/capture-memory-manager.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
import { SparklesIcon, X } from "lucide-react";
import { createRoot, type Root } from "react-dom/client";
import type { ContentScriptContext } from "wxt/utils/content-script-context";
import {
createShadowRootUi,
type ShadowRootContentScriptUi,
} from "wxt/utils/content-script-ui/shadow-root";
import {
Accordion,
AccordionContent,
Expand All @@ -24,21 +17,47 @@ import {
} from "@/components/ui/card";
import { contentAutofillMessaging } from "@/lib/autofill/content-autofill-messaging";
import { createLogger } from "@/lib/logger";
import { storage } from "@/lib/storage";
import { storage as storageItems } from "@/lib/storage";
import { addNeverAskSite } from "@/lib/storage/capture-settings";
import type { CapturedFieldData } from "@/types/autofill";
import { Theme } from "@/types/theme";
import { SparklesIcon, X } from "lucide-react";
import { createRoot, type Root } from "react-dom/client";
import type { ContentScriptContext } from "wxt/utils/content-script-context";
import {
createShadowRootUi,
type ShadowRootContentScriptUi,
} from "wxt/utils/content-script-ui/shadow-root";
import { CaptureResultLoader } from "./capture-result-loader";

const logger = createLogger("capture-memory-manager");

const HOST_ID = "superfill-capture-memory";
const STORAGE_KEY = "session:pendingCaptureState";
const STORAGE_VERSION = 1;
const MAX_AGE_MS = 10 * 60 * 1000;

const pendingCaptureStorage = storage.defineItem<StoredCaptureState | null>(
STORAGE_KEY,
{
fallback: null,
version: STORAGE_VERSION,
},
);

type CaptureResultState = "saving" | "success" | "info" | "error";

interface CaptureMemoryProps {
interface StoredCaptureState {
capturedFields: CapturedFieldData[];
siteUrl: string;
siteTitle: string;
siteDomain: string;
timestamp: number;
}

interface CaptureMemoryProps {
siteTitle: string | undefined;
siteDomain: string | undefined;
capturedFields: CapturedFieldData[];
onSave: () => void;
onDismiss: () => void;
Expand Down Expand Up @@ -91,7 +110,7 @@ const CaptureMemory = ({
id="save-memory-title"
>
Superfill detected {totalFields} field
{totalFields !== 1 ? "s" : ""} you filled on {siteTitle}. Save
{totalFields !== 1 ? "s" : ""} you filled on {siteTitle || "this site"}. Save
them for future use?
</CardDescription>
<CardAction>
Expand Down Expand Up @@ -158,9 +177,9 @@ const CaptureMemory = ({
>
<span
className="truncate inline-block max-w-full"
title={`Never ask for ${siteDomain}`}
title={`Never ask for ${siteDomain || "this site"}`}
>
Never ask for {siteDomain}
Never ask for {siteDomain || "this site"}
</span>
</Button>
</CardFooter>
Expand All @@ -187,18 +206,29 @@ export class CaptureMemoryManager {
private resultState: CaptureResultState | null = null;
private savedCount = 0;
private skippedCount = 0;
private siteTitle: string | undefined = undefined;
private siteDomain: string | undefined = undefined;

async show(
ctx: ContentScriptContext,
capturedFields: CapturedFieldData[],
options?: { skipPersist?: boolean },
): Promise<void> {
this.currentFields = capturedFields;
const siteTitle = document.title;
const siteDomain = window.location.hostname;
if (this.siteTitle === undefined) {
this.siteTitle = document.title;
}
if (this.siteDomain === undefined) {
this.siteDomain = window.location.hostname;
}

if (!options?.skipPersist) {
await this.savePendingState();
}

logger.info("Showing capture memory", {
fieldsCount: capturedFields.length,
siteDomain,
siteDomain: this.siteDomain,
});

try {
Expand All @@ -214,7 +244,7 @@ export class CaptureMemoryManager {

void this.applyTheme(shadow);
this.root = createRoot(container);
this.render(siteTitle, siteDomain);
this.render();
return this.root;
},
onRemove: (root) => {
Expand All @@ -223,16 +253,20 @@ export class CaptureMemoryManager {
},
});
this.ui.mount();
} else {
this.render();
}

this.render(siteTitle, siteDomain);
this.isVisible = true;
} catch (error) {
logger.error("Failed to show capture memory:", error);
await this.clearPendingState();

try {
this.ui?.remove();
} catch {}
} catch (error) {
logger.debug("Error removing UI during cleanup:", error);
}

this.ui = null;
this.root = null;
Expand All @@ -245,6 +279,8 @@ export class CaptureMemoryManager {

logger.info("Hiding capture memory");

await this.clearPendingState();

if (this.ui) {
this.ui.remove();
this.ui = null;
Expand All @@ -257,7 +293,7 @@ export class CaptureMemoryManager {

private async applyTheme(shadow: ShadowRoot): Promise<void> {
try {
const settings = await storage.uiSettings.getValue();
const settings = await storageItems.uiSettings.getValue();
const theme = settings.theme;

const host = shadow.host as HTMLElement;
Expand All @@ -278,17 +314,20 @@ export class CaptureMemoryManager {
}
}

private render(siteTitle: string, siteDomain: string): void {
private render(): void {
if (!this.root) return;

this.root.render(
<CaptureMemory
siteTitle={siteTitle}
siteDomain={siteDomain}
siteTitle={this.siteTitle ?? document.title}
siteDomain={this.siteDomain ?? window.location.hostname}
capturedFields={this.currentFields}
onSave={() => this.handleSave()}
onDismiss={() => this.handleDismiss()}
onNeverAsk={() => this.handleNeverAsk(siteDomain)}
onNeverAsk={() => {
const domain = this.siteDomain ?? window.location.hostname;
this.handleNeverAsk(domain);
}}
resultState={this.resultState}
savedCount={this.savedCount}
skippedCount={this.skippedCount}
Expand All @@ -301,19 +340,14 @@ export class CaptureMemoryManager {
this.resultState = null;
this.savedCount = 0;
this.skippedCount = 0;
const siteTitle = document.title;
const siteDomain = window.location.hostname;
this.render(siteTitle, siteDomain);
this.render();
}

private async handleSave(): Promise<void> {
logger.info("Saving captured memories");

const siteTitle = document.title;
const siteDomain = window.location.hostname;

this.resultState = "saving";
this.render(siteTitle, siteDomain);
this.render();

try {
const result = await contentAutofillMessaging.sendMessage(
Expand All @@ -335,22 +369,22 @@ export class CaptureMemoryManager {
);

this.resultState = this.savedCount > 0 ? "success" : "info";
this.render(siteTitle, siteDomain);
this.render();

await new Promise((resolve) => setTimeout(resolve, 3000));
await this.hide();
} else {
logger.error("Failed to save memories");
this.resultState = "error";
this.render(siteTitle, siteDomain);
this.render();

await new Promise((resolve) => setTimeout(resolve, 3000));
await this.hide();
}
} catch (error) {
logger.error("Error saving memories:", error);
this.resultState = "error";
this.render(siteTitle, siteDomain);
this.render();

await new Promise((resolve) => setTimeout(resolve, 3000));
await this.hide();
Expand All @@ -373,4 +407,123 @@ export class CaptureMemoryManager {

await this.hide();
}

private async savePendingState(): Promise<void> {
try {
const state: StoredCaptureState = {
capturedFields: this.currentFields,
siteUrl: window.location.href,
siteTitle: this.siteTitle ?? document.title,
siteDomain: this.siteDomain ?? window.location.hostname,
timestamp: Date.now(),
};
await pendingCaptureStorage.setValue(state);
logger.debug(
"Saved pending capture modal state to browser.storage.session (extension-isolated)",
);
} catch (error) {
logger.error("Failed to save pending capture state:", error);
}
}

private async clearPendingState(): Promise<void> {
try {
await pendingCaptureStorage.setValue(null);
logger.debug(
"Cleared pending capture modal state from browser.storage.session",
);
} catch (error) {
logger.error("Failed to clear pending capture state:", error);
}
}

static async restoreIfNeeded(
ctx: ContentScriptContext,
): Promise<CaptureMemoryManager | null> {
try {
const pendingState = await pendingCaptureStorage.getValue();

if (!pendingState) {
return null;
}

if (!CaptureMemoryManager.isValidStoredState(pendingState)) {
logger.warn("Invalid stored capture state, clearing");
await pendingCaptureStorage.setValue(null);
return null;
}

const currentUrl = window.location.href;
const pendingUrl = new URL(pendingState.siteUrl);
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The URL construction on lines 457-458 can throw an error if pendingState.siteUrl is not a valid URL. While this is caught by the outer try-catch, it would be better to validate the URL before attempting to parse it, or handle the specific error case with a more informative log message. This would help distinguish between corrupted URL data versus other types of errors.

Suggested change
const pendingUrl = new URL(pendingState.siteUrl);
let pendingUrl: URL;
try {
pendingUrl = new URL(pendingState.siteUrl);
} catch (parseError) {
logger.warn("Invalid pending capture siteUrl, clearing", {
siteUrl: pendingState.siteUrl,
error:
parseError instanceof Error ? parseError.message : String(parseError),
});
await pendingCaptureStorage.setValue(null);
return null;
}

Copilot uses AI. Check for mistakes.
const currentUrlObj = new URL(currentUrl);

if (pendingUrl.hostname !== currentUrlObj.hostname) {
await pendingCaptureStorage.setValue(null);
logger.info("Cleared stale pending capture from different domain");
return null;
}
Comment on lines +456 to +464
Copy link

@coderabbitai coderabbitai bot Feb 10, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

new URL() on stored siteUrl can throw on malformed data — handled by outer catch, but consider validating earlier.

If a corrupted siteUrl passes the typeof === "string" check in isValidStoredState but isn't a valid URL, new URL(pendingState.siteUrl) on line 457 throws. The outer try-catch handles it gracefully (clears state and returns null), so this isn't a functional bug, but it means a recoverable validation failure takes the same heavy error-logging path as an unexpected crash.

Proposed enhancement — validate URL in isValidStoredState 
   private static isValidStoredState(data: unknown): data is StoredCaptureState {
     if (typeof data !== "object" || data === null) return false;
 
     const state = data as Record<string, unknown>;
 
+    // Validate siteUrl is a parseable URL
+    let parsedUrl: URL;
+    try {
+      parsedUrl = new URL(state.siteUrl as string);
+    } catch {
+      return false;
+    }
+
     return (
       Array.isArray(state.capturedFields) &&
       typeof state.siteUrl === "string" &&
       typeof state.siteTitle === "string" &&
🤖 Prompt for AI Agents 
In `@src/entrypoints/content/components/capture-memory-manager.tsx` around lines
456 - 464, The code currently constructs new URL(pendingState.siteUrl) which can
throw for malformed siteUrl; update validation in isValidStoredState to verify
pendingState.siteUrl is a well-formed URL (e.g., attempt new URL(...) inside
that validator or use a safe URL-regex) so invalid stored states are rejected
earlier, or add a small try-catch around new URL(pendingState.siteUrl) before
comparing hostnames to convert malformed values into the same
stale-state-clearing path (pendingCaptureStorage.setValue(null) and logger.info)
without bubbling to the outer error log; reference pendingState.siteUrl,
isValidStoredState, pendingCaptureStorage.setValue, and logger.info when making
the change.

Comment on lines +460 to +464
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

When restoring state, the code only checks if the hostname matches (line 460) but doesn't validate that the current page is suitable for showing the capture modal. If the user navigates from one page to another on the same domain where capture is disabled (e.g., due to settings changes or messaging sites), the modal will still be restored. Consider adding a check using shouldAutoCaptureForCurrentPage() before restoring the modal.

Copilot uses AI. Check for mistakes.

const ageMs = Date.now() - pendingState.timestamp;
if (ageMs > MAX_AGE_MS) {
await pendingCaptureStorage.setValue(null);
logger.info("Cleared expired pending capture state");
return null;
}

logger.info(
"Restoring pending capture modal from browser.storage.session (extension-isolated)",
{
fieldsCount: pendingState.capturedFields.length,
extensionIsolated: true,
ageSeconds: Math.floor(ageMs / 1000),
},
);

const manager = new CaptureMemoryManager();
manager.currentFields = pendingState.capturedFields;
manager.siteTitle = pendingState.siteTitle;
manager.siteDomain = pendingState.siteDomain;

await manager.show(ctx, pendingState.capturedFields, {
skipPersist: true,
});
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

If manager.show() fails (line 487), the error is caught internally by show() which doesn't re-throw, and the manager is still returned on line 490. This means restoreIfNeeded() returns a manager instance that failed to display the UI. The caller in index.ts assigns this manager to captureMemoryManager, but it's not in a usable state. Consider checking manager.isVisible after calling show() and returning null if the UI failed to display, or have show() re-throw errors during restoration.

Suggested change
});
});
if (!manager.isVisible) {
logger.error(
"CaptureMemoryManager.show did not display the UI during restoration; clearing pending state",
);
try {
await pendingCaptureStorage.setValue(null);
} catch (cleanupError) {
logger.debug("Error clearing corrupted state:", cleanupError);
}
return null;
}

Copilot uses AI. Check for mistakes.
return manager;
Comment on lines +482 to +490
Copy link

@coderabbitai coderabbitai bot Feb 10, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Redundant assignment of currentFields before show().

Line 483 assigns manager.currentFields, but show() (line 217) immediately overwrites it with the same value. The assignment is harmless but unnecessary.

Proposed cleanup 
       const manager = new CaptureMemoryManager();
-      manager.currentFields = pendingState.capturedFields;
       manager.siteTitle = pendingState.siteTitle;
       manager.siteDomain = pendingState.siteDomain;
🤖 Prompt for AI Agents 
In `@src/entrypoints/content/components/capture-memory-manager.tsx` around lines
482 - 490, The assignment to manager.currentFields is redundant because
CaptureMemoryManager.show() already sets currentFields from the passed
capturedFields; remove the line "manager.currentFields =
pendingState.capturedFields" and keep the instantiation and the call to
manager.show(ctx, pendingState.capturedFields, { skipPersist: true }); ensure
manager.siteTitle and manager.siteDomain assignments remain as they are.

} catch (error) {
logger.error("Failed to restore pending capture state:", error);
try {
await pendingCaptureStorage.setValue(null);
} catch (cleanupError) {
logger.debug("Error clearing corrupted state:", cleanupError);
}
return null;
}
}

private static isValidStoredState(data: unknown): data is StoredCaptureState {
if (typeof data !== "object" || data === null) return false;

const state = data as Record<string, unknown>;

return (
Array.isArray(state.capturedFields) &&
typeof state.siteUrl === "string" &&
typeof state.siteTitle === "string" &&
typeof state.siteDomain === "string" &&
typeof state.timestamp === "number" &&
state.capturedFields.length > 0 &&
state.capturedFields.every(
(f: unknown) =>
typeof f === "object" &&
f !== null &&
typeof (f as Record<string, unknown>).fieldOpid === "string" &&
typeof (f as Record<string, unknown>).formOpid === "string" &&
typeof (f as Record<string, unknown>).question === "string" &&
typeof (f as Record<string, unknown>).answer === "string" &&
typeof (f as Record<string, unknown>).timestamp === "number" &&
typeof (f as Record<string, unknown>).wasAIFilled === "boolean" &&
typeof (f as Record<string, unknown>).fieldMetadata === "object" &&
(f as Record<string, unknown>).fieldMetadata !== null,
)
Comment on lines +514 to +526
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The validation function doesn't check for optional fields in CapturedFieldData. According to the CapturedFieldData interface, fields like originalAIValue and aiConfidence are optional, and fieldMetadata has nested optional properties like placeholder. The validation should handle these optional fields gracefully rather than rejecting stored data that lacks them. Consider checking that these fields are either undefined or have the correct type when present.

Suggested change
state.capturedFields.every(
(f: unknown) =>
typeof f === "object" &&
f !== null &&
typeof (f as Record<string, unknown>).fieldOpid === "string" &&
typeof (f as Record<string, unknown>).formOpid === "string" &&
typeof (f as Record<string, unknown>).question === "string" &&
typeof (f as Record<string, unknown>).answer === "string" &&
typeof (f as Record<string, unknown>).timestamp === "number" &&
typeof (f as Record<string, unknown>).wasAIFilled === "boolean" &&
typeof (f as Record<string, unknown>).fieldMetadata === "object" &&
(f as Record<string, unknown>).fieldMetadata !== null,
)
state.capturedFields.every((f: unknown) => {
if (typeof f !== "object" || f === null) {
return false;
}
const field = f as Record<string, unknown>;
if (
typeof field.fieldOpid !== "string" ||
typeof field.formOpid !== "string" ||
typeof field.question !== "string" ||
typeof field.answer !== "string" ||
typeof field.timestamp !== "number" ||
typeof field.wasAIFilled !== "boolean"
) {
return false;
}
if (
typeof field.fieldMetadata !== "object" ||
field.fieldMetadata === null
) {
return false;
}
const fieldMetadata = field.fieldMetadata as Record<string, unknown>;
// Optional top-level field: originalAIValue (string when present)
if (
"originalAIValue" in field &&
field.originalAIValue !== undefined &&
typeof field.originalAIValue !== "string"
) {
return false;
}
// Optional top-level field: aiConfidence (number when present)
if (
"aiConfidence" in field &&
field.aiConfidence !== undefined &&
typeof field.aiConfidence !== "number"
) {
return false;
}
// Optional nested field: fieldMetadata.placeholder (string when present)
if (
"placeholder" in fieldMetadata &&
fieldMetadata.placeholder !== undefined &&
typeof fieldMetadata.placeholder !== "string"
) {
return false;
}
return true;
})

Copilot uses AI. Check for mistakes.
Comment on lines +514 to +526
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The validation function doesn't validate the nested fieldMetadata properties according to the CapturedFieldData interface. The interface specifies that fieldMetadata should have properties like type, purpose, labels (array), required (boolean), and optional placeholder. Consider adding validation for these specific properties to ensure the stored state matches the expected structure.

Suggested change
state.capturedFields.every(
(f: unknown) =>
typeof f === "object" &&
f !== null &&
typeof (f as Record<string, unknown>).fieldOpid === "string" &&
typeof (f as Record<string, unknown>).formOpid === "string" &&
typeof (f as Record<string, unknown>).question === "string" &&
typeof (f as Record<string, unknown>).answer === "string" &&
typeof (f as Record<string, unknown>).timestamp === "number" &&
typeof (f as Record<string, unknown>).wasAIFilled === "boolean" &&
typeof (f as Record<string, unknown>).fieldMetadata === "object" &&
(f as Record<string, unknown>).fieldMetadata !== null,
)
state.capturedFields.every((f: unknown) => {
if (typeof f !== "object" || f === null) {
return false;
}
const field = f as Record<string, unknown>;
if (
typeof field.fieldOpid !== "string" ||
typeof field.formOpid !== "string" ||
typeof field.question !== "string" ||
typeof field.answer !== "string" ||
typeof field.timestamp !== "number" ||
typeof field.wasAIFilled !== "boolean"
) {
return false;
}
const meta = field.fieldMetadata as Record<string, unknown> | null | undefined;
if (typeof meta !== "object" || meta === null) {
return false;
}
if (
typeof meta.type !== "string" ||
typeof meta.purpose !== "string" ||
!Array.isArray(meta.labels) ||
!meta.labels.every((label: unknown) => typeof label === "string") ||
typeof meta.required !== "boolean"
) {
return false;
}
if (
"placeholder" in meta &&
meta.placeholder !== undefined &&
typeof meta.placeholder !== "string"
) {
return false;
}
return true;
})

Copilot uses AI. Check for mistakes.
);
}
}
Loading
Loading