Skip to content

Restrict draft recovery to 8-hour window#15

Merged
aidmax merged 3 commits intomainfrom
claude/restrict-draft-recovery-time-MON5b
Apr 11, 2026
Merged

Restrict draft recovery to 8-hour window#15
aidmax merged 3 commits intomainfrom
claude/restrict-draft-recovery-time-MON5b

Conversation

@aidmax
Copy link
Copy Markdown
Owner

@aidmax aidmax commented Apr 11, 2026

Drafts older than maxAgeMs (default 8 hours) are silently discarded on restore and removed from localStorage, preventing stale drafts from being recovered the next day.

Storage format updated to { data, savedAt } to track draft age. Old-format drafts (plain JSON values without savedAt) are also discarded and cleaned up on first load.

https://claude.ai/code/session_01DroMmmZbjEwG2WV4N2QM7S

claude added 2 commits April 11, 2026 06:39
Drafts older than maxAgeMs (default 24h) are silently discarded on
restore and removed from localStorage, preventing stale drafts from
being recovered the next day.

Storage format updated to { data, savedAt } to track draft age.

https://claude.ai/code/session_01DroMmmZbjEwG2WV4N2QM7S
@aidmax aidmax changed the title Restrict draft recovery to 24-hour window Restrict draft recovery to 8-hour window Apr 11, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 11, 2026

Greptile Summary

This PR restricts draft recovery to an 8-hour window by wrapping persisted values in a { data, savedAt } envelope and discarding entries older than maxAgeMs on restore. A type guard added in the latest commit also correctly disposes of old-format drafts (plain JSON, no savedAt) without triggering a spurious wasRestored state, addressing the previous review concern.

Confidence Score: 5/5

Safe to merge — logic is correct, backward compatibility is handled, and all new paths are covered by tests

Both issues from prior review rounds are fully resolved: the type guard prevents old-format drafts from setting wasRestored, and the age check correctly expires stale drafts. No P0/P1 findings remain.

No files require special attention

Important Files Changed

Filename Overview
client/src/hooks/use-form-persistence.ts Adds { data, savedAt } envelope and 8-hour expiry; type guard correctly handles old-format drafts and malformed values before the age check
client/src/test/hooks/use-form-persistence.test.ts Tests updated to use new { data, savedAt } format; new cases cover expired drafts, custom maxAgeMs, and old-format discard

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([Mount]) --> B[localStorage.getItem key]
    B --> C{saved exists?}
    C -- No --> Z([done, wasRestored=false])
    C -- Yes --> D[JSON.parse saved]
    D --> E{Parse error?}
    E -- Yes --> F[console.error]
    F --> Z
    E -- No --> G{Type guard:\nobject, savedAt:number,\ndata truthy?}
    G -- Fails --> H[removeItem key]
    H --> Z
    G -- Passes --> I{age = Date.now - savedAt\nage > maxAgeMs?}
    I -- Yes --> J[removeItem key]
    J --> Z
    I -- No --> K[form.reset draft.data]
    K --> L([wasRestored=true])
Loading

Reviews (2): Last reviewed commit: "Guard against old-format drafts missing ..." | Re-trigger Greptile

import { useEffect, useRef, useState } from "react";
import { FieldValues, UseFormReturn } from "react-hook-form";

const DEFAULT_MAX_AGE_MS = 8 * 60 * 60 * 1000; // 8 hours
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Default expiry is 8 h, but PR says 24 h

DEFAULT_MAX_AGE_MS resolves to 28,800,000 ms (8 hours), yet the PR title is "Restrict draft recovery to 24-hour window" and the description states "default 24h". If the intended ceiling really is 24 hours, the constant should be 24 * 60 * 60 * 1000; if 8 hours was intentional, the PR title and description are misleading.

Suggested change
const DEFAULT_MAX_AGE_MS = 8 * 60 * 60 * 1000; // 8 hours
const DEFAULT_MAX_AGE_MS = 24 * 60 * 60 * 1000; // 24 hours

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

8 hours is intentional — the user explicitly requested it after the initial 24h implementation. PR title and description have been updated to reflect the correct 8-hour default.


Generated by Claude Code

Comment on lines 32 to 39
const parsed: PersistedDraft<T> = JSON.parse(saved);
const age = Date.now() - parsed.savedAt;
if (age > maxAgeMs) {
localStorage.removeItem(key);
return;
}
form.reset(parsed.data);
setWasRestored(true);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Old-format drafts silently corrupt wasRestored state

Any draft saved by the previous version of the hook (plain JSON.stringify(values), no savedAt key) will reach this path with parsed.savedAt === undefined. Date.now() - undefined evaluates to NaN, and NaN > maxAgeMs is always false, so the age guard never fires. Execution continues to form.reset(parsed.data) where parsed.data is also undefined — React Hook Form resets the form to its defaultValues. Then setWasRestored(true) is called, triggering the "Draft restored" toast in home.tsx even though the form shows default values. The stale old-format key is never removed from localStorage, so this misfires on every subsequent page load.

Add a type guard before the age check to discard and remove the old format:

if (
  typeof parsed !== "object" ||
  parsed === null ||
  typeof parsed.savedAt !== "number" ||
  !parsed.data
) {
  localStorage.removeItem(key);
  return;
}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good catch. Fixed in e250bbe — a type guard now validates the stored value has a numeric savedAt and a data field before proceeding. Anything missing either is removed from localStorage and skipped, so old-format drafts no longer trigger a false wasRestored. A dedicated test case covers this path.


Generated by Claude Code

Drafts written by the previous hook version (plain JSON values, no
savedAt) would pass the age check (NaN > maxAgeMs is false), reset
the form to undefined data, and incorrectly set wasRestored=true.

Now any stored value that lacks a numeric savedAt or a data field is
treated as invalid: it is removed from localStorage and recovery is
skipped.

https://claude.ai/code/session_01DroMmmZbjEwG2WV4N2QM7S
@aidmax
Copy link
Copy Markdown
Owner Author

aidmax commented Apr 11, 2026

All done. Here's a summary of what was addressed:
Comment 1 — PR title mismatch: The 8h default is intentional (user requested it). Updated the PR title and description to correctly say "8-hour window".
Comment 2 — Old-format draft bug: This was a real bug. Added a type guard before the age check — if the stored value is missing a numeric savedAt or a data field, it's removed from localStorage and recovery is skipped. This prevents old-format drafts from triggering a false wasRestored toast. A new test ("discards old-format draft (no savedAt) and does not set wasRestored") covers the path.

@greptile-apps

@aidmax aidmax merged commit 2e9a321 into main Apr 11, 2026
2 checks passed
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.

2 participants