fix: keep sync store available during route changes by Astro-Han · Pull Request #101 · Astro-Han/pawwork

Astro-Han · 2026-04-21T13:23:51Z

Summary

Fix a renderer crash when switching session routes across workspaces by resolving the current sync child store from the consuming owner instead of caching it in the Sync provider owner. Add a regression test for the owner lifecycle invariant.

Why

Fixes #94. The crash stack pointed to sync.data -> current()[0] after a route change. The previous createMemo(() => globalSync.child(sdk.directory)) resolved and pinned the child store under the provider owner, so stale consumer computations could still read through a disposed provider during cross-directory session navigation.

Related Issue

Fixes #94

How To Verify

Ran before dependency cleanup in this worktree:

bun test src/context/sync-current-child.test.ts
bun test src/context/sync-current-child.test.ts src/context/sync-optimistic.test.ts src/context/global-sync/child-store.test.ts
bun --cwd packages/app typecheck
bun --cwd packages/app test:unit
bun --cwd packages/app build

Also ran a real Electron client-route E2E against the patched renderer: started from the /Users/yuhan/PawWork session, routed to /Users/yuhan/workspace/dev/pawwork sessions ses_25248bc9cffePSNl6PXk12tw86 and ses_2544c191effeu1Nw5UECHonKbl, and confirmed no Something went wrong page or Cannot read properties of undefined crash.

Screenshots or Recordings

Not applicable. No visible UI change.

Checklist

I ran the relevant verification steps
I tested visible changes manually when needed
I am targeting the dev branch

Summary by CodeRabbit

Tests
- Added comprehensive test coverage for sync-child lookup persistence across provider lifecycle changes.
Refactor
- Improved internal sync-child initialization mechanism for enhanced reliability and maintainability.

coderabbitai · 2026-04-21T13:24:08Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 704d3e16-4c8d-48b0-9843-dc0388174b6f

📥 Commits

Reviewing files that changed from the base of the PR and between 1a02ca9 and 4310484.

📒 Files selected for processing (2)

packages/app/src/context/sync-current-child.test.ts
packages/app/src/context/sync.tsx

📝 Walkthrough

Walkthrough

A new exported helper function createCurrentSyncChild is added to the sync context to memoize construction of a "current" child selector via callback functions, with comprehensive test coverage validating it remains usable after provider root disposal.

Changes

Cohort / File(s)	Summary
Sync Context Helper `packages/app/src/context/sync.tsx`	Added `createCurrentSyncChild` helper that accepts `directory()` and `child()` callbacks to decouple directory retrieval from child selector construction. Updated `Sync` context initialization to use this helper instead of directly creating a memo.
Helper Test Coverage `packages/app/src/context/sync-current-child.test.ts`	New test file validating `createCurrentSyncChild` behavior across provider lifecycle boundaries, ensuring the returned accessor remains callable and returns correct values after provider root disposal.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A helper born from callbacks true,
Survives the disposal, tried and new,
With memoized grace through sync's terrain,
It persists when stores slip away again!
🌿 Resilience in every child() refrain.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly describes the main fix: preventing the sync store from becoming unavailable during route changes, which matches the core code changes and objectives.
Description check	✅ Passed	The PR description covers all required sections: Summary, Why, Related Issue, How To Verify with specific commands and manual testing, and completed Checklist. It provides clear context for the fix.
Linked Issues check	✅ Passed	The PR directly fixes issue `#94` by implementing the solution—resolving the sync child from consuming owner instead of caching in provider owner—which prevents the TypeError crash when the sync store becomes unavailable during route changes.
Out of Scope Changes check	✅ Passed	All changes are in-scope: the new `createCurrentSyncChild` helper and test file directly address the issue of sync store availability during route changes, with no unrelated modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch codex/fix-sync-store-crash

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

gemini-code-assist

Code Review

This pull request introduces the createCurrentSyncChild utility function to handle child lookups within the sync context, replacing a previous createMemo implementation. Additionally, a new test suite has been added to verify that the child lookup remains functional even after the provider's owner is disposed. I have no feedback to provide.

fix: keep sync store available during route changes

4310484

Astro-Han added bug Something isn't working P1 High priority app Application behavior and product flows labels Apr 21, 2026

gemini-code-assist Bot reviewed Apr 21, 2026

View reviewed changes

Astro-Han merged commit 2ef022a into dev Apr 21, 2026
17 checks passed

Astro-Han deleted the codex/fix-sync-store-crash branch April 21, 2026 13:31

This was referenced Apr 21, 2026

[Bug] Session timeline can crash when a rendered reasoning part becomes stale #112

Closed

[Bug] Update notice shows raw release download link #124

Closed

coderabbitai Bot mentioned this pull request Apr 23, 2026

fix: preserve workspace state during session switches #203

Merged

7 tasks

This was referenced May 4, 2026

fix(app): keep session timeline stable across worktree exit #436

Merged

fix(app): add non-creating child-store peek for sidebar timestamps (closes #475) #552

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: keep sync store available during route changes#101

fix: keep sync store available during route changes#101
Astro-Han merged 1 commit into
devfrom
codex/fix-sync-store-crash

Astro-Han commented Apr 21, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 21, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Astro-Han commented Apr 21, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Related Issue

How To Verify

Screenshots or Recordings

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Astro-Han commented Apr 21, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 21, 2026 •

edited

Loading