refactor: eliminate code duplication across storage adapters, persistence strategies, and React provider by Copilot · Pull Request #104 · passiveintent/core

Copilot · 2026-04-21T10:22:29Z

Four meaningful duplications removed across @passiveintent/core and @passiveintent/react, plus 13 new tests filling coverage gaps exposed by the refactor (506 → 519).

Changes

`LocalStorageAdapter` → delegates to `BrowserStorageAdapter`

LocalStorageAdapter was reimplementing ~60 lines already in BrowserStorageAdapter: namespace field, nsKey(), SSR guards, and legacy key migration. It now holds an internal BrowserStorageAdapter instance and delegates all three methods:

load → getItem
save → setItem (wrapped in try/catch to preserve swallow-on-error contract)
delete → new removeItem added to BrowserStorageAdapter

Bonus fix: BrowserStorageAdapter.getItem's window guard was outside the try/catch. Accessing window.localStorage as a property can itself throw SecurityError in sandboxed iframes — the guard is now inside the try.

`isQuotaError` helper extracted (`persistence-strategies.ts`)

Identical 2-line quota detection was copy-pasted in SyncPersistStrategy.persist and AsyncPersistStrategy.handleAsyncWriteError. Extracted as a module-level isQuotaError(err: unknown): boolean.

`getConfidence` exported from `signal-engine.ts`

intent-engine.ts inlined sampleSize < 10 ? 'low' : sampleSize < 30 ? 'medium' : 'high' — identical to the private getConfidence in signal-engine.ts. Made it exported and imported in intent-engine.ts.

`buildMergedConfig` extracted in `provider.tsx`

The adapter-merge block was copy-pasted between the render-phase init path and the React Strict Mode effect-phase re-init path. Extracted as a module-level buildMergedConfig(configRef, adaptersRef) helper.

New tests (13 added)

Area	Tests added
`LocalStorageAdapter.delete()`	happy path, no-op in Node.js, SecurityError swallowed
Legacy key migration	migrates + removes old key; custom namespace skips migration (both `LocalStorageAdapter` and `BrowserStorageAdapter`)
`BrowserStorageAdapter.removeItem()`	namespaced removal, no-op when window absent
`BrowserStorageAdapter` namespace isolation	two adapters with different namespaces are fully independent
`isQuotaError` message branch	`err.message.includes('quota')` path in both Sync and Async strategies
`getConfidence` thresholds	`'low'` (sampleSize < 10) and `'medium'` (10 ≤ sampleSize < 30) via `trajectory_anomaly`

Agent-Logs-Url: https://github.com/passiveintent/core/sessions/b01972fe-1abc-4e16-a6c5-ea0640fe2ad2 Co-authored-by: purushpsm147 <19334242+purushpsm147@users.noreply.github.com>

…ace isolation, isQuotaError message branch, and getConfidence thresholds Agent-Logs-Url: https://github.com/passiveintent/core/sessions/9a9c8eeb-53e7-488b-b026-15c3406371ec Co-authored-by: purushpsm147 <19334242+purushpsm147@users.noreply.github.com>

Copilot

Pull request overview

Refactors duplicated logic across storage adapters, persistence strategies, the intent engine, and the React provider, with additional tests added to cover refactor-exposed gaps.

Changes:

Delegate LocalStorageAdapter behavior to BrowserStorageAdapter and add BrowserStorageAdapter.removeItem().
Extract shared helpers (isQuotaError, getConfidence, buildMergedConfig) to remove duplicated logic.
Add/extend tests for quota detection, confidence thresholds, storage key migration, and removal behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
packages/react/src/provider.tsx	Extracts adapter/config merge logic into `buildMergedConfig` to avoid duplication across init paths.
packages/core/src/plugins/web/LocalStorageAdapter.ts	Delegates persistence operations to `BrowserStorageAdapter` and updates documentation.
packages/core/src/adapters.ts	Hardens `getItem` window/localStorage access and adds `removeItem`.
packages/core/src/engine/persistence-strategies.ts	Extracts `isQuotaError` helper and reuses it in sync/async strategies.
packages/core/src/engine/signal-engine.ts	Exports `getConfidence` for reuse.
packages/core/src/engine/intent-engine.ts	Uses shared `getConfidence(sampleSize)` instead of inlined thresholds.
packages/core/tests/persist-strategies.test.mjs	Adds tests for quota detection via message text.
packages/core/tests/microkernel.test.mjs	Adds confidence-threshold tests and `LocalStorageAdapter.delete`/migration tests.
packages/core/tests/compatibility-matrix.test.mjs	Adds tests for `BrowserStorageAdapter.removeItem` and namespace/migration behaviors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-21T10:28:42Z

+   * propagating to the caller.  Higher-layer error handling (IntentEngine's
+   * `onError` callback) is the right place to observe these failures; the
+   * caller (`IntentEngine._persist()`) already wraps this call in its own
+   * try/catch and routes any thrown error through `onError`.


The updated JSDoc for save() says higher-layer error handling via IntentEngine._persist()/onError is the right place to observe quota/security failures, but this method intentionally swallows those errors in its own try/catch. As written, callers generally won’t observe these failures via onError because nothing is thrown. Please adjust the comment to reflect the actual behavior/contract (swallowed errors are not observable upstream).

Suggested change

* propagating to the caller. Higher-layer error handling (IntentEngine's

* `onError` callback) is the right place to observe these failures; the

* caller (`IntentEngine._persist()`) already wraps this call in its own

* try/catch and routes any thrown error through `onError`.

* propagating to the caller. As a result, these storage write failures

* are intentionally not observable upstream via `IntentEngine._persist()`

* or an `onError` callback, because nothing is rethrown from this method.

Copilot · 2026-04-21T10:28:43Z

+test('BrowserStorageAdapter: removeItem() removes a namespaced key from localStorage', () => {
+  const store = new Map();
+  globalThis.window = {
+    localStorage: {
+      getItem: (k) => store.get(k) ?? null,


This test mutates globalThis.window and then deletes it in finally, but it doesn’t preserve/restore any pre-existing globalThis.window. The test above in this file saves originalWindow and restores it, which avoids cross-test contamination when running under environments that already define window (e.g. jsdom). Consider using the same save/restore pattern here.

Copilot · 2026-04-21T10:28:43Z

+test('BrowserStorageAdapter: getItem() migrates legacy unprefixed key to namespaced key on first read', () => {
+  const store = new Map();
+  // Simulate an old SDK installation that wrote the key without a namespace prefix.
+  store.set('intent-key', 'legacy-value');
+  globalThis.window = {
+    localStorage: {
+      getItem: (k) => store.get(k) ?? null,


This test deletes globalThis.window on cleanup without restoring any prior value. For better test isolation (and consistency with the first test in this file), capture const originalWindow = globalThis.window and restore it in finally when it was defined.

Copilot · 2026-04-21T10:28:43Z

+test('BrowserStorageAdapter: getItem() does NOT migrate legacy key for custom namespaces', () => {
+  const store = new Map();
+  store.set('intent-key', 'legacy-value');
+  globalThis.window = {
+    localStorage: {
+      getItem: (k) => store.get(k) ?? null,


Test isolation: this test overwrites globalThis.window and then deletes it, which can break subsequent tests if window was already defined. Please preserve originalWindow and restore it in finally (as done in the earlier degradation test).

Copilot · 2026-04-21T10:28:44Z

+test('BrowserStorageAdapter: custom namespace prefixes keys independently from the default namespace', () => {
+  const store = new Map();
+  globalThis.window = {
+    localStorage: {
+      getItem: (k) => store.get(k) ?? null,


Same globalThis.window cleanup issue as earlier: this test deletes window rather than restoring any previous value. Saving/restoring originalWindow will keep the test file safe to run under jsdom-like environments.

Copilot · 2026-04-21T10:28:44Z

+  configRef: React.RefObject<IntentManagerConfig>,
+  adaptersRef: React.RefObject<
+    Partial<{ storage: StorageAdapter; timer: TimerAdapter; lifecycle: LifecycleAdapter }>


buildMergedConfig is typed with React.RefObject, but both refs here are created via useRef(...) and have non-null current values (configRef) and possibly undefined (adaptersRef). With strict TS settings this signature can cause type errors (e.g., configRef.current possibly null when spreading, and adaptersRef not assignable if its current can be undefined). Consider changing the helper to accept React.MutableRefObject<IntentManagerConfig> and React.MutableRefObject<Partial<...> | undefined> (or normalize adaptersRef to {}/null at creation) so the types match actual usage.

Suggested change

configRef: React.RefObject<IntentManagerConfig>,

adaptersRef: React.RefObject<

Partial<{ storage: StorageAdapter; timer: TimerAdapter; lifecycle: LifecycleAdapter }>

configRef: React.MutableRefObject<IntentManagerConfig>,

adaptersRef: React.MutableRefObject<

Partial<{ storage: StorageAdapter; timer: TimerAdapter; lifecycle: LifecycleAdapter }> | undefined

Copilot AI and others added 3 commits April 21, 2026 10:04

refactor: eliminate code duplications across core and react packages

11fd7a9

Agent-Logs-Url: https://github.com/passiveintent/core/sessions/b01972fe-1abc-4e16-a6c5-ea0640fe2ad2 Co-authored-by: purushpsm147 <19334242+purushpsm147@users.noreply.github.com>

docs: add JSDoc for BrowserStorageAdapter.removeItem

d37de36

Agent-Logs-Url: https://github.com/passiveintent/core/sessions/b01972fe-1abc-4e16-a6c5-ea0640fe2ad2 Co-authored-by: purushpsm147 <19334242+purushpsm147@users.noreply.github.com>

Copilot AI assigned Copilot and purushpsm147 Apr 21, 2026

Copilot created this pull request from a session on behalf of purushpsm147 April 21, 2026 10:22 View session

purushpsm147 marked this pull request as ready for review April 21, 2026 10:22

purushpsm147 self-requested a review as a code owner April 21, 2026 10:22

Copilot AI review requested due to automatic review settings April 21, 2026 10:22

Copilot started reviewing on behalf of purushpsm147 April 21, 2026 10:23 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: eliminate code duplication across storage adapters, persistence strategies, and React provider#104

refactor: eliminate code duplication across storage adapters, persistence strategies, and React provider#104
Copilot wants to merge 3 commits into
mainfrom
copilot/check-code-duplications

Copilot AI commented Apr 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Apr 21, 2026

Changes

LocalStorageAdapter → delegates to BrowserStorageAdapter

isQuotaError helper extracted (persistence-strategies.ts)

getConfidence exported from signal-engine.ts

buildMergedConfig extracted in provider.tsx

New tests (13 added)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`LocalStorageAdapter` → delegates to `BrowserStorageAdapter`

`isQuotaError` helper extracted (`persistence-strategies.ts`)

`getConfidence` exported from `signal-engine.ts`

`buildMergedConfig` extracted in `provider.tsx`