diff --git a/skills/subagents/SKILL.md b/skills/subagents/SKILL.md new file mode 100644 index 0000000..2f2df6f --- /dev/null +++ b/skills/subagents/SKILL.md @@ -0,0 +1,84 @@ +--- +name: subagents +description: Launching and managing coding subagents via tmux. Use when delegating tasks to parallel agents, monitoring their progress, or coordinating multi-agent work. Triggers include "launch an agent", "start a subagent", "delegate this task", "run this in parallel", or any task coordination involving multiple agents. +--- + +# Subagent Management + +Run coding subagents in tmux sessions so both the orchestrator and the human have visibility. The orchestrator (you) acts as project manager — writing tasks, launching agents, monitoring progress, and validating results. You don't code directly. + +## Launching a subagent + +```bash +tmux new-session -d -s -x 220 -y 50 \ + "pi --model '' 2>&1; echo '[AGENT DONE]'; sleep 99999" +``` + +- **Session name**: short, descriptive (e.g. `agent1`, `refactor-data`, `fix-tests`). +- **Prompt**: tell the agent what task file to read, what guidelines to follow (e.g. `AGENTS.md`), and what the success criteria are. +- The `echo '[AGENT DONE]'; sleep 99999` tail keeps the session alive after the agent finishes so you can review its final output. + +## Monitoring progress + +```bash +# Peek at the last N lines of output +tmux capture-pane -t -p | tail -40 + +# The human can watch live +tmux attach -t +``` + +Check in periodically. Don't just launch and forget — catch issues early. + +## Key rules + +### Never kill a working agent + +An agent accumulates deep context over many minutes of reading, reasoning, and coding. Killing it mid-task destroys all of that. A new agent starting from scratch will: + +- Waste time re-reading everything +- Miss implicit decisions the previous agent made +- Likely produce worse or inconsistent results + +**If you need to do something in the repo while an agent is running** (create a branch, install a dep, check types), do it in your own terminal. The filesystem is shared — you can work alongside the agent without disrupting it. + +### One task per agent + +Each agent gets a single task file from `tasks/`. Don't overload an agent with multiple unrelated goals. If a task turns out to be bigger than expected, split it. + +### Give agents the right starting context + +A good launch prompt includes: + +1. Which task file to read +2. Which project guidelines to follow (e.g. `AGENTS.md`) +3. Orientation on where to start in the codebase +4. What "done" looks like + +A bad launch prompt is vague ("fix the app") or over-specified with implementation details (let the agent figure out the how). + +### Parallel agents + +Multiple agents can work simultaneously on independent tasks. Use distinct tmux session names and make sure their tasks touch different files to avoid conflicts. + +If tasks are sequential (agent B depends on agent A's output), wait for A to finish and validate before launching B. + +### Validation + +When an agent signals it's done (or you see `[AGENT DONE]` in the session): + +1. Check the output: `tmux capture-pane -t -p | tail -80` +2. Run the validation criteria from the task file (typically `npm run check`, `npm test`, manual grep checks) +3. Review the diff: `git diff` +4. If it passes, update the task status to `done` +5. If it fails, either relaunch with specific fix instructions or fix manually + +### Cleanup + +```bash +# Kill a finished session +tmux kill-session -t + +# List all sessions +tmux list-sessions +``` diff --git a/src/App.tsx b/src/App.tsx index c98fa02..f1f73ae 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,21 +1,55 @@ -import React, { useCallback, useEffect, useState } from 'react'; -import { useRoute } from './ui/routing'; +// Top-level application component. Instantiates the data layer once and +// provides a thin routing adapter that syncs URL ↔ data state. +import React, { useEffect, useRef } from 'react'; +import { useRoute, parseRoute } from './ui/routing'; import { RepoView } from './ui/RepoView'; import { HomeView } from './ui/HomeView'; -import { listRecentRepos, recordRecentRepo, type RecentRepo } from './storage/local'; +import { useAppData } from './data'; +import { listRecentRepos } from './storage/local'; export function App() { + // Data layer: instantiated once at the app level. + // The initial route is read from the URL synchronously so the first render + // already reflects the correct slug/path without waiting for a dispatch. + const { state, dispatch } = useAppData(parseRoute(window.location.pathname)); + + // URL routing: parse/navigate the browser history. const { route, navigate } = useRoute(); - // Adjust page title based on route + // URL → data: inform the data layer whenever the browser route changes. useEffect(() => { - document.title = route.kind === 'repo' ? `${route.owner}/${route.repo}` : 'VibeNote'; + dispatch({ type: 'route-changed', route }); }, [route]); - // redirects + // Data → URL: when the data layer sets a pendingNavigation (e.g. after a + // rename or sync), reflect it in the URL via navigate(). + let lastNavRef = useRef(state.pendingNavigation); + useEffect(() => { + let nav = state.pendingNavigation; + // Skip if no pending nav, or if we already processed this exact object. + if (!nav || nav === lastNavRef.current) return; + lastNavRef.current = nav; + + // Build the target route from the current active route + new path. + let activeRoute = state.activeRoute; + if (activeRoute.kind === 'repo') { + navigate({ ...activeRoute, notePath: nav.path }, { replace: nav.replace }); + } else if (activeRoute.kind === 'new') { + navigate({ kind: 'new', notePath: nav.path }, { replace: nav.replace }); + } + }, [state.pendingNavigation]); + + // Adjust page title based on active route. + useEffect(() => { + let r = state.activeRoute; + document.title = r.kind === 'repo' ? `${r.owner}/${r.repo}` : 'VibeNote'; + }, [state.activeRoute]); + + // Redirects based on the URL route (not the data layer route). useEffect(() => { // if the route is /start, redirect to the most recent repo or /home if (route.kind === 'start') { + let recents = state.recents; let candidate = recents.find((entry) => entry.owner !== undefined && entry.repo !== undefined); if (candidate !== undefined) { @@ -33,12 +67,8 @@ export function App() { } }, [route]); - // list of recent repos, kept in local storage and updated when navigating to a new repo - // or updating information about an existing one - const [recents, recordRecent] = useRecents(); - if (route.kind === 'home') { - return ; + return ; } if (route.kind === 'start') { @@ -46,39 +76,9 @@ export function App() { return null; } - if (route.kind === 'new') { - return ; - } - - if (route.kind === 'repo') { - return ( - - ); + if (route.kind === 'new' || route.kind === 'repo') { + return ; } return null; } - -function useRecents() { - const [recents, setRecents] = useState(() => listRecentRepos()); - - useEffect(() => { - const onStorage = () => setRecents(listRecentRepos()); - window.addEventListener('storage', onStorage); - return () => window.removeEventListener('storage', onStorage); - }, []); - - const recordRecent = useCallback( - (entry: { slug: string; owner?: string; repo?: string; title?: string; connected?: boolean }) => { - recordRecentRepo(entry); - setRecents(listRecentRepos()); - }, - [] - ); - return [recents, recordRecent] as const; -} diff --git a/src/data.ts b/src/data.ts index c00b56e..e671aa6 100644 --- a/src/data.ts +++ b/src/data.ts @@ -1,4 +1,5 @@ -// Data-layer hook that orchestrates repo auth, storage, and sync state for RepoView. +// Data-layer hook that orchestrates repo auth, storage, and sync state for the app. +// Exported as a single app-level hook (useAppData) with a typed dispatch interface. import { useMemo, useState, useEffect, useRef, useSyncExternalStore, useCallback } from 'react'; import { isRepoLinked, @@ -15,6 +16,9 @@ import { getRepoStore, computeSyncedHash, extractDir, + listRecentRepos, + recordRecentRepo, + type RecentRepo, } from './storage/local'; import { signInWithGitHubApp, @@ -50,14 +54,16 @@ import { useReadOnlyFiles } from './data/useReadOnlyFiles'; import { normalizePath } from './lib/util'; import { prepareClipboardImage } from './lib/image-processing'; import { relativePathBetween, COMMON_ASSET_DIR } from './lib/pathing'; -import type { RepoRoute } from './ui/routing'; +import type { Route, RepoRoute } from './ui/routing'; -export { useRepoData }; +export { useAppData }; export type { + Action, + Dispatch, RepoAccessState, - RepoDataInputs, - RepoDataState, - RepoDataActions, + AppDataState, + // Keep RepoDataState as an alias for backward compat with existing type references + AppDataState as RepoDataState, ShareState, RepoAccessErrorType, ImportedAsset, @@ -76,7 +82,44 @@ type ShareState = { // Compact set of error outcomes we surface to the UI for repo access. type RepoAccessErrorType = 'auth' | 'not-found' | 'forbidden' | 'network' | 'rate-limited' | 'unknown'; -type RepoDataState = { +// Discriminated union of all actions the UI can dispatch to the data layer. +type Action = + | { type: 'route-changed'; route: Route } + | { type: 'sign-in' } + | { type: 'sign-out' } + | { type: 'open-repo-access' } + | { type: 'sync-now' } + | { type: 'set-autosync'; enabled: boolean } + | { type: 'select-file'; path: string | undefined } + | { type: 'create-note'; dir: string; name: string } + | { type: 'create-folder'; parentDir: string; name: string } + | { type: 'rename-file'; path: string; name: string } + | { type: 'move-file'; path: string; targetDir: string } + | { type: 'delete-file'; path: string } + | { type: 'rename-folder'; dir: string; newName: string } + | { type: 'move-folder'; dir: string; targetDir: string } + | { type: 'delete-folder'; dir: string } + | { type: 'save-file'; path: string; text: string } + | { type: 'import-pasted-assets'; notePath: string; files: File[] } + | { type: 'create-share-link' } + | { type: 'refresh-share-link' } + | { type: 'revoke-share-link' }; + +// Typed dispatch: returns specific values for actions that produce results. +// Most actions return void or Promise; the overloads cover exceptions. +type Dispatch = { + (action: { type: 'create-note'; dir: string; name: string }): string | undefined; + (action: { type: 'move-file'; path: string; targetDir: string }): string | undefined; + (action: { type: 'move-folder'; dir: string; targetDir: string }): string | undefined; + (action: { type: 'import-pasted-assets'; notePath: string; files: File[] }): Promise; + (action: Action): void | Promise; +}; + +// Pending navigation driven by the data layer (e.g. after rename or sync). +// The routing adapter in App.tsx picks this up and calls navigate(). +type PendingNavigation = { path: string | undefined; replace: boolean }; + +type AppDataState = { // session state hasSession: boolean; user: AppUser | undefined; @@ -106,59 +149,54 @@ type RepoDataState = { // general info statusMessage: string | undefined; -}; -type RepoDataActions = { - // auth actions - signIn: () => Promise; - signOut: () => Promise; - openRepoAccess: () => Promise; - - // syncing actions - syncNow: () => Promise; - setAutosync: (enabled: boolean) => void; - - // edit notes/folders - selectFile: (path: string | undefined) => Promise; - createNote: (dir: string, name: string) => string | undefined; - createFolder: (parentDir: string, name: string) => void; - renameFile: (path: string, name: string) => void; - moveFile: (path: string, targetDir: string) => string | undefined; - deleteFile: (path: string) => void; - renameFolder: (dir: string, newName: string) => void; - moveFolder: (dir: string, targetDir: string) => string | undefined; - deleteFolder: (dir: string) => void; - saveFile: (path: string, text: string) => void; - importPastedAssets: (params: { notePath: string; files: File[] }) => Promise; - createShareLink: () => Promise; - refreshShareLink: () => Promise; - revokeShareLink: () => Promise; + // routing info (new in app-level hook) + activeSlug: string; + activeRoute: Route; + + // Pending URL navigation from the data layer. The routing adapter should + // call navigate() with this and then dispatch route-changed when done. + pendingNavigation: PendingNavigation | undefined; + + // list of recently opened repos (for HomeView) + recents: RecentRepo[]; }; + type ImportedAsset = { assetPath: string; markdownPath: string; altText: string; }; -type RepoDataInputs = { - slug: string; - route: RepoRoute; - recordRecent: (entry: { slug: string; owner?: string; repo?: string; connected?: boolean }) => void; - setActivePath: (notePath: string | undefined, options?: { replace?: boolean }) => void; -}; - /** - * Data layer entry point. + * App-level data hook. Instantiate once in App.tsx. * - * Invariants when calling this hook: - * - `slug` and `route` are always in sync, and never change througout the component lifetime - * - `recordRecent` and `setActivePath` are stable as well - * - none of these will be put in dependency arrays + * Route changes flow IN via dispatch({ type: 'route-changed', route }). + * The data layer expresses navigation intent via state.pendingNavigation. + * A thin adapter in App.tsx handles URL↔state sync. */ -function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInputs): { - state: RepoDataState; - actions: RepoDataActions; +function useAppData(initialRoute: Route): { + state: AppDataState; + dispatch: Dispatch; } { + // Internal route state — the data layer's canonical view of "where we are". + // Updated via route-changed dispatch (URL→data) and navigateInternal (data→URL). + let [internalRoute, setInternalRoute] = useState(initialRoute); + + // Pending navigation for the routing adapter to pick up and call navigate(). + let [pendingNavigation, setPendingNavigation] = useState(undefined); + + // Recents list (synced to localStorage). + let [recents, setRecents] = useState(() => listRecentRepos()); + + // Derive slug and the repo-level route from the internal route. + let slug = + internalRoute.kind === 'repo' ? `${internalRoute.owner}/${internalRoute.repo}` : 'new'; + let repoRoute: RepoRoute = + internalRoute.kind === 'repo' || internalRoute.kind === 'new' + ? internalRoute + : { kind: 'new' }; + // ORIGINAL STATE AND MAIN HOOKS // Local storage wrapper let { files: localFiles, folders: localFolders } = useLocalRepoSnapshot(slug); @@ -180,18 +218,28 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput // Keep the signed-in GitHub App user details for header UI. let [user, setUser] = useState(() => getAppSessionUser() ?? undefined); + // --- Per-slug state reset (synchronous, runs before render completes) --- + // When the slug changes, reset per-slug state to avoid leaking state between repos. + let [prevSlugForReset, setPrevSlugForReset] = useState(slug); + if (prevSlugForReset !== slug) { + setPrevSlugForReset(slug); + setLinked(isRepoLinked(slug)); + setShareState({ status: 'idle' }); + setStatusMessage(undefined); + } + // Query GitHub for repo access state and other metadata. - let repoAccess = useRepoAccess({ route, sessionToken }); + let repoAccess = useRepoAccess({ route: repoRoute, sessionToken }); // DERIVED STATE (and hooks that depend on it) let { defaultBranch, manageUrl } = repoAccess; - let repoOwner = route.kind === 'repo' ? route.owner : undefined; - let repoName = route.kind === 'repo' ? route.repo : undefined; + let repoOwner = repoRoute.kind === 'repo' ? repoRoute.owner : undefined; + let repoName = repoRoute.kind === 'repo' ? repoRoute.repo : undefined; let accessStatusReady = repoAccess.status === 'ready' || repoAccess.status === 'error'; let accessStatusUnknown = !accessStatusReady || repoAccess.errorType === 'network'; - let desiredPath = normalizePath(route.notePath); + let desiredPath = normalizePath(repoRoute.notePath); // in readonly mode, we store nothing locally and just fetch content from github no demand let isReadOnly = repoAccess.level === 'read'; @@ -207,9 +255,9 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput // note that we are optimistic about write access until the access check completes, // to avoid flickering the UI when revisiting a known writable repo let canEdit = - route.kind === 'new' || (hasSession && (repoAccess.level === 'write' || (accessStatusUnknown && linked))); + repoRoute.kind === 'new' || (hasSession && (repoAccess.level === 'write' || (accessStatusUnknown && linked))); - let canSync = canEdit && route.kind === 'repo' && linked; + let canSync = canEdit && repoRoute.kind === 'repo' && linked; let { autosync, syncing, setAutosync, scheduleAutoSync, performSync } = useSync({ slug, @@ -268,24 +316,30 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput if (currentPath === undefined) return; if (currentPath === prevPath) return; if (pathsEqual(desiredPath, currentPath)) return; - setActivePath(currentPath, { replace: true }); + navigateInternal(currentPath, { replace: true }); }, [activeFile?.path, desiredPath]); // Remember recently opened repos once we know the current repo is reachable. // TODO this shouldn't a useEffect, the only place a repo ever becomes reachable is after // fetching metadata, so just record it there useEffect(() => { - if (route.kind !== 'repo') return; + if (repoRoute.kind !== 'repo') return; if (repoAccess.level === 'none') return; - recordRecent({ + recordRecentRepo({ slug, - owner: route.owner, - repo: route.repo, + owner: repoRoute.owner, + repo: repoRoute.repo, connected: repoAccess.level === 'write' && linked, }); - }, [slug, route, linked, recordRecent, repoAccess.level]); - - let initialPullRef = useRef({ done: false }); + // Refresh the in-memory recents list so HomeView stays in sync. + setRecents(listRecentRepos()); + }, [slug, repoRoute, linked, repoAccess.level]); + + let initialPullRef = useRef({ done: false, slug: '' }); + // Reset the "done" flag synchronously when the slug changes. + if (initialPullRef.current.slug !== slug) { + initialPullRef.current = { done: false, slug }; + } let shareRequestRef = useRef<{ owner: string; repo: string; path: string } | null>(null); // Synchronous localStorage lookup — no network call needed. @@ -298,7 +352,7 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput // Kick off the one-time remote import when visiting a writable repo we have not linked yet. useEffect(() => { (async () => { - if (route.kind !== 'repo') return; + if (repoRoute.kind !== 'repo') return; if (repoAccess.level !== 'write') return; if (!canEdit) return; if (linked) return; @@ -323,7 +377,7 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput let readmePath = synced.find((note) => note.path.toLowerCase() === 'readme.md')?.path; let initialPath = storedPath ?? readmePath; if (initialPath !== undefined && !pathsEqual(desiredPath, initialPath)) { - setActivePath(initialPath, { replace: true }); + navigateInternal(initialPath, { replace: true }); } } markRepoLinked(slug); @@ -335,7 +389,7 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput initialPullRef.current.done = true; } })(); - }, [route, repoAccess.level, linked, slug, canEdit, defaultBranch, desiredPath]); + }, [repoRoute, repoAccess.level, linked, slug, canEdit, defaultBranch, desiredPath]); useEffect(() => { if (!repoOwner || !repoName || !hasSession || !canEdit) { @@ -376,14 +430,27 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput loadShareForTarget, ]); - // CLICK HANDLERS + // INTERNAL NAVIGATION HELPER + // Replaces the old setActivePath callback. Updates internal route state so + // activePath reflects the new path immediately, and signals the routing adapter + // via pendingNavigation to sync the URL. + const navigateInternal = (path: string | undefined, options: { replace?: boolean } = {}) => { + setInternalRoute((prev) => { + if (prev.kind === 'repo') return { ...prev, notePath: path }; + if (prev.kind === 'new') return { ...prev, notePath: path }; + return prev; + }); + setPendingNavigation({ path, replace: options.replace ?? false }); + }; const ensureActivePath = (nextPath: string | undefined, options?: { replace?: boolean }) => { - if (pathsEqual(route.notePath, nextPath)) return; + if (pathsEqual(repoRoute.notePath, nextPath)) return; // hack: we navigate on the next event loop task to give React state time to update active doc - setTimeout(() => setActivePath(nextPath, options), 0); + setTimeout(() => navigateInternal(nextPath, options), 0); }; + // CLICK HANDLERS + // "Connect GitHub" button in the header const signIn = async () => { try { @@ -405,8 +472,8 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput window.open(manageUrl, '_blank', 'noopener'); return; } - if (route.kind !== 'repo') return; - let url = await apiGetInstallUrl(route.owner, route.repo, window.location.href); + if (repoRoute.kind !== 'repo') return; + let url = await apiGetInstallUrl(repoRoute.owner, repoRoute.repo, window.location.href); window.open(url, '_blank', 'noopener'); } catch (error) { logError(error); @@ -675,7 +742,71 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput await loadShareForTarget({ owner: repoOwner, repo: repoName, path: activePath }); }; - let state: RepoDataState = { + // DISPATCH: routes action objects to the appropriate internal handler. + // The implementation function returns `unknown` to cover all overloads; + // the typed Dispatch cast enforces the correct return type at call sites. + const dispatchImpl = (action: Action): unknown => { + switch (action.type) { + case 'route-changed': { + // URL changed externally: update internal route and clear pending navigation. + let newRoute = action.route; + setInternalRoute(newRoute); + setPendingNavigation(undefined); + return; + } + case 'sign-in': + return signIn(); + case 'sign-out': + return signOut(); + case 'open-repo-access': + return openRepoAccess(); + case 'sync-now': + return syncNow(); + case 'set-autosync': + setAutosync(action.enabled); + return; + case 'select-file': + return selectFile(action.path); + case 'create-note': + return createNote(action.dir, action.name); + case 'create-folder': + createFolder(action.parentDir, action.name); + return; + case 'rename-file': + renameFile(action.path, action.name); + return; + case 'move-file': + return moveFile(action.path, action.targetDir); + case 'delete-file': + deleteFile(action.path); + return; + case 'rename-folder': + renameFolder(action.dir, action.newName); + return; + case 'move-folder': + return moveFolder(action.dir, action.targetDir); + case 'delete-folder': + deleteFolder(action.dir); + return; + case 'save-file': + saveFile(action.path, action.text); + return; + case 'import-pasted-assets': + return importPastedAssets({ notePath: action.notePath, files: action.files }); + case 'create-share-link': + return createShare(); + case 'refresh-share-link': + return refreshShare(); + case 'revoke-share-link': + return revokeShare(); + } + }; + + // Cast to the typed Dispatch interface — justified because dispatchImpl correctly + // returns the right type for each action case. + const dispatch = dispatchImpl as Dispatch; + + let state: AppDataState = { hasSession, user, @@ -697,32 +828,15 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput share: shareState, statusMessage, defaultBranch, - }; - let actions: RepoDataActions = { - signIn, - signOut, - openRepoAccess, - - syncNow, - setAutosync, - selectFile, - createNote, - createFolder, - renameFile, - moveFile, - deleteFile, - renameFolder, - moveFolder, - deleteFolder, - saveFile, - importPastedAssets, - createShareLink: createShare, - refreshShareLink: refreshShare, - revokeShareLink: revokeShare, + // Routing info exposed for the UI adapter and consumers. + activeSlug: slug, + activeRoute: internalRoute, + pendingNavigation, + recents, }; - return { state, actions }; + return { state, dispatch }; } // Subscribe to the LocalStore's internal cache so React re-renders whenever diff --git a/src/data/data.test.ts b/src/data/data.test.ts index e95f699..59fb2d3 100644 --- a/src/data/data.test.ts +++ b/src/data/data.test.ts @@ -1,9 +1,7 @@ import { Buffer } from 'node:buffer'; import { act, renderHook, waitFor } from '@testing-library/react'; -import { useEffect, useState } from 'react'; import { beforeAll, beforeEach, describe, expect, test, vi } from 'vitest'; import type { RepoMetadata } from '../lib/backend'; -import type { RepoRoute } from '../ui/routing'; import { LocalStore, markRepoLinked, recordAutoSyncRun, setLastActiveFileId } from '../storage/local'; import type { RemoteFile } from '../sync/git-sync'; import type { RepoDataState, ImportedAsset } from '../data'; @@ -99,10 +97,11 @@ vi.mock('../sync/git-sync', async () => { }; }); -let useRepoData: typeof import('../data').useRepoData; +// Dynamic import ensures vi.mock hoisting takes effect before the module loads. +let useAppData: typeof import('../data').useAppData; beforeAll(async () => { - ({ useRepoData } = await import('../data')); + ({ useAppData } = await import('../data')); }); const mockSignInWithGitHubApp = authModule.signInWithGitHubApp; @@ -150,34 +149,7 @@ function createDeferred() { return { promise, resolve, reject } as const; } -type RecordRecentFn = (entry: { slug: string; owner?: string; repo?: string; connected?: boolean }) => void; - -type RenderRepoDataProps = { slug: string; route: RepoRoute; recordRecent: RecordRecentFn }; - -function renderRepoData(initial: RenderRepoDataProps) { - return renderHook( - ({ slug, route, recordRecent }: RenderRepoDataProps) => { - const [routeState, setRouteState] = useState(route); - useEffect(() => { - setRouteState(route); - }, [route]); - return useRepoData({ - slug, - route: routeState, - recordRecent, - setActivePath: (nextPath) => { - setRouteState((prev) => { - if (prev.kind === 'repo') return { ...prev, notePath: nextPath }; - return { kind: 'new', notePath: nextPath }; - }); - }, - }); - }, - { initialProps: initial } - ); -} - -describe('useRepoData', () => { +describe('useAppData', () => { beforeEach(() => { localStorage.clear(); @@ -212,8 +184,7 @@ describe('useRepoData', () => { // New workspaces should immediately surface the seeded welcome note without contacting remote APIs. test('seeds welcome note for a new workspace and keeps it editable', async () => { - const recordRecent = vi.fn(); - const { result } = renderRepoData({ slug: 'new', route: { kind: 'new' }, recordRecent }); + const { result } = renderHook(() => useAppData({ kind: 'new' })); expect(result.current.state.canEdit).toBe(true); expect(result.current.state.canSync).toBe(false); @@ -223,16 +194,16 @@ describe('useRepoData', () => { expect(welcomePath).toBeDefined(); act(() => { - result.current.actions.selectFile(welcomePath); + result.current.dispatch({ type: 'select-file', path: welcomePath }); }); await waitFor(() => expect(result.current.state.activeFile?.path).toBe(welcomePath)); expect(result.current.state.activeFile?.content).toContain('Welcome to VibeNote'); - expect(recordRecent).not.toHaveBeenCalled(); + // 'new' workspace never records recents + expect(result.current.state.recents).toHaveLength(0); }); test('tracks active note path on the new route', async () => { - const recordRecent = vi.fn(); const store = new LocalStore('new'); const alphaId = store.createFile('Alpha.md', 'alpha text'); const welcome = store.listFiles().find((note) => note.path === 'README.md'); @@ -240,27 +211,16 @@ describe('useRepoData', () => { if (!alpha) throw new Error('Failed to seed alpha note'); if (!welcome) throw new Error('Missing welcome note'); - const { result } = renderHook(() => { - const [routeState, setRouteState] = useState({ kind: 'new', notePath: alpha.path }); - const data = useRepoData({ - slug: 'new', - route: routeState, - recordRecent, - setActivePath: (nextPath) => setRouteState({ kind: 'new', notePath: nextPath }), - }); - return { data, routeState }; - }); + const { result } = renderHook(() => useAppData({ kind: 'new', notePath: alpha.path })); - await waitFor(() => expect(result.current.data.state.activePath).toBe(alpha.path)); - expect(result.current.data.state.activeFile?.content).toBe('alpha text'); - expect(result.current.routeState.notePath).toBe(alpha.path); + await waitFor(() => expect(result.current.state.activePath).toBe(alpha.path)); + expect(result.current.state.activeFile?.content).toBe('alpha text'); - await act(async () => { - await result.current.data.actions.selectFile(welcome.path); + act(() => { + result.current.dispatch({ type: 'select-file', path: welcome.path }); }); - await waitFor(() => expect(result.current.data.state.activePath).toBe(welcome.path)); - expect(result.current.routeState.notePath).toBe(welcome.path); + await waitFor(() => expect(result.current.state.activePath).toBe(welcome.path)); }); test('activates the route note path when the file exists locally', async () => { @@ -280,9 +240,9 @@ describe('useRepoData', () => { }); setRepoMetadata(writableMeta); - const recordRecent = vi.fn(); - const route: RepoRoute = { kind: 'repo', owner: 'acme', repo: 'docs', notePath: target.path }; - const { result } = renderRepoData({ slug, route, recordRecent }); + const { result } = renderHook(() => + useAppData({ kind: 'repo', owner: 'acme', repo: 'docs', notePath: target.path }) + ); await waitFor(() => expect(result.current.state.activePath).toBe(target.path)); expect(result.current.state.activeFile?.path).toBe(target.path); @@ -303,9 +263,7 @@ describe('useRepoData', () => { }); setRepoMetadata(writableMeta); - const recordRecent = vi.fn(); - const route: RepoRoute = { kind: 'repo', owner: 'acme', repo: 'assets' }; - const { result } = renderRepoData({ slug, route, recordRecent }); + const { result } = renderHook(() => useAppData({ kind: 'repo', owner: 'acme', repo: 'assets' })); await waitFor(() => expect(result.current.state.files.length).toBe(2)); const byPath = new Map(result.current.state.files.map((file) => [file.path, file.kind])); @@ -314,7 +272,6 @@ describe('useRepoData', () => { }); test('loads a read-only note that matches the route note path', async () => { - const slug = 'acme/docs'; setRepoMetadata(readOnlyMeta); mockListRepoFiles.mockResolvedValue([{ path: 'guides/Intro.md', sha: 'sha-intro', kind: 'markdown' }]); mockPullRepoFile.mockResolvedValue({ @@ -324,9 +281,9 @@ describe('useRepoData', () => { kind: 'markdown', }); - const recordRecent = vi.fn(); - const route: RepoRoute = { kind: 'repo', owner: 'acme', repo: 'docs', notePath: 'guides/Intro.md' }; - const { result } = renderRepoData({ slug, route, recordRecent }); + const { result } = renderHook(() => + useAppData({ kind: 'repo', owner: 'acme', repo: 'docs', notePath: 'guides/Intro.md' }) + ); await waitFor(() => expect(result.current.state.activePath).toBe('guides/Intro.md')); await waitFor(() => expect(result.current.state.activeFile?.content).toBe('# Intro')); @@ -336,7 +293,6 @@ describe('useRepoData', () => { // Writable repos should sync on demand and reflect updated auth/session state without losing edits. test('syncing a linked repo updates storage, reports status, and refreshes auth state', async () => { const slug = 'acme/docs'; - const recordRecent = vi.fn(); const seededUuid = '00000000-0000-0000-0000-000000000001'; const uuidSpy = vi.spyOn(globalThis.crypto, 'randomUUID').mockReturnValueOnce(seededUuid); @@ -366,11 +322,7 @@ describe('useRepoData', () => { user: { login: 'hubot', name: null, avatarUrl: 'https://example.com/hubot.png' }, }); - const { result } = renderRepoData({ - slug, - route: { kind: 'repo', owner: 'acme', repo: 'docs' }, - recordRecent, - }); + const { result } = renderHook(() => useAppData({ kind: 'repo', owner: 'acme', repo: 'docs' })); await waitFor(() => expect(result.current.state.repoQueryStatus).toBe('ready')); await waitFor(() => expect(result.current.state.files).toHaveLength(1)); @@ -378,25 +330,28 @@ describe('useRepoData', () => { expect(result.current.state.canEdit).toBe(true); expect(result.current.state.canSync).toBe(true); + // Once the repo is reachable and linked, it should appear in recents. await waitFor(() => - expect(recordRecent).toHaveBeenCalledWith(expect.objectContaining({ slug, connected: true })) + expect(result.current.state.recents).toContainEqual( + expect.objectContaining({ slug, connected: true }) + ) ); act(() => { - result.current.actions.selectFile(notePath); + result.current.dispatch({ type: 'select-file', path: notePath }); }); await waitFor(() => expect(result.current.state.activeFile?.path).toBe(notePath)); act(() => { - result.current.actions.saveFile(notePath, 'updated text'); + result.current.dispatch({ type: 'save-file', path: notePath, text: 'updated text' }); }); const storedAfterEdit = new LocalStore(slug).loadFileById(noteId); expect(storedAfterEdit?.content).toBe('updated text'); await act(async () => { - await result.current.actions.syncNow(); + await result.current.dispatch({ type: 'sync-now' }); }); expect(mockSyncBidirectional).toHaveBeenCalledTimes(1); @@ -406,7 +361,7 @@ describe('useRepoData', () => { expect(result.current.state.statusMessage).toBe('Synced: pulled 1, pushed 2'); await act(async () => { - await result.current.actions.signIn(); + await result.current.dispatch({ type: 'sign-in' }); }); expect(mockSignInWithGitHubApp).toHaveBeenCalledTimes(1); @@ -416,7 +371,6 @@ describe('useRepoData', () => { test('syncing a linked repo refreshes the active file contents after store updates', async () => { const slug = 'acme/docs'; - const recordRecent = vi.fn(); const store = new LocalStore(slug); const noteId = store.createFile('Seed.md', 'initial text'); @@ -442,22 +396,18 @@ describe('useRepoData', () => { }; }); - const { result } = renderRepoData({ - slug, - route: { kind: 'repo', owner: 'acme', repo: 'docs' }, - recordRecent, - }); + const { result } = renderHook(() => useAppData({ kind: 'repo', owner: 'acme', repo: 'docs' })); await waitFor(() => expect(result.current.state.repoQueryStatus).toBe('ready')); act(() => { - result.current.actions.selectFile(notePath); + result.current.dispatch({ type: 'select-file', path: notePath }); }); await waitFor(() => expect(result.current.state.activeFile?.content).toBe('initial text')); await act(async () => { - await result.current.actions.syncNow(); + await result.current.dispatch({ type: 'sync-now' }); }); await waitFor(() => expect(result.current.state.activeFile?.content).toBe('remote text')); @@ -465,7 +415,6 @@ describe('useRepoData', () => { test('sync surfaces detailed message when GitHub returns 422', async () => { const slug = 'acme/docs'; - const recordRecent = vi.fn(); mockGetSessionToken.mockReturnValue('session-token'); mockGetSessionUser.mockReturnValue({ @@ -483,17 +432,13 @@ describe('useRepoData', () => { }); mockSyncBidirectional.mockRejectedValue(ghError); - const { result } = renderRepoData({ - slug, - route: { kind: 'repo', owner: 'acme', repo: 'docs' }, - recordRecent, - }); + const { result } = renderHook(() => useAppData({ kind: 'repo', owner: 'acme', repo: 'docs' })); await waitFor(() => expect(result.current.state.repoQueryStatus).toBe('ready')); await waitFor(() => expect(result.current.state.canSync).toBe(true)); await act(async () => { - await result.current.actions.syncNow(); + await result.current.dispatch({ type: 'sync-now' }); }); expect(mockSyncBidirectional).toHaveBeenCalledWith(expect.any(LocalStore), slug); @@ -505,7 +450,6 @@ describe('useRepoData', () => { test('importPastedAssets creates binary assets and returns markdown-friendly paths', async () => { const slug = 'acme/docs'; - const recordRecent = vi.fn(); const uuidSpy = vi .spyOn(globalThis.crypto, 'randomUUID') @@ -540,17 +484,13 @@ describe('useRepoData', () => { folder: 'assets', }); - const { result } = renderRepoData({ - slug, - route: { kind: 'repo', owner: 'acme', repo: 'docs' }, - recordRecent, - }); + const { result } = renderHook(() => useAppData({ kind: 'repo', owner: 'acme', repo: 'docs' })); await waitFor(() => expect(result.current.state.repoQueryStatus).toBe('ready')); await waitFor(() => expect(result.current.state.canEdit).toBe(true)); act(() => { - result.current.actions.selectFile(notePath); + result.current.dispatch({ type: 'select-file', path: notePath }); }); await waitFor(() => expect(result.current.state.activeFile?.path).toBe(notePath)); @@ -558,7 +498,8 @@ describe('useRepoData', () => { let imported: ImportedAsset[] = []; await act(async () => { - imported = await result.current.actions.importPastedAssets({ + imported = await result.current.dispatch({ + type: 'import-pasted-assets', notePath, files: [new File(['binary'], 'paste.png', { type: 'image/png' })], }); @@ -592,7 +533,6 @@ describe('useRepoData', () => { // Read-only repos should list remote notes and refresh on selection. test('read-only repos surface notes and refresh on selection', async () => { const slug = 'octo/wiki'; - const recordRecent = vi.fn(); mockGetSessionToken.mockReturnValue(null); setRepoMetadata(readOnlyMeta); @@ -606,11 +546,7 @@ describe('useRepoData', () => { }) ); - const { result } = renderRepoData({ - slug, - route: { kind: 'repo', owner: 'octo', repo: 'wiki' }, - recordRecent, - }); + const { result } = renderHook(() => useAppData({ kind: 'repo', owner: 'octo', repo: 'wiki' })); await waitFor(() => expect(result.current.state.repoQueryStatus).toBe('ready')); expect(result.current.state.canEdit).toBe(false); @@ -623,12 +559,15 @@ describe('useRepoData', () => { expect(result.current.state.activePath).toBeUndefined(); expect(result.current.state.activeFile).toBeUndefined(); + // Once the repo is reachable (read-only), it should appear in recents as not connected. await waitFor(() => - expect(recordRecent).toHaveBeenCalledWith(expect.objectContaining({ slug, connected: false })) + expect(result.current.state.recents).toContainEqual( + expect.objectContaining({ slug, connected: false }) + ) ); act(() => { - result.current.actions.selectFile('docs/alpha.md'); + result.current.dispatch({ type: 'select-file', path: 'docs/alpha.md' }); }); await waitFor(() => expect(result.current.state.activeFile?.content).toBe('# docs/alpha.md')); @@ -643,7 +582,7 @@ describe('useRepoData', () => { }); act(() => { - result.current.actions.selectFile('docs/alpha.md'); + result.current.dispatch({ type: 'select-file', path: 'docs/alpha.md' }); }); await waitFor(() => expect(result.current.state.activeFile?.content).toBe('# updated remote')); @@ -651,9 +590,6 @@ describe('useRepoData', () => { }); test('read-only repos list README without auto-selecting it', async () => { - const slug = 'octo/wiki'; - const recordRecent = vi.fn(); - mockGetSessionToken.mockReturnValue(null); setRepoMetadata(readOnlyMeta); mockListRepoFiles.mockResolvedValue([ @@ -669,11 +605,7 @@ describe('useRepoData', () => { }) ); - const { result } = renderRepoData({ - slug, - route: { kind: 'repo', owner: 'octo', repo: 'wiki' }, - recordRecent, - }); + const { result } = renderHook(() => useAppData({ kind: 'repo', owner: 'octo', repo: 'wiki' })); await waitFor(() => expect(result.current.state.files.length).toBe(2)); expect(result.current.state.activePath).toBe('README.md'); @@ -683,7 +615,6 @@ describe('useRepoData', () => { test('linked repos focus README after initial import', async () => { const slug = 'acme/docs'; - const recordRecent = vi.fn(); mockGetSessionToken.mockReturnValue('session-token'); mockGetSessionUser.mockReturnValue({ @@ -710,11 +641,7 @@ describe('useRepoData', () => { .mockReturnValueOnce('00000000-0000-0000-0000-000000000111') .mockReturnValueOnce('00000000-0000-0000-0000-000000000222'); - const { result } = renderRepoData({ - slug, - route: { kind: 'repo', owner: 'acme', repo: 'docs' }, - recordRecent, - }); + const { result } = renderHook(() => useAppData({ kind: 'repo', owner: 'acme', repo: 'docs' })); await waitFor(() => expect(result.current.state.files.length).toBe(2)); const readmeEntry = result.current.state.files.find((file) => file.path === 'README.md'); @@ -728,7 +655,6 @@ describe('useRepoData', () => { // During the repo access check, the active document should never flicker away in the UI. test('doc remains loaded while repo access resolves', async () => { const slug = 'acme/docs'; - const recordRecent = vi.fn(); const seededUuid = '00000000-0000-0000-0000-000000000042'; const uuidSpy = vi.spyOn(globalThis.crypto, 'randomUUID').mockReturnValueOnce(seededUuid); @@ -752,15 +678,7 @@ describe('useRepoData', () => { const seenNeedsInstall: boolean[] = []; const seenCanEdit: boolean[] = []; const { result } = renderHook(() => { - const [route, setRoute] = useState({ kind: 'repo', owner: 'acme', repo: 'docs' }); - const value = useRepoData({ - slug, - route, - recordRecent, - setActivePath: (nextPath) => { - setRoute((prev) => (prev.kind === 'repo' ? { ...prev, notePath: nextPath } : prev)); - }, - }); + const value = useAppData({ kind: 'repo', owner: 'acme', repo: 'docs' }); seenDocIds.push(value.state.activeFile?.id); seenNeedsInstall.push(needsInstall(value.state) || needsSessionRefresh(value.state)); seenCanEdit.push(value.state.canEdit); @@ -784,7 +702,6 @@ describe('useRepoData', () => { // Autosync should run quietly in the background without flickering UI state. test('autosync schedules background sync without surfacing UI noise', async () => { const slug = 'acme/docs'; - const recordRecent = vi.fn(); const uuidSpy = vi .spyOn(globalThis.crypto, 'randomUUID') @@ -814,29 +731,25 @@ describe('useRepoData', () => { const setTimeoutSpy = vi.spyOn(window, 'setTimeout'); - const { result } = renderRepoData({ - slug, - route: { kind: 'repo', owner: 'acme', repo: 'docs' }, - recordRecent, - }); + const { result } = renderHook(() => useAppData({ kind: 'repo', owner: 'acme', repo: 'docs' })); await waitFor(() => expect(result.current.state.repoQueryStatus).toBe('ready')); act(() => { - result.current.actions.selectFile(notePath); + result.current.dispatch({ type: 'select-file', path: notePath }); }); await waitFor(() => expect(result.current.state.activeFile?.path).toBe(notePath)); act(() => { - result.current.actions.setAutosync(true); + result.current.dispatch({ type: 'set-autosync', enabled: true }); }); mockSyncBidirectional.mockClear(); setTimeoutSpy.mockClear(); act(() => { - result.current.actions.saveFile(notePath, 'updated text'); + result.current.dispatch({ type: 'save-file', path: notePath, text: 'updated text' }); }); const lastCall = setTimeoutSpy.mock.calls.at(-1); @@ -858,8 +771,6 @@ describe('useRepoData', () => { // Switching to another repo should swap all derived state without leaking the previous doc. test('switching repositories replaces local state without leaking the previous doc', async () => { - const recordRecent = vi.fn(); - const slugA = 'acme/docs'; const slugB = 'acme/wiki'; @@ -891,50 +802,30 @@ describe('useRepoData', () => { const seenDocIds: Array = []; const seenNeedsInstall: boolean[] = []; - const { result, rerender } = renderHook( - ({ slug, route, recordRecent }: { slug: string; route: RepoRoute; recordRecent: RecordRecentFn }) => { - const [routeState, setRouteState] = useState(route); - const value = useRepoData({ - slug, - route: routeState, - recordRecent, - setActivePath: (nextPath) => { - setRouteState((prev) => (prev.kind === 'repo' ? { ...prev, notePath: nextPath } : prev)); - }, - }); - seenDocIds.push(value.state.activeFile?.id); - seenNeedsInstall.push(needsInstall(value.state)); - return value; - }, - { - initialProps: { - slug: slugA, - route: { kind: 'repo', owner: 'acme', repo: 'docs' }, - recordRecent, - }, - } - ); + const { result } = renderHook(() => { + const value = useAppData({ kind: 'repo', owner: 'acme', repo: 'docs' }); + seenDocIds.push(value.state.activeFile?.id); + seenNeedsInstall.push(needsInstall(value.state)); + return value; + }); await waitFor(() => expect(result.current.state.repoQueryStatus).toBe('ready')); act(() => { - result.current.actions.selectFile(noteAPath); + result.current.dispatch({ type: 'select-file', path: noteAPath }); }); await waitFor(() => expect(result.current.state.activeFile?.id).toBe(noteA)); + // Switch to repo B via dispatch — no rerender needed. act(() => { - rerender({ - slug: slugB, - route: { kind: 'repo', owner: 'acme', repo: 'wiki' }, - recordRecent, - }); + result.current.dispatch({ type: 'route-changed', route: { kind: 'repo', owner: 'acme', repo: 'wiki' } }); }); await waitFor(() => expect(result.current.state.repoQueryStatus).toBe('ready')); act(() => { - result.current.actions.selectFile(noteBPath); + result.current.dispatch({ type: 'select-file', path: noteBPath }); }); await waitFor(() => expect(result.current.state.activeFile?.id).toBe(noteB)); @@ -946,7 +837,6 @@ describe('useRepoData', () => { // The needs-relogin flow should keep the doc visible and toggle the banner off after re-auth. test('token refresh flow preserves the current doc while awaiting GitHub access', async () => { const slug = 'acme/private'; - const recordRecent = vi.fn(); const store = new LocalStore(slug); const noteId = store.createFile('Secret.md', 'classified'); @@ -979,15 +869,7 @@ describe('useRepoData', () => { const seenRepoLinked: boolean[] = []; const { result } = renderHook(() => { - const [route, setRoute] = useState({ kind: 'repo', owner: 'acme', repo: 'private' }); - const value = useRepoData({ - slug, - route, - recordRecent, - setActivePath: (nextPath) => { - setRoute((prev) => (prev.kind === 'repo' ? { ...prev, notePath: nextPath } : prev)); - }, - }); + const value = useAppData({ kind: 'repo', owner: 'acme', repo: 'private' }); seenUserActionRequired.push(needsInstall(value.state) || needsSessionRefresh(value.state)); seenRepoLinked.push(value.state.repoLinked); return value; @@ -1002,11 +884,11 @@ describe('useRepoData', () => { mockGetRepoMetadata.mockResolvedValue({ ...writableMeta }); await act(async () => { - await result.current.actions.signIn(); + await result.current.dispatch({ type: 'sign-in' }); }); act(() => { - result.current.actions.selectFile(notePath); + result.current.dispatch({ type: 'select-file', path: notePath }); }); await waitFor(() => expect(result.current.state.activeFile?.id).toBe(noteId)); @@ -1016,7 +898,6 @@ describe('useRepoData', () => { test('auth refresh failure surfaces re-login prompt without clearing notes', async () => { const slug = 'acme/lost-auth'; - const recordRecent = vi.fn(); const store = new LocalStore(slug); const noteId = store.createFile('Draft.md', 'pending changes'); @@ -1048,17 +929,9 @@ describe('useRepoData', () => { user: { login: 'mona', name: 'Mona', avatarUrl: '' }, }); - const { result } = renderHook(() => { - const [route, setRoute] = useState({ kind: 'repo', owner: 'acme', repo: 'lost-auth' }); - return useRepoData({ - slug, - route, - recordRecent, - setActivePath: (nextPath) => { - setRoute((prev) => (prev.kind === 'repo' ? { ...prev, notePath: nextPath } : prev)); - }, - }); - }); + const { result } = renderHook(() => + useAppData({ kind: 'repo', owner: 'acme', repo: 'lost-auth' }) + ); act(() => { firstMeta.resolve(lostAuthMeta); @@ -1075,14 +948,14 @@ describe('useRepoData', () => { mockGetRepoMetadata.mockResolvedValue({ ...writableMeta }); await act(async () => { - await result.current.actions.signIn(); + await result.current.dispatch({ type: 'sign-in' }); }); await waitFor(() => expect(needsInstall(result.current.state)).toBe(false)); expect(result.current.state.repoLinked).toBe(true); await waitFor(() => expect(result.current.state.repoErrorType).toBeUndefined()); act(() => { - result.current.actions.selectFile(notePath); + result.current.dispatch({ type: 'select-file', path: notePath }); }); await waitFor(() => expect(result.current.state.activeFile?.id).toBe(noteId)); }); @@ -1090,7 +963,6 @@ describe('useRepoData', () => { // Switching notes in read-only mode should respect loading states without toggling install banners. test('read-only selection keeps install state stable', async () => { const slug = 'octo/wiki'; - const recordRecent = vi.fn(); mockGetSessionToken.mockReturnValue(null); setRepoMetadata(readOnlyMeta); @@ -1113,15 +985,7 @@ describe('useRepoData', () => { const seenRepoLinked: boolean[] = []; const { result } = renderHook(() => { - const [route, setRoute] = useState({ kind: 'repo', owner: 'octo', repo: 'wiki' }); - const value = useRepoData({ - slug, - route, - recordRecent, - setActivePath: (nextPath) => { - setRoute((prev) => (prev.kind === 'repo' ? { ...prev, notePath: nextPath } : prev)); - }, - }); + const value = useAppData({ kind: 'repo', owner: 'octo', repo: 'wiki' }); seenNeedsInstall.push(needsInstall(value.state)); seenRepoLinked.push(value.state.repoLinked); return value; @@ -1131,11 +995,11 @@ describe('useRepoData', () => { await waitFor(() => expect(result.current.state.files.length).not.toBe(0)); expect(result.current.state.activePath).toBeUndefined(); act(() => { - result.current.actions.selectFile('docs/alpha.md'); + result.current.dispatch({ type: 'select-file', path: 'docs/alpha.md' }); }); await waitFor(() => expect(result.current.state.activeFile?.path).toBe('docs/alpha.md')); act(() => { - result.current.actions.selectFile('docs/beta.md'); + result.current.dispatch({ type: 'select-file', path: 'docs/beta.md' }); }); await waitFor(() => expect(result.current.state.activeFile?.path).toBe('docs/beta.md')); @@ -1147,9 +1011,6 @@ describe('useRepoData', () => { // Regression test for issue #91: switching files on public repos should not cause infinite loops. // The bug was caused by race conditions between selectFile() and the auto-load effect. test('read-only file selection does not oscillate between files', async () => { - const slug = 'octo/public'; - const recordRecent = vi.fn(); - mockGetSessionToken.mockReturnValue(null); setRepoMetadata(readOnlyMeta); @@ -1158,7 +1019,7 @@ describe('useRepoData', () => { { path: 'docs/guide.md', sha: 'sha-guide', kind: 'markdown' }, ]); - // Track all paths that setActivePath is called with to detect oscillation + // Track activePath on every render to detect oscillation between files. let activePathHistory: (string | undefined)[] = []; let pullCount = 0; @@ -1170,30 +1031,23 @@ describe('useRepoData', () => { }); const { result } = renderHook(() => { - const [route, setRoute] = useState({ kind: 'repo', owner: 'octo', repo: 'public' }); - return useRepoData({ - slug, - route, - recordRecent, - setActivePath: (nextPath) => { - activePathHistory.push(nextPath); - setRoute((prev) => (prev.kind === 'repo' ? { ...prev, notePath: nextPath } : prev)); - }, - }); + const value = useAppData({ kind: 'repo', owner: 'octo', repo: 'public' }); + activePathHistory.push(value.state.activePath); + return value; }); await waitFor(() => expect(result.current.state.files.length).toBe(2)); // Select docs/guide.md act(() => { - result.current.actions.selectFile('docs/guide.md'); + result.current.dispatch({ type: 'select-file', path: 'docs/guide.md' }); }); // Wait for selection to complete await waitFor(() => expect(result.current.state.activeFile?.path).toBe('docs/guide.md')); // Check that we didn't oscillate - path history should not have repeated back-and-forth - // A healthy history might be: ['README.md', 'docs/guide.md'] or similar + // A healthy history might be: [undefined, undefined, ..., 'README.md', ..., 'docs/guide.md'] // An oscillating history would be: ['docs/guide.md', 'README.md', 'docs/guide.md', ...] let oscillations = 0; for (let i = 2; i < activePathHistory.length; i++) { @@ -1210,7 +1064,6 @@ describe('useRepoData', () => { // Signing out should clear local data and disable syncing. test('signing out clears local state and disables syncing', async () => { const slug = 'acme/docs'; - const recordRecent = vi.fn(); const store = new LocalStore(slug); const noteId = store.createFile('Seed.md', 'content'); @@ -1227,22 +1080,18 @@ describe('useRepoData', () => { setRepoMetadata(writableMeta); mockSignOutFromGitHubApp.mockResolvedValue(undefined); - const { result } = renderRepoData({ - slug, - route: { kind: 'repo', owner: 'acme', repo: 'docs' }, - recordRecent, - }); + const { result } = renderHook(() => useAppData({ kind: 'repo', owner: 'acme', repo: 'docs' })); await waitFor(() => expect(result.current.state.repoQueryStatus).toBe('ready')); act(() => { - result.current.actions.selectFile(notePath); + result.current.dispatch({ type: 'select-file', path: notePath }); }); await waitFor(() => expect(result.current.state.activeFile?.id).toBe(noteId)); await act(async () => { - await result.current.actions.signOut(); + await result.current.dispatch({ type: 'sign-out' }); }); expect(mockSignOutFromGitHubApp).toHaveBeenCalledTimes(1); diff --git a/src/ui/RepoSwitcher.tsx b/src/ui/RepoSwitcher.tsx index 389d325..a83c1b6 100644 --- a/src/ui/RepoSwitcher.tsx +++ b/src/ui/RepoSwitcher.tsx @@ -1,6 +1,6 @@ import { useEffect, useMemo, useRef, useState, type RefObject } from 'react'; import type { Route } from './routing'; -import { listRecentRepos, type RecentRepo } from '../storage/local'; +import { listRecentRepos, recordRecentRepo, type RecentRepo } from '../storage/local'; import { repoExists } from '../sync/git-sync'; import { useOnClickOutside } from './useOnClickOutside'; @@ -9,7 +9,6 @@ type Props = { slug: string; navigate: (route: Route, options?: { replace?: boolean }) => void; onClose: () => void; - onRecordRecent: (entry: { slug: string; owner?: string; repo?: string; connected?: boolean }) => void; triggerRef?: RefObject; }; @@ -22,7 +21,7 @@ function parseOwnerRepo(input: string): Parsed { return { owner, repo }; } -export function RepoSwitcher({ route, slug, navigate, onClose, onRecordRecent, triggerRef }: Props) { +export function RepoSwitcher({ route, slug, navigate, onClose, triggerRef }: Props) { const [input, setInput] = useState(''); const [recents, setRecents] = useState(() => listRecentRepos()); const [checking, setChecking] = useState(false); @@ -81,7 +80,8 @@ export function RepoSwitcher({ route, slug, navigate, onClose, onRecordRecent, t }, [input]); const goTo = (owner: string, repo: string) => { - onRecordRecent({ slug: `${owner}/${repo}`, owner, repo }); + // Record the repo immediately on navigation so it appears in recents right away. + recordRecentRepo({ slug: `${owner}/${repo}`, owner, repo }); navigate({ kind: 'repo', owner, repo }); onClose(); }; diff --git a/src/ui/RepoView.tsx b/src/ui/RepoView.tsx index a01574f..2669b99 100644 --- a/src/ui/RepoView.tsx +++ b/src/ui/RepoView.tsx @@ -7,7 +7,7 @@ import { AssetViewer } from './AssetViewer'; import { RepoSwitcher } from './RepoSwitcher'; import { Toggle } from './Toggle'; import { GitHubIcon, ExternalLinkIcon, NotesIcon, CloseIcon, SyncIcon, ShareIcon } from './RepoIcons'; -import { useRepoData } from '../data'; +import type { AppDataState, Dispatch } from '../data'; import type { FileMeta } from '../storage/local'; import { getExpandedFolders, @@ -20,47 +20,30 @@ import { extractDir, stripExtension, } from '../storage/local'; -import type { RepoRoute, Route } from './routing'; +import type { Route } from './routing'; import { normalizePath, pathsEqual } from '../lib/util'; import { useRepoAssetLoader } from './useRepoAssetLoader'; import { ShareDialog } from './ShareDialog'; import { useOnClickOutside } from './useOnClickOutside'; type RepoViewProps = { - slug: string; - route: RepoRoute; + state: AppDataState; + dispatch: Dispatch; navigate: (route: Route, options?: { replace?: boolean }) => void; - recordRecent: (entry: { - slug: string; - owner?: string; - repo?: string; - title?: string; - connected?: boolean; - }) => void; }; const primaryModifier = detectPrimaryShortcut(); +// RepoView is a thin wrapper that resets all ephemeral UI state when the slug changes. export function RepoView(props: RepoViewProps) { - return ; + return ; } -function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { - const setActivePath = (nextPath: string | undefined, options?: { replace?: boolean }) => { - let replace = options?.replace === true; - if (route.kind === 'repo') { - if (pathsEqual(route.notePath, nextPath)) return; - navigate({ kind: 'repo', owner: route.owner, repo: route.repo, notePath: nextPath }, { replace }); - return; - } - if (route.kind === 'new') { - if (pathsEqual(route.notePath, nextPath)) return; - navigate({ kind: 'new', notePath: nextPath }, { replace }); - } - }; +function RepoViewInner({ state, dispatch, navigate }: RepoViewProps) { + // Narrow the active route to the repo/new context for local use. + const route = state.activeRoute; + const slug = state.activeSlug; - // Data layer exposes repo-backed state and the high-level actions the UI needs. - const { state, actions } = useRepoData({ slug, route, recordRecent, setActivePath }); const { hasSession, user, @@ -112,8 +95,8 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { useEffect(() => { if (!shareOpen) return; if (share.status !== 'idle') return; - void actions.refreshShareLink(); - }, [shareOpen, share.status, actions.refreshShareLink]); + void dispatch({ type: 'refresh-share-link' }); + }, [shareOpen, share.status]); const [showSwitcher, setShowSwitcher] = useState(false); @@ -159,7 +142,7 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { }, []); const onSelect = async (path: string | undefined) => { - await actions.selectFile(path); + await dispatch({ type: 'select-file', path }); setSidebarOpen(false); }; @@ -233,7 +216,7 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) {
{!hasSession ? ( - ) : ( @@ -260,7 +243,7 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { {canSync && ( )} @@ -372,9 +355,11 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { readOnly={!canEdit} slug={slug} loadAsset={loadAsset} - onImportAssets={actions.importPastedAssets} + onImportAssets={(params) => + dispatch({ type: 'import-pasted-assets', notePath: params.notePath, files: params.files }) + } onChange={(path, text) => { - actions.saveFile(path, text); + dispatch({ type: 'save-file', path, text }); }} /> ) : isBinaryFile(activeFile) || isAssetUrlFile(activeFile) ? ( @@ -385,7 +370,7 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { doc={activeFile} readOnly={!canEdit} onChange={(path, text) => { - actions.saveFile(path, text); + dispatch({ type: 'save-file', path, text }); }} /> ) : null} @@ -397,7 +382,7 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { <>

VibeNote lost permission to talk to GitHub for this repository.

Sign in again to refresh your session without clearing any local notes.

- @@ -413,7 +398,7 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { , or grant access to all repositories (not recommended).

{hasSession ? ( - ) : ( @@ -461,7 +446,7 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) {
diff --git a/src/ui/ShareDialog.tsx b/src/ui/ShareDialog.tsx index 148d3f6..612cda3 100644 --- a/src/ui/ShareDialog.tsx +++ b/src/ui/ShareDialog.tsx @@ -11,9 +11,9 @@ type ShareDialogProps = { notePath: string | undefined; triggerRef: RefObject; onClose: () => void; - onCreate: () => Promise; - onRevoke: () => Promise; - onRefresh: () => Promise; + onCreate: () => void | Promise; + onRevoke: () => void | Promise; + onRefresh: () => void | Promise; }; function ShareDialog({ diff --git a/tasks/clean-data-ui-boundary.md b/tasks/clean-data-ui-boundary.md new file mode 100644 index 0000000..ce054e2 --- /dev/null +++ b/tasks/clean-data-ui-boundary.md @@ -0,0 +1,43 @@ +--- +status: done +completed: 2026-03-05 +created: 2026-03-05 +--- + +# Clean up the data layer ↔ UI boundary (#99) + +## Context + +We're preparing to rebuild the data layer (storage, git/sync, app state) from scratch as "V2". Before that, we need the current boundary between data and UI to be crisp, so V2 can be a drop-in replacement behind the same API surface. + +Today, `useRepoData` in `src/data.ts` already returns `{ state, actions }` — the shape is mostly right. But there are a few things that need to change to make it a clean, swappable contract. + +## Goal + +Refactor the current `useRepoData` hook and its consumers so that: + +1. **Actions are data, not function calls.** The UI dispatches action objects (`dispatch({ type: 'create-note', dir, name })`) instead of calling `actions.createNote(dir, name)`. Define a discriminated union `Action` type and a single `dispatch(action: Action)` function. Internally, `dispatch` can just call the existing action functions — no logic changes needed. + +2. **The data layer is app-level, not per-repo.** Today `useRepoData` takes a `slug` and is mounted per-repo. Lift it so there's a single app-level hook (call it `useAppData` or similar) that handles session/auth, repo transitions, and recents — things that are already global but awkwardly live inside a per-repo hook. + +3. **Routing is bidirectional but outside the data layer.** Today the UI passes `setActivePath` and `recordRecent` callbacks *into* the data hook, so the data layer can drive navigation. Remove those inputs. Instead: + - Route changes flow **in** to the data layer as actions: `dispatch({ type: 'route-changed', route })`. + - The data layer expresses "where the user is" as **state** (e.g. `activePath`, `activeSlug`). + - A thin UI adapter syncs the two: URL changes → dispatch, state changes → `navigate(...)`. + - The data layer never knows about URLs or calls navigate. + +## Must-haves + +- The app works exactly as before from a user's perspective. No behavioral changes. +- All existing tests pass (`npm test`). +- `npm run check` passes with no type errors. +- The `Action` union type and `dispatch` function are exported and used by all UI consumers. No UI component calls action functions directly. +- `setActivePath` and `recordRecent` are no longer inputs to the data hook. +- The data hook is instantiated once at the app level, not per-repo. + +## Validation + +1. `npm run check` clean. +2. `npm test` green. +3. Manual review: grep confirms no UI file imports or calls action functions directly (only `dispatch`). `setActivePath` and `recordRecent` don't appear as data hook inputs. +4. We will do a browser smoke test after the code changes.