Merge pull request 'Resume the last garden on this device (#97)' (#108) from feat/resume-last-garden into main

Gadfly (2 models, correctness): the redirect effect checked live.isError
but the rendered "taking you to your gardens…" message read full.error —
which is the SEASON query when viewing a past year. A season-view 404 with
a healthy live garden would then show a redirect promise the effect never
fires, stranding the user.

Derive gardenGone once from the LIVE query (garden existence doesn't depend
on the season viewed) and use it for BOTH the bounce effect and the render
message, so the promise and the redirect can't disagree. Also removes the
duplicated not-found reasoning the maintainability finding flagged.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ

web/src/lib/api.ts

@@ -30,6 +30,11 @@ export class ApiError extends Error {
   get isUnauthorized(): boolean {
     return this.status === 401
   }
+
+  /** Resource is gone or masked (pansy returns 404 for no-access, not 403). */
+  get isNotFound(): boolean {
+    return this.status === 404
+  }
 }
 
 type ParamValue = string | number | boolean | undefined | null

web/src/lib/lastGarden.test.ts

@@ -0,0 +1,70 @@
+import { afterEach, beforeEach, describe, expect, it } from 'vitest'
+import { getLastGardenId, rememberLastGarden, forgetLastGarden } from './lastGarden'
+
+// The tests run in the node environment (no DOM), so stand up a minimal
+// in-memory localStorage rather than pull in jsdom for one thin module.
+function installStorage(impl?: Partial<Storage>) {
+  const store = new Map<string, string>()
+  const base: Storage = {
+    getItem: (k) => store.get(k) ?? null,
+    setItem: (k, v) => void store.set(k, String(v)),
+    removeItem: (k) => void store.delete(k),
+    clear: () => store.clear(),
+    key: (i) => [...store.keys()][i] ?? null,
+    get length() {
+      return store.size
+    },
+  }
+  ;(globalThis as { localStorage?: Storage }).localStorage = { ...base, ...impl }
+}
+
+beforeEach(() => installStorage())
+afterEach(() => {
+  delete (globalThis as { localStorage?: Storage }).localStorage
+})
+
+describe('lastGarden', () => {
+  it('round-trips a remembered garden id', () => {
+    expect(getLastGardenId()).toBeNull()
+    rememberLastGarden(42)
+    expect(getLastGardenId()).toBe(42)
+  })
+
+  it('rejects a non-positive or unparseable stored value', () => {
+    localStorage.setItem('pansy:last-garden', 'not-a-number')
+    expect(getLastGardenId()).toBeNull()
+    localStorage.setItem('pansy:last-garden', '0')
+    expect(getLastGardenId()).toBeNull()
+    localStorage.setItem('pansy:last-garden', '-3')
+    expect(getLastGardenId()).toBeNull()
+  })
+
+  it('forgets unconditionally with no argument', () => {
+    rememberLastGarden(7)
+    forgetLastGarden()
+    expect(getLastGardenId()).toBeNull()
+  })
+
+  it('forgets only when the stored id matches onlyIfEquals', () => {
+    rememberLastGarden(5)
+    // A 404 on a different (directly-linked) garden must not wipe the good resume.
+    forgetLastGarden(9)
+    expect(getLastGardenId()).toBe(5)
+    // A 404 on the stored garden itself does clear it.
+    forgetLastGarden(5)
+    expect(getLastGardenId()).toBeNull()
+  })
+
+  it('swallows storage failures instead of throwing', () => {
+    installStorage({
+      setItem: () => {
+        throw new Error('quota')
+      },
+      getItem: () => {
+        throw new Error('blocked')
+      },
+    })
+    expect(() => rememberLastGarden(1)).not.toThrow()
+    expect(getLastGardenId()).toBeNull() // getItem throwing → null, not a crash
+  })
+})

web/src/lib/lastGarden.ts

@@ -0,0 +1,46 @@
+// The last garden opened on THIS device, so a returning user resumes where they
+// were instead of always landing on the gardens list. Per-device, in
+// localStorage (same rationale as the seed tray and PlantPicker recents): a
+// convenience, not authoritative state — a quota/availability failure is
+// swallowed, and a stored id that no longer loads clears itself (see the editor).
+//
+// Only the id is stored. The garden is resolved by the editor on load; if it's
+// gone (deleted, or access revoked), forgetLastGarden() drops it so `/` stops
+// resuming a garden that can't open.
+
+const KEY = 'pansy:last-garden'
+
+/** The last-opened garden id on this device, or null if none/unparseable. */
+export function getLastGardenId(): number | null {
+  try {
+    const raw = localStorage.getItem(KEY)
+    if (raw == null) return null
+    const n = Number(raw)
+    return Number.isInteger(n) && n > 0 ? n : null
+  } catch {
+    return null
+  }
+}
+
+/** Record the garden the device is now in, so `/` resumes here next time. */
+export function rememberLastGarden(id: number): void {
+  try {
+    localStorage.setItem(KEY, String(id))
+  } catch {
+    // Resume is a convenience; ignore quota/availability failures.
+  }
+}
+
+/**
+ * Forget the stored last garden. With `onlyIfEquals`, clears only when the stored
+ * id matches — so a 404 on a directly-linked garden can't wipe a different, still
+ * good resume target the user had.
+ */
+export function forgetLastGarden(onlyIfEquals?: number): void {
+  try {
+    if (onlyIfEquals != null && getLastGardenId() !== onlyIfEquals) return
+    localStorage.removeItem(KEY)
+  } catch {
+    // ignore
+  }
+}

web/src/pages/GardenEditorPage.tsx

@@ -36,6 +36,8 @@ import { useJournalCounts } from '@/lib/journal'
 import { attributableLots, lotsByPlant, useSeedLots, type SeedLot } from '@/lib/seedLots'
 import { useSeedTray } from '@/lib/seedTray'
 import { usePageTitle } from '@/lib/usePageTitle'
+import { rememberLastGarden, forgetLastGarden } from '@/lib/lastGarden'
+import { ApiError } from '@/lib/api'
 
 const routeApi = getRouteApi('/gardens/$gardenId')
 
@@ -55,6 +57,14 @@ export function GardenEditorPage() {
   const me = useMe()
   usePageTitle(full.data?.garden.name ?? 'Garden')
 
+  // The garden itself is gone (deleted, or access revoked) — keyed on the LIVE
+  // query, since whether the garden EXISTS doesn't depend on the season being
+  // viewed. Both the bounce effect and the "gone" render message read this one
+  // value, so the promise ("taking you to your gardens…") and the redirect can't
+  // disagree — e.g. a season-view error while the live garden is fine must not
+  // claim a redirect that never fires.
+  const gardenGone = live.error instanceof ApiError && live.error.isNotFound
+
   const selectedId = useEditorStore((s) => s.selectedId)
   const select = useEditorStore((s) => s.select)
   const selectedPlantingId = useEditorStore((s) => s.selectedPlantingId)
@@ -133,6 +143,24 @@ export function GardenEditorPage() {
     navigate({ search: (prev) => ({ ...prev, focus: focusedObjectId ?? undefined }), replace: true })
   }, [focusedObjectId, navigate])
 
+  // Remember this garden as the device's last-opened, so `/` resumes here next
+  // visit. Keyed on the live query: which garden EXISTS doesn't depend on the
+  // season being viewed.
+  useEffect(() => {
+    if (live.isSuccess) rememberLastGarden(gid)
+  }, [live.isSuccess, gid])
+
+  // A last garden that 404s (deleted, or access revoked) must not strand the user
+  // on an error screen the `/` resume keeps returning to: forget it (only if it's
+  // this one, so a bad direct link can't wipe a good resume target) and bounce to
+  // the list. Transient errors (500/network) fall through to the retryable screen.
+  useEffect(() => {
+    if (gardenGone) {
+      forgetLastGarden(gid)
+      navigate({ to: '/gardens' })
+    }
+  }, [gardenGone, gid, navigate])
+
   // If the focused object vanished — deleted, or a stale ?focus id on load — leave
   // focus mode so the canvas isn't stuck dimmed with no way out.
   useEffect(() => {
@@ -256,12 +284,20 @@ export function GardenEditorPage() {
   }, [])
 
   if (full.isPending) return <p className="p-6 text-sm text-muted">Loading garden…</p>
-  if (full.isError)
+  if (full.isError) {
+    // Only promise the bounce when the GARDEN is gone (the effect above keys on
+    // the same gardenGone). A season-view error while the live garden is fine is a
+    // different, non-redirecting failure and gets the generic message.
     return (
       <div className="p-6">
-        <Alert>Could not load this garden.</Alert>
+        <Alert>
+          {gardenGone
+            ? 'That garden is no longer available — taking you to your gardens…'
+            : 'Could not load this garden.'}
+        </Alert>
       </div>
     )
+  }
 
   const g = full.data.garden
   const garden: EditorGarden = {

web/src/router.tsx

@@ -18,6 +18,7 @@ import { SettingsPage } from '@/pages/SettingsPage'
 import { meQueryOptions } from '@/lib/auth'
 import { queryClient } from '@/lib/queryClient'
 import { safeRedirectPath } from '@/lib/redirect'
+import { getLastGardenId } from '@/lib/lastGarden'
 
 interface RouterContext {
   queryClient: QueryClient
@@ -66,7 +67,15 @@ async function requireAdmin(context: RouterContext, path: string) {
 const indexRoute = createRoute({
   getParentRoute: () => rootRoute,
   path: '/',
+  // Resume the garden this device was last in, so a returning user lands back in
+  // their plot rather than on the list every time. A stored id that no longer
+  // loads is cleared by the editor and bounces here to /gardens on the next
+  // visit, so this can't trap anyone on a dead garden.
   beforeLoad: () => {
+    const last = getLastGardenId()
+    if (last != null) {
+      throw redirect({ to: '/gardens/$gardenId', params: { gardenId: String(last) } })
+    }
     throw redirect({ to: '/gardens' })
   },
 })