Skip to content

fix(web): guard terminal persistence when storage is unavailable#1021

Open
kfiramar wants to merge 1 commit intopingdotgg:mainfrom
kfiramar:fix/web-terminal-storage-fallback
Open

fix(web): guard terminal persistence when storage is unavailable#1021
kfiramar wants to merge 1 commit intopingdotgg:mainfrom
kfiramar:fix/web-terminal-storage-fallback

Conversation

@kfiramar
Copy link

@kfiramar kfiramar commented Mar 13, 2026

Summary

  • guard the terminal Zustand persistence layer so it only uses localStorage when the runtime exposes the full storage API and otherwise falls back to a no-op storage backend
  • stabilize terminalStateStore tests with an in-memory storage double so the suite exercises terminal behavior instead of runner-specific storage quirks
  • document the root cause and verification details here so the fix stays focused without broad dependency churn

Problem

On my local repro, the repo mostly validated cleanly after a reinstall, but apps/web/src/terminalStateStore.test.ts consistently failed under the current Bun/Vitest environment.

All 8 failures collapsed to the same persistence crash:

TypeError: storage.setItem is not a function

The issue was not the terminal state logic itself. The problem was that the persisted Zustand store assumed localStorage was a complete Storage implementation whenever it existed. In this runtime, localStorage was present enough to be detected, but not complete enough for Zustand persistence to safely call setItem.

Fix Details

  • add getTerminalStateStorage() in apps/web/src/terminalStateStore.ts to validate the runtime storage shape before passing it into createJSONStorage
  • return a no-op storage implementation when the runtime storage is missing or incomplete, which preserves app behavior while avoiding persistence crashes in unsupported environments
  • replace the test's global storage assumptions with a deterministic in-memory storage mock in apps/web/src/terminalStateStore.test.ts

This keeps the fix intentionally narrow:

  • no product behavior changes for normal browser environments
  • no dependency upgrades or lockfile churn
  • no unrelated web/store refactors

Why this approach

I considered fixing this only in the test, but the crash originates inside the persisted store configuration itself. Making the store resilient is the more robust fix because it protects both tests and any non-browser or partially browser-like runtime edge cases.

I also avoided a broader dependency update pass here. The repo is already on a modern stack, and the failure was reproducible as a runtime compatibility issue rather than an outdated-package issue.

Verification

I ran the focused suite first and then the full required repo checks:

bun x vitest run apps/web/src/terminalStateStore.test.ts
bun run fmt:check
bun lint
bun run typecheck
bun run test

All of the commands above pass with this change.

Notes

  • I did a full reinstall during debugging, but I reverted the generated mockServiceWorker diff so this PR only contains the targeted fix.
  • The local machine that reproduced this had toolchain skew from the repo pins (.mise.toml requests Bun 1.3.9 and Node 24.13.1), which made it even more valuable to keep the fix defensive.

Note

Guard terminal state persistence when localStorage is unavailable

The useTerminalStateStore in terminalStateStore.ts previously called localStorage directly in the persist middleware, causing errors when storage is absent or incomplete. A getTerminalStateStorage utility now validates that localStorage exposes the expected methods at runtime, falling back to a no-op Storage implementation if not. In valid environments, behavior is unchanged.

Macroscope summarized aa7dbd7.

@coderabbitai
Copy link

coderabbitai bot commented Mar 13, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: dcacb2ae-6b7b-47e5-af6d-f0c3b57fcead

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

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.

Tip

CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.

OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required.

@github-actions github-actions bot added size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Mar 13, 2026

const TERMINAL_STATE_STORAGE_KEY = "t3code:terminal-state:v1";

const noopStorage: Storage = {
Copy link
Member

Choose a reason for hiding this comment

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

feels like we have a lot of these mocks now in a lot of different test files. should unify it to one central MemoryStorage that we can use as runtmie fallback and also in all tests

you up for doing this? i know it will increase the scope of the PR a bit but for maintainability would be nice to do.

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

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants