From 2eaf28faa0d77980e2045f470d5bf26fefd8eb74 Mon Sep 17 00:00:00 2001 From: Gregor Date: Fri, 6 Mar 2026 00:32:31 +0100 Subject: [PATCH 01/18] Refactor UI around app-level data contract for #99 --- src/App.tsx | 105 ++-- src/data.ts | 541 +++++++++++++++++- src/data/data.integration.test.ts | 169 ++++++ src/data/data.test.ts | 116 ++-- src/ui/HomeView.tsx | 14 +- src/ui/RepoSwitcher.tsx | 51 +- src/ui/RepoView.tsx | 218 ++++--- src/ui/ShareDialog.tsx | 11 +- ...-current-implementation-to-app-contract.md | 63 ++ tasks/app-level-data-contract.md | 38 ++ tasks/audit-ui-and-app-state-ownership.md | 93 +++ tasks/define-action-data-protocol.md | 130 +++++ tasks/define-app-data-state-contract.md | 157 +++++ 13 files changed, 1400 insertions(+), 306 deletions(-) create mode 100644 src/data/data.integration.test.ts create mode 100644 tasks/adapt-current-implementation-to-app-contract.md create mode 100644 tasks/app-level-data-contract.md create mode 100644 tasks/audit-ui-and-app-state-ownership.md create mode 100644 tasks/define-action-data-protocol.md create mode 100644 tasks/define-app-data-state-contract.md diff --git a/src/App.tsx b/src/App.tsx index c98fa02..7646516 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,84 +1,59 @@ -import React, { useCallback, useEffect, useState } from 'react'; -import { useRoute } from './ui/routing'; +import React, { useEffect } from 'react'; +import { useAppData, type AppNavigationState } from './data'; +import { useRoute, type Route } from './ui/routing'; import { RepoView } from './ui/RepoView'; import { HomeView } from './ui/HomeView'; -import { listRecentRepos, recordRecentRepo, type RecentRepo } from './storage/local'; export function App() { const { route, navigate } = useRoute(); + let { state, dispatch, helpers } = useAppData({ route }); // Adjust page title based on route useEffect(() => { - document.title = route.kind === 'repo' ? `${route.owner}/${route.repo}` : 'VibeNote'; - }, [route]); + let target = state.workspace?.target; + document.title = + target !== undefined && target.kind === 'github' ? `${target.owner}/${target.repo}` : 'VibeNote'; + }, [state.workspace?.target]); - // redirects useEffect(() => { - // if the route is /start, redirect to the most recent repo or /home - if (route.kind === 'start') { - let candidate = recents.find((entry) => entry.owner !== undefined && entry.repo !== undefined); - - if (candidate !== undefined) { - navigate({ kind: 'repo', owner: candidate.owner!, repo: candidate.repo! }, { replace: true }); - return; - } - navigate({ kind: 'home' }, { replace: true }); - } - - // if the route is /home and there are no recent repos, redirect to /new for the onboarding flow - if (route.kind === 'home') { - if (listRecentRepos().length === 0) { - navigate({ kind: 'new', notePath: 'README.md' }, { replace: true }); - } - } - }, [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 ; + let nextRoute = routeFromNavigation(state.navigation); + if (nextRoute === undefined) return; + if (routesEqual(route, nextRoute)) return; + navigate(nextRoute, { replace: state.navigation.replace === true }); + }, [route, navigate, state.navigation]); + + if (state.navigation.screen === 'home') { + return ; } - if (route.kind === 'start') { - // will redirect immediately - return null; - } - - if (route.kind === 'new') { - return ; - } - - if (route.kind === 'repo') { - return ( - - ); + if (state.navigation.screen === 'workspace' && state.workspace !== undefined) { + return ; } return null; } -function useRecents() { - const [recents, setRecents] = useState(() => listRecentRepos()); - - useEffect(() => { - const onStorage = () => setRecents(listRecentRepos()); - window.addEventListener('storage', onStorage); - return () => window.removeEventListener('storage', onStorage); - }, []); +function routeFromNavigation(navigation: AppNavigationState): Route | undefined { + if (navigation.screen === 'home') return { kind: 'home' } as const; + if (navigation.screen !== 'workspace' || navigation.target === undefined) return undefined; + if (navigation.target.repo.kind === 'new') { + return { kind: 'new', notePath: navigation.target.notePath } as const; + } + return { + kind: 'repo', + owner: navigation.target.repo.owner, + repo: navigation.target.repo.repo, + notePath: navigation.target.notePath, + } as const; +} - const recordRecent = useCallback( - (entry: { slug: string; owner?: string; repo?: string; title?: string; connected?: boolean }) => { - recordRecentRepo(entry); - setRecents(listRecentRepos()); - }, - [] - ); - return [recents, recordRecent] as const; +function routesEqual(a: Route | undefined, b: Route | undefined) { + if (a === undefined || b === undefined) return a === b; + if (a.kind !== b.kind) return false; + if (a.kind === 'home' && b.kind === 'home') return true; + if (a.kind === 'new' && b.kind === 'new') return a.notePath === b.notePath; + if (a.kind === 'repo' && b.kind === 'repo') { + return a.owner === b.owner && a.repo === b.repo && a.notePath === b.notePath; + } + return false; } diff --git a/src/data.ts b/src/data.ts index c00b56e..be10ab3 100644 --- a/src/data.ts +++ b/src/data.ts @@ -15,6 +15,9 @@ import { getRepoStore, computeSyncedHash, extractDir, + recordRecentRepo, + listRecentRepos, + type RecentRepo, } from './storage/local'; import { signInWithGitHubApp, @@ -42,6 +45,7 @@ import { type SyncSummary, listRepoFiles, pullRepoFile, + repoExists, type RemoteFile, formatSyncFailure, } from './sync/git-sync'; @@ -50,14 +54,20 @@ 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, useRepoData }; export type { + AppDataAction, + AppDataResult, + AppDataState, + AppNavigationState, + AppNavigationTarget, RepoAccessState, RepoDataInputs, RepoDataState, RepoDataActions, + RepoDataRouteSync, ShareState, RepoAccessErrorType, ImportedAsset, @@ -134,6 +144,13 @@ type RepoDataActions = { refreshShareLink: () => Promise; revokeShareLink: () => Promise; }; + +type RepoDataRouteSync = { + revision: number; + replace: boolean; + route: RepoRoute; +}; + type ImportedAsset = { assetPath: string; markdownPath: string; @@ -143,21 +160,113 @@ type ImportedAsset = { 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; +}; + +type AppNavigationTarget = + | { repo: { kind: 'new'; slug: 'new' }; notePath?: string } + | { repo: { kind: 'github'; slug: string; owner: string; repo: string }; notePath?: string }; + +type AppNavigationState = { + screen: 'resolving' | 'home' | 'workspace'; + target?: AppNavigationTarget; + replace?: boolean; +}; + +type RepoProbeState = { + status: 'idle' | 'checking' | 'ready'; + owner?: string; + repo?: string; + exists?: boolean; +}; + +type AppDataState = { + session: { + status: 'signed-out' | 'signed-in'; + user: AppUser | undefined; + }; + navigation: AppNavigationState; + repos: { + recents: RecentRepo[]; + probe: RepoProbeState; + }; + workspace?: { + target: AppNavigationTarget['repo']; + access: { + status: RepoQueryStatus; + level: RepoAccessLevel; + canRead: boolean; + canEdit: boolean; + canSync: boolean; + linked: boolean; + manageUrl: string | undefined; + defaultBranch: string | undefined; + errorType: RepoAccessErrorType | undefined; + }; + tree: { + files: FileMeta[]; + folders: string[]; + }; + document: { + activeFile: RepoFile | undefined; + activePath: string | undefined; + }; + sync: { + autosync: boolean; + syncing: boolean; + statusMessage: string | undefined; + }; + share: { + status: ShareState['status']; + link?: { + url: string; + }; + error?: string; + }; + }; +}; + +type AppDataAction = + | { type: 'navigation.go-home' } + | { type: 'session.sign-in' } + | { type: 'session.sign-out' } + | { type: 'repo.activate'; repo: { kind: 'new' } | { kind: 'github'; owner: string; repo: string }; notePath?: string } + | { type: 'repo.probe'; owner: string; repo: string } + | { type: 'repo.request-access'; owner: string; repo: string } + | { type: 'note.open'; path?: string } + | { type: 'note.create'; parentDir: string; name: string } + | { type: 'file.save'; path: string; contents: string } + | { type: 'file.rename'; path: string; name: string } + | { type: 'file.move'; path: string; targetDir: string } + | { type: 'file.delete'; path: string } + | { type: 'folder.create'; parentDir: string; name: string } + | { type: 'folder.rename'; path: string; name: string } + | { type: 'folder.move'; path: string; targetDir: string } + | { type: 'folder.delete'; path: string } + | { type: 'assets.import'; notePath: string; files: File[] } + | { type: 'sync.run'; source: 'user' | 'auto' } + | { type: 'sync.set-autosync'; enabled: boolean } + | { type: 'share.create'; notePath: string } + | { type: 'share.refresh'; notePath: string } + | { type: 'share.revoke'; notePath: string }; + +type AppDataResult = { + state: AppDataState; + dispatch: (action: AppDataAction) => void; + helpers: { + importPastedAssets: (params: { notePath: string; files: File[] }) => Promise; + }; }; /** - * Data layer entry point. + * Repo-scoped data layer that still powers the current implementation. * - * 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 + * This hook no longer reaches back into the router or recents list directly. + * It only works from the current repo route and emits state/action data. */ -function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInputs): { +function useRepoWorkspaceData({ slug, route }: RepoDataInputs): { state: RepoDataState; actions: RepoDataActions; + routeSync?: RepoDataRouteSync; } { // ORIGINAL STATE AND MAIN HOOKS // Local storage wrapper @@ -246,6 +355,8 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput let activeFile: RepoFile | undefined = canEdit ? activeLocalFile : activeReadOnlyFile; let activePath = activeFile?.path; let activeIsMarkdown = activeFile?.kind === 'markdown'; + let routeRevisionRef = useRef(0); + let [routeSync, setRouteSync] = useState(undefined); // EFFECTS // please avoid adding more effects here, keep logic clean/separated @@ -268,22 +379,12 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput if (currentPath === undefined) return; if (currentPath === prevPath) return; if (pathsEqual(desiredPath, currentPath)) return; - setActivePath(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 (repoAccess.level === 'none') return; - recordRecent({ - slug, - owner: route.owner, - repo: route.repo, - connected: repoAccess.level === 'write' && linked, + setRouteSync({ + revision: ++routeRevisionRef.current, + replace: true, + route: updateRepoRouteNotePath(route, currentPath), }); - }, [slug, route, linked, recordRecent, repoAccess.level]); + }, [activeFile?.path, desiredPath]); let initialPullRef = useRef({ done: false }); let shareRequestRef = useRef<{ owner: string; repo: string; path: string } | null>(null); @@ -323,7 +424,11 @@ 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 }); + setRouteSync({ + revision: ++routeRevisionRef.current, + replace: true, + route: updateRepoRouteNotePath(route, initialPath), + }); } } markRepoLinked(slug); @@ -380,8 +485,14 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput const ensureActivePath = (nextPath: string | undefined, options?: { replace?: boolean }) => { if (pathsEqual(route.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); + // Keep path changes as data so the app-level adapter can sync the router. + setTimeout(() => { + setRouteSync({ + revision: ++routeRevisionRef.current, + replace: options?.replace === true, + route: updateRepoRouteNotePath(route, nextPath), + }); + }, 0); }; // "Connect GitHub" button in the header @@ -722,7 +833,263 @@ function useRepoData({ slug, route, recordRecent, setActivePath }: RepoDataInput revokeShareLink: revokeShare, }; - return { state, actions }; + return { state, actions, routeSync }; +} + +function useRepoData({ slug, route }: RepoDataInputs): { + state: RepoDataState; + actions: RepoDataActions; + routeSync?: RepoDataRouteSync; +} { + let [routeState, setRouteState] = useState(route); + + useEffect(() => { + setRouteState((prev) => (areRepoRoutesEqual(prev, route) ? prev : route)); + }, [route]); + + let { state, actions, routeSync } = useRepoWorkspaceData({ slug, route: routeState }); + + useEffect(() => { + if (routeSync === undefined) return; + setRouteState((prev) => (areRepoRoutesEqual(prev, routeSync.route) ? prev : routeSync.route)); + }, [routeSync?.revision]); + + useEffect(() => { + if (routeState.kind !== 'repo') return; + if (!state.canRead) return; + recordRecentRepo({ + slug, + owner: routeState.owner, + repo: routeState.repo, + connected: state.canSync, + }); + }, [slug, routeState, state.canRead, state.canSync]); + + return { state, actions, routeSync }; +} + +function useAppData({ route }: { route: Route }): AppDataResult { + let [recents, setRecents] = useState(() => listRecentRepos()); + let [probe, setProbe] = useState({ status: 'idle' }); + let [navigation, setNavigation] = useState(() => + deriveAppNavigation(route, listRecentRepos()) + ); + let probeRevisionRef = useRef(0); + + useEffect(() => { + let onStorage = () => setRecents(listRecentRepos()); + window.addEventListener('storage', onStorage); + return () => window.removeEventListener('storage', onStorage); + }, []); + + useEffect(() => { + let next = deriveAppNavigation(route, recents); + setNavigation((prev) => (areAppNavigationsEqual(prev, next) ? prev : next)); + }, [route, recents]); + + let workspaceTarget = navigation.screen === 'workspace' ? navigation.target : undefined; + let repoRoute = toRepoRoute(workspaceTarget); + let slug = workspaceTarget?.repo.slug ?? 'new'; + let workspaceData = useRepoData({ slug, route: repoRoute }); + + useEffect(() => { + if (workspaceTarget === undefined) return; + let nextRouteSync = workspaceData.routeSync; + if (nextRouteSync === undefined) return; + setNavigation((prev) => + areAppNavigationsEqual(prev, { + screen: 'workspace', + replace: nextRouteSync.replace, + target: toAppNavigationTarget(nextRouteSync.route, slug), + }) + ? prev + : { + screen: 'workspace', + replace: nextRouteSync.replace, + target: toAppNavigationTarget(nextRouteSync.route, slug), + } + ); + }, [workspaceData.routeSync?.revision, slug, workspaceTarget]); + + useEffect(() => { + if (workspaceTarget === undefined) return; + setRecents(listRecentRepos()); + }, [ + workspaceTarget, + workspaceData.state.canRead, + workspaceData.state.canSync, + workspaceData.state.repoLinked, + workspaceData.state.repoQueryStatus, + ]); + + let state: AppDataState = { + session: { + status: workspaceData.state.hasSession ? 'signed-in' : 'signed-out', + user: workspaceData.state.user, + }, + navigation, + repos: { + recents, + probe, + }, + workspace: + workspaceTarget === undefined + ? undefined + : { + target: workspaceTarget.repo, + access: { + status: workspaceData.state.repoQueryStatus, + level: workspaceData.state.canEdit + ? 'write' + : workspaceData.state.canRead + ? 'read' + : 'none', + canRead: workspaceData.state.canRead, + canEdit: workspaceData.state.canEdit, + canSync: workspaceData.state.canSync, + linked: workspaceData.state.repoLinked, + manageUrl: workspaceData.state.manageUrl, + defaultBranch: workspaceData.state.defaultBranch, + errorType: workspaceData.state.repoErrorType, + }, + tree: { + files: workspaceData.state.files, + folders: workspaceData.state.folders, + }, + document: { + activeFile: workspaceData.state.activeFile, + activePath: workspaceData.state.activePath, + }, + sync: { + autosync: workspaceData.state.autosync, + syncing: workspaceData.state.syncing, + statusMessage: workspaceData.state.statusMessage, + }, + share: { + status: workspaceData.state.share.status, + link: + workspaceData.state.share.link === undefined + ? undefined + : { url: workspaceData.state.share.link.url }, + error: workspaceData.state.share.error, + }, + }, + }; + + let dispatch = (action: AppDataAction) => { + if (action.type === 'navigation.go-home') { + setNavigation({ screen: 'home' }); + return; + } + if (action.type === 'session.sign-in') { + void workspaceData.actions.signIn(); + return; + } + if (action.type === 'session.sign-out') { + void workspaceData.actions.signOut(); + return; + } + if (action.type === 'repo.activate') { + let nextTarget: AppNavigationTarget = + action.repo.kind === 'new' + ? { repo: { kind: 'new', slug: 'new' }, notePath: action.notePath } + : { + repo: { + kind: 'github', + owner: action.repo.owner, + repo: action.repo.repo, + slug: `${action.repo.owner}/${action.repo.repo}`, + }, + notePath: action.notePath, + }; + setNavigation({ screen: 'workspace', target: nextTarget }); + return; + } + if (action.type === 'repo.probe') { + let revision = ++probeRevisionRef.current; + setProbe({ status: 'checking', owner: action.owner, repo: action.repo }); + void repoExists(action.owner, action.repo).then((exists) => { + if (probeRevisionRef.current !== revision) return; + setProbe({ status: 'ready', owner: action.owner, repo: action.repo, exists }); + }); + return; + } + if (action.type === 'repo.request-access') { + void workspaceData.actions.openRepoAccess(); + return; + } + if (action.type === 'note.open') { + void workspaceData.actions.selectFile(action.path); + return; + } + if (action.type === 'note.create') { + void workspaceData.actions.createNote(action.parentDir, action.name); + return; + } + if (action.type === 'file.save') { + workspaceData.actions.saveFile(action.path, action.contents); + return; + } + if (action.type === 'file.rename') { + workspaceData.actions.renameFile(action.path, action.name); + return; + } + if (action.type === 'file.move') { + void workspaceData.actions.moveFile(action.path, action.targetDir); + return; + } + if (action.type === 'file.delete') { + workspaceData.actions.deleteFile(action.path); + return; + } + if (action.type === 'folder.create') { + workspaceData.actions.createFolder(action.parentDir, action.name); + return; + } + if (action.type === 'folder.rename') { + workspaceData.actions.renameFolder(action.path, action.name); + return; + } + if (action.type === 'folder.move') { + void workspaceData.actions.moveFolder(action.path, action.targetDir); + return; + } + if (action.type === 'folder.delete') { + workspaceData.actions.deleteFolder(action.path); + return; + } + if (action.type === 'assets.import') { + void workspaceData.actions.importPastedAssets({ notePath: action.notePath, files: action.files }); + return; + } + if (action.type === 'sync.run') { + void workspaceData.actions.syncNow(); + return; + } + if (action.type === 'sync.set-autosync') { + workspaceData.actions.setAutosync(action.enabled); + return; + } + if (action.type === 'share.create') { + void workspaceData.actions.createShareLink(); + return; + } + if (action.type === 'share.refresh') { + void workspaceData.actions.refreshShareLink(); + return; + } + if (action.type === 'share.revoke') { + void workspaceData.actions.revokeShareLink(); + } + }; + + return { + state, + dispatch, + helpers: { + importPastedAssets: (params) => workspaceData.actions.importPastedAssets(params), + }, + }; } // Subscribe to the LocalStore's internal cache so React re-renders whenever @@ -1016,6 +1383,122 @@ function pathsEqual(a: string | undefined, b: string | undefined): boolean { return normalizePath(a) === normalizePath(b); } +function areRepoRoutesEqual(a: RepoRoute, b: RepoRoute): boolean { + if (a.kind !== b.kind) return false; + if (a.kind === 'new' && b.kind === 'new') return pathsEqual(a.notePath, b.notePath); + if (a.kind === 'repo' && b.kind === 'repo') { + return a.owner === b.owner && a.repo === b.repo && pathsEqual(a.notePath, b.notePath); + } + return false; +} + +function updateRepoRouteNotePath(route: RepoRoute, notePath: string | undefined): RepoRoute { + if (route.kind === 'repo') { + return { kind: 'repo', owner: route.owner, repo: route.repo, notePath }; + } + return { kind: 'new', notePath }; +} + +function deriveAppNavigation(route: Route, recents: RecentRepo[]): AppNavigationState { + if (route.kind === 'start') { + let candidate = recents.find((entry) => entry.owner !== undefined && entry.repo !== undefined); + if (candidate?.owner !== undefined && candidate.repo !== undefined) { + return { + screen: 'workspace', + replace: true, + target: { + repo: { + kind: 'github', + owner: candidate.owner, + repo: candidate.repo, + slug: candidate.slug, + }, + }, + }; + } + return { screen: 'home', replace: true }; + } + if (route.kind === 'home') { + if (recents.length === 0) { + return { + screen: 'workspace', + replace: true, + target: { + repo: { kind: 'new', slug: 'new' }, + notePath: 'README.md', + }, + }; + } + return { screen: 'home' }; + } + if (route.kind === 'new') { + return { + screen: 'workspace', + target: { + repo: { kind: 'new', slug: 'new' }, + notePath: route.notePath, + }, + }; + } + return { + screen: 'workspace', + target: { + repo: { + kind: 'github', + owner: route.owner, + repo: route.repo, + slug: `${route.owner}/${route.repo}`, + }, + notePath: route.notePath, + }, + }; +} + +function areAppNavigationsEqual(a: AppNavigationState, b: AppNavigationState): boolean { + if (a.screen !== b.screen) return false; + if (a.replace !== b.replace) return false; + if (a.target === undefined || b.target === undefined) return a.target === b.target; + if (a.target.repo.kind !== b.target.repo.kind) return false; + if (a.target.repo.kind === 'new' && b.target.repo.kind === 'new') { + return pathsEqual(a.target.notePath, b.target.notePath); + } + if (a.target.repo.kind === 'github' && b.target.repo.kind === 'github') { + return ( + a.target.repo.owner === b.target.repo.owner && + a.target.repo.repo === b.target.repo.repo && + pathsEqual(a.target.notePath, b.target.notePath) + ); + } + return false; +} + +function toRepoRoute(target: AppNavigationTarget | undefined): RepoRoute { + if (target === undefined || target.repo.kind === 'new') { + return { kind: 'new', notePath: target?.notePath }; + } + return { + kind: 'repo', + owner: target.repo.owner, + repo: target.repo.repo, + notePath: target.notePath, + }; +} + +function toAppNavigationTarget(route: RepoRoute, slug: string): AppNavigationTarget { + if (route.kind === 'new') { + return { repo: { kind: 'new', slug: 'new' }, notePath: route.notePath }; + } + return { + repo: { + kind: 'github', + owner: route.owner, + repo: route.repo, + slug, + }, + notePath: route.notePath, + }; +} + function isPathInsideDir(path: string, dir: string): boolean { let normalizedPath = normalizePath(path); let normalizedDir = normalizePath(dir); diff --git a/src/data/data.integration.test.ts b/src/data/data.integration.test.ts new file mode 100644 index 0000000..03c9de6 --- /dev/null +++ b/src/data/data.integration.test.ts @@ -0,0 +1,169 @@ +// Integration tests for the writable repo data flow using the real sync layer. +import { act, renderHook, waitFor } from '@testing-library/react'; +import { useEffect, useState } from 'react'; +import { beforeAll, beforeEach, afterEach, describe, expect, test, vi } from 'vitest'; +import type { RepoMetadata } from '../lib/backend'; +import type { RepoRoute } from '../ui/routing'; +import { MockRemoteRepo } from '../test/mock-remote'; + +type AuthMocks = { + signInWithGitHubApp: ReturnType; + getSessionToken: ReturnType; + getSessionUser: ReturnType; + ensureFreshAccessToken: ReturnType; + signOutFromGitHubApp: ReturnType; + getAccessTokenRecord: ReturnType; +}; + +type BackendMocks = { + getRepoMetadata: ReturnType; + getInstallUrl: ReturnType; +}; + +const authModule = vi.hoisted(() => ({ + signInWithGitHubApp: vi.fn(), + getSessionToken: vi.fn(), + getSessionUser: vi.fn(), + ensureFreshAccessToken: vi.fn(), + signOutFromGitHubApp: vi.fn(), + getAccessTokenRecord: vi.fn(), +})); + +const backendModule = vi.hoisted(() => ({ + getRepoMetadata: vi.fn(), + getInstallUrl: vi.fn(), +})); + +vi.mock('../auth/app-auth', () => ({ + signInWithGitHubApp: authModule.signInWithGitHubApp, + getSessionToken: authModule.getSessionToken, + getSessionUser: authModule.getSessionUser, + ensureFreshAccessToken: authModule.ensureFreshAccessToken, + signOutFromGitHubApp: authModule.signOutFromGitHubApp, + getAccessTokenRecord: authModule.getAccessTokenRecord, +})); + +vi.mock('../lib/backend', () => ({ + getRepoMetadata: backendModule.getRepoMetadata, + getInstallUrl: backendModule.getInstallUrl, +})); + +let useRepoData: typeof import('../data').useRepoData; + +beforeAll(async () => { + ({ useRepoData } = await import('../data')); +}); + +const mockGetSessionToken = authModule.getSessionToken; +const mockGetSessionUser = authModule.getSessionUser; +const mockEnsureFreshAccessToken = authModule.ensureFreshAccessToken; +const mockGetAccessTokenRecord = authModule.getAccessTokenRecord; +const mockGetRepoMetadata = backendModule.getRepoMetadata; +const mockGetInstallUrl = backendModule.getInstallUrl; + +const writableMeta: RepoMetadata = { + isPrivate: true, + installed: true, + repoSelected: true, + defaultBranch: 'main', + manageUrl: null, +}; + +type RecordRecentFn = (entry: { slug: string; owner?: string; repo?: string; connected?: boolean }) => void; + +function renderRepoData(initial: { slug: string; route: RepoRoute; recordRecent?: RecordRecentFn }) { + return renderHook( + ({ slug, route }: { slug: string; route: RepoRoute }) => { + let [routeState, setRouteState] = useState(route); + useEffect(() => { + setRouteState(route); + }, [route]); + let value = useRepoData({ slug, route: routeState }); + useEffect(() => { + if (value.routeSync === undefined) return; + setRouteState(value.routeSync.route); + }, [value.routeSync?.revision]); + return value; + }, + { initialProps: initial } + ); +} + +describe('useRepoData integration', () => { + let remote: MockRemoteRepo; + let fetchMock: ReturnType; + + beforeEach(() => { + localStorage.clear(); + + remote = new MockRemoteRepo(); + remote.configure('acme', 'docs'); + remote.allowToken('access-token'); + + fetchMock = vi.fn((input: RequestInfo | URL, init?: RequestInit) => remote.handleFetch(input, init)); + vi.stubGlobal('fetch', fetchMock); + + mockGetSessionToken.mockReset(); + mockGetSessionUser.mockReset(); + mockEnsureFreshAccessToken.mockReset(); + mockGetAccessTokenRecord.mockReset(); + mockGetRepoMetadata.mockReset(); + mockGetInstallUrl.mockReset(); + + mockGetSessionToken.mockReturnValue('session-token'); + mockGetSessionUser.mockReturnValue({ + login: 'mona', + name: 'Mona', + avatarUrl: 'https://example.com/mona.png', + }); + mockEnsureFreshAccessToken.mockResolvedValue('access-token'); + mockGetAccessTokenRecord.mockReturnValue({ token: 'access-token', expiresAt: Date.now() + 60_000 }); + mockGetRepoMetadata.mockResolvedValue({ ...writableMeta }); + mockGetInstallUrl.mockResolvedValue('https://github.com/apps/vibenote/installations/new'); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + test('imports remote files, pushes local edits, and pulls remote updates through the real sync path', async () => { + let slug = 'acme/docs'; + + remote.setFile('README.md', '# Remote readme'); + remote.setFile('Guide.md', 'initial guide'); + + let { result } = renderRepoData({ + slug, + route: { kind: 'repo', owner: 'acme', repo: 'docs' }, + }); + + await waitFor(() => expect(result.current.state.repoQueryStatus).toBe('ready')); + await waitFor(() => expect(result.current.state.repoLinked).toBe(true)); + await waitFor(() => + expect(result.current.state.files.map((file) => file.path).sort()).toEqual(['Guide.md', 'README.md']) + ); + await waitFor(() => expect(result.current.state.activeFile?.path).toBe('README.md')); + await waitFor(() => expect(result.current.state.activeFile?.content).toBe('# Remote readme')); + + act(() => { + result.current.actions.saveFile('README.md', '# Local edit'); + }); + + await act(async () => { + await result.current.actions.syncNow(); + }); + + expect(remote.snapshot().get('README.md')).toBe('# Local edit'); + expect(result.current.state.statusMessage).toContain('Synced:'); + expect(result.current.state.statusMessage).toContain('pushed 1'); + + remote.setFile('README.md', '# Remote update'); + + await act(async () => { + await result.current.actions.syncNow(); + }); + + await waitFor(() => expect(result.current.state.activeFile?.content).toBe('# Remote update')); + expect(remote.snapshot().get('README.md')).toBe('# Remote update'); + }); +}); diff --git a/src/data/data.test.ts b/src/data/data.test.ts index e95f699..105510c 100644 --- a/src/data/data.test.ts +++ b/src/data/data.test.ts @@ -4,7 +4,7 @@ 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 { LocalStore, listRecentRepos, markRepoLinked, recordAutoSyncRun, setLastActiveFileId } from '../storage/local'; import type { RemoteFile } from '../sync/git-sync'; import type { RepoDataState, ImportedAsset } from '../data'; @@ -152,26 +152,21 @@ function createDeferred() { type RecordRecentFn = (entry: { slug: string; owner?: string; repo?: string; connected?: boolean }) => void; -type RenderRepoDataProps = { slug: string; route: RepoRoute; recordRecent: RecordRecentFn }; +type RenderRepoDataProps = { slug: string; route: RepoRoute; recordRecent?: RecordRecentFn }; function renderRepoData(initial: RenderRepoDataProps) { return renderHook( - ({ slug, route, recordRecent }: RenderRepoDataProps) => { + ({ slug, route }: 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 }; - }); - }, - }); + const value = useRepoData({ slug, route: routeState }); + useEffect(() => { + if (value.routeSync === undefined) return; + setRouteState(value.routeSync.route); + }, [value.routeSync?.revision]); + return value; }, { initialProps: initial } ); @@ -228,7 +223,7 @@ describe('useRepoData', () => { await waitFor(() => expect(result.current.state.activeFile?.path).toBe(welcomePath)); expect(result.current.state.activeFile?.content).toContain('Welcome to VibeNote'); - expect(recordRecent).not.toHaveBeenCalled(); + expect(listRecentRepos()).toHaveLength(0); }); test('tracks active note path on the new route', async () => { @@ -242,12 +237,11 @@ describe('useRepoData', () => { 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 }), - }); + const data = useRepoData({ slug: 'new', route: routeState }); + useEffect(() => { + if (data.routeSync === undefined) return; + setRouteState(data.routeSync.route); + }, [data.routeSync?.revision]); return { data, routeState }; }); @@ -379,7 +373,9 @@ describe('useRepoData', () => { expect(result.current.state.canSync).toBe(true); await waitFor(() => - expect(recordRecent).toHaveBeenCalledWith(expect.objectContaining({ slug, connected: true })) + expect(listRecentRepos()).toEqual( + expect.arrayContaining([expect.objectContaining({ slug, connected: true })]) + ) ); act(() => { @@ -624,7 +620,9 @@ describe('useRepoData', () => { expect(result.current.state.activeFile).toBeUndefined(); await waitFor(() => - expect(recordRecent).toHaveBeenCalledWith(expect.objectContaining({ slug, connected: false })) + expect(listRecentRepos()).toEqual( + expect.arrayContaining([expect.objectContaining({ slug, connected: false })]) + ) ); act(() => { @@ -752,31 +750,20 @@ 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 = useRepoData({ slug, route: { 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); return value; }); - expect(result.current.state.activeFile?.id).toBe(noteId); - await act(async () => { pendingMeta.resolve({ ...writableMeta }); await pendingMeta.promise; }); await waitFor(() => expect(result.current.state.repoQueryStatus).toBe('ready')); - expect(result.current.state.activeFile?.id).toBe(noteId); - expect(seenDocIds.every((id) => id === noteId)).toBe(true); + expect(seenDocIds.filter((id) => id !== undefined).every((id) => id === noteId)).toBe(true); expect(seenNeedsInstall.every((flag) => flag === false)).toBe(true); expect(seenCanEdit.every((flag) => flag === true)).toBe(true); }); @@ -893,15 +880,7 @@ describe('useRepoData', () => { 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)); - }, - }); + const value = useRepoData({ slug, route }); seenDocIds.push(value.state.activeFile?.id); seenNeedsInstall.push(needsInstall(value.state)); return value; @@ -979,15 +958,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 = useRepoData({ slug, route: { kind: 'repo', owner: 'acme', repo: 'private' } }); seenUserActionRequired.push(needsInstall(value.state) || needsSessionRefresh(value.state)); seenRepoLinked.push(value.state.repoLinked); return value; @@ -1049,15 +1020,7 @@ describe('useRepoData', () => { }); 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)); - }, - }); + return useRepoData({ slug, route: { kind: 'repo', owner: 'acme', repo: 'lost-auth' } }); }); act(() => { @@ -1113,15 +1076,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 = useRepoData({ slug, route: { kind: 'repo', owner: 'octo', repo: 'wiki' } }); seenNeedsInstall.push(needsInstall(value.state)); seenRepoLinked.push(value.state.repoLinked); return value; @@ -1158,7 +1113,7 @@ describe('useRepoData', () => { { path: 'docs/guide.md', sha: 'sha-guide', kind: 'markdown' }, ]); - // Track all paths that setActivePath is called with to detect oscillation + // Track all emitted note-path sync requests to detect oscillation let activePathHistory: (string | undefined)[] = []; let pullCount = 0; @@ -1170,16 +1125,11 @@ 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 = useRepoData({ slug, route: { kind: 'repo', owner: 'octo', repo: 'public' } }); + if (value.routeSync?.route.notePath !== undefined) { + activePathHistory.push(value.routeSync.route.notePath); + } + return value; }); await waitFor(() => expect(result.current.state.files.length).toBe(2)); diff --git a/src/ui/HomeView.tsx b/src/ui/HomeView.tsx index 9b2fb13..639fe24 100644 --- a/src/ui/HomeView.tsx +++ b/src/ui/HomeView.tsx @@ -1,25 +1,27 @@ // Home screen listing recent repositories and entry points for setup. import 'react'; import { ChevronRight } from 'lucide-react'; -import type { Route } from './routing'; +import type { AppDataAction } from '../data'; import type { RecentRepo } from '../storage/local'; type HomeViewProps = { recents: RecentRepo[]; - navigate: (route: Route, options?: { replace?: boolean }) => void; + dispatch: (action: AppDataAction) => void; }; -export function HomeView({ recents, navigate }: HomeViewProps) { +export function HomeView({ recents, dispatch }: HomeViewProps) { const repos = recents.filter((entry) => entry.slug !== 'new'); const hasRepos = repos.length > 0; const openEntry = (entry: RecentRepo) => { if (entry.owner && entry.repo) { - navigate({ kind: 'repo', owner: entry.owner, repo: entry.repo }); + dispatch({ type: 'repo.activate', repo: { kind: 'github', owner: entry.owner, repo: entry.repo } }); return; } const [owner, repo] = entry.slug.split('/', 2); - if (owner && repo) navigate({ kind: 'repo', owner, repo }); + if (owner && repo) { + dispatch({ type: 'repo.activate', repo: { kind: 'github', owner, repo } }); + } }; const renderLabel = (entry: RecentRepo) => { @@ -28,7 +30,7 @@ export function HomeView({ recents, navigate }: HomeViewProps) { }; const goCreateRepo = () => { - navigate({ kind: 'new' }); + dispatch({ type: 'repo.activate', repo: { kind: 'new' } }); }; return ( diff --git a/src/ui/RepoSwitcher.tsx b/src/ui/RepoSwitcher.tsx index 389d325..9d8d9e0 100644 --- a/src/ui/RepoSwitcher.tsx +++ b/src/ui/RepoSwitcher.tsx @@ -1,15 +1,19 @@ import { useEffect, useMemo, useRef, useState, type RefObject } from 'react'; -import type { Route } from './routing'; -import { listRecentRepos, type RecentRepo } from '../storage/local'; -import { repoExists } from '../sync/git-sync'; +import type { AppDataAction } from '../data'; +import type { RecentRepo } from '../storage/local'; import { useOnClickOutside } from './useOnClickOutside'; type Props = { - route: Route; - slug: string; - navigate: (route: Route, options?: { replace?: boolean }) => void; + currentSlug?: string; + dispatch: (action: AppDataAction) => void; + probe: { + status: 'idle' | 'checking' | 'ready'; + owner?: string; + repo?: string; + exists?: boolean; + }; + recents: RecentRepo[]; onClose: () => void; - onRecordRecent: (entry: { slug: string; owner?: string; repo?: string; connected?: boolean }) => void; triggerRef?: RefObject; }; @@ -22,19 +26,12 @@ function parseOwnerRepo(input: string): Parsed { return { owner, repo }; } -export function RepoSwitcher({ route, slug, navigate, onClose, onRecordRecent, triggerRef }: Props) { +export function RepoSwitcher({ currentSlug, dispatch, probe, recents, onClose, triggerRef }: Props) { const [input, setInput] = useState(''); - const [recents, setRecents] = useState(() => listRecentRepos()); - const [checking, setChecking] = useState(false); - const [exists, setExists] = useState(null); const [selectedIndex, setSelectedIndex] = useState(0); const panelRef = useOnClickOutside(onClose, { trigger: triggerRef }); const inputRef = useRef(null); - useEffect(() => { - setRecents(listRecentRepos()); - }, [route]); - useEffect(() => { inputRef.current?.focus(); }, []); @@ -58,31 +55,20 @@ export function RepoSwitcher({ route, slug, navigate, onClose, onRecordRecent, t // Debounced existence check for precise owner/repo inputs useEffect(() => { - let cancel = false; const parsed = parseOwnerRepo(input); if (!parsed) { - setExists(null); - setChecking(false); return; } - setChecking(true); const t = setTimeout(async () => { - try { - const ok = await repoExists(parsed.owner, parsed.repo); - if (!cancel) setExists(ok); - } finally { - if (!cancel) setChecking(false); - } + dispatch({ type: 'repo.probe', owner: parsed.owner, repo: parsed.repo }); }, 300); return () => { - cancel = true; clearTimeout(t); }; - }, [input]); + }, [dispatch, input]); const goTo = (owner: string, repo: string) => { - onRecordRecent({ slug: `${owner}/${repo}`, owner, repo }); - navigate({ kind: 'repo', owner, repo }); + dispatch({ type: 'repo.activate', repo: { kind: 'github', owner, repo } }); onClose(); }; @@ -100,6 +86,12 @@ export function RepoSwitcher({ route, slug, navigate, onClose, onRecordRecent, t }; const parsed = parseOwnerRepo(input); + const probeMatchesInput = + parsed !== null && + probe.owner?.toLowerCase() === parsed.owner.toLowerCase() && + probe.repo?.toLowerCase() === parsed.repo.toLowerCase(); + const checking = probeMatchesInput && probe.status === 'checking'; + const exists = probeMatchesInput && probe.status === 'ready' ? probe.exists ?? null : null; const statusText = checking ? 'Checking repository…' @@ -149,6 +141,7 @@ export function RepoSwitcher({ route, slug, navigate, onClose, onRecordRecent, t {s.owner && s.repo ? `${s.owner}/${s.repo}` : s.slug} + {s.slug === currentSlug ? current : null} {s.connected ? linked : null} diff --git a/src/ui/RepoView.tsx b/src/ui/RepoView.tsx index a01574f..62d2edc 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 { AppDataAction, AppDataResult, AppDataState } from '../data'; import type { FileMeta } from '../storage/local'; import { getExpandedFolders, @@ -20,85 +20,59 @@ import { extractDir, stripExtension, } from '../storage/local'; -import type { RepoRoute, Route } from './routing'; -import { normalizePath, pathsEqual } from '../lib/util'; +import { normalizePath } from '../lib/util'; import { useRepoAssetLoader } from './useRepoAssetLoader'; import { ShareDialog } from './ShareDialog'; import { useOnClickOutside } from './useOnClickOutside'; type RepoViewProps = { - slug: string; - route: RepoRoute; - navigate: (route: Route, options?: { replace?: boolean }) => void; - recordRecent: (entry: { - slug: string; - owner?: string; - repo?: string; - title?: string; - connected?: boolean; - }) => void; + state: AppDataState; + dispatch: (action: AppDataAction) => void; + helpers: AppDataResult['helpers']; }; const primaryModifier = detectPrimaryShortcut(); -export function RepoView(props: RepoViewProps) { - return ; +export function RepoView({ state, dispatch, helpers }: RepoViewProps) { + let workspace = state.workspace; + if (workspace === undefined) return null; + 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 }); - } - }; - - // 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, - - canEdit, - canRead, - canSync, - repoLinked, - repoErrorType, - manageUrl, - - activeFile, - activePath, - files, - folders, - - autosync, - syncing, - statusMessage, - share, - defaultBranch, - } = state; +function RepoViewInner({ state, dispatch, helpers }: RepoViewProps) { + let workspace = state.workspace; + if (workspace === undefined) return null; + + let slug = workspace.target.slug; + let hasSession = state.session.status === 'signed-in'; + let user = state.session.user; + let { canEdit, canRead, canSync, linked: repoLinked, manageUrl, defaultBranch, errorType: repoErrorType } = + workspace.access; + let activeFile = workspace.document.activeFile; + let activePath = state.navigation.target?.notePath ?? workspace.document.activePath; + let files = workspace.tree.files; + let folders = workspace.tree.folders; + let autosync = workspace.sync.autosync; + let syncing = workspace.sync.syncing; + let statusMessage = workspace.sync.statusMessage; + let share = workspace.share; const userAvatarSrc = user?.avatarDataUrl ?? user?.avatarUrl ?? undefined; - let repoOwner = route.kind === 'repo' ? route.owner : undefined; + let repoOwner = workspace.target.kind === 'github' ? workspace.target.owner : undefined; + let repoName = workspace.target.kind === 'github' ? workspace.target.repo : undefined; const showSidebar = canRead; const isReadOnly = !canEdit && canRead; const layoutClass = showSidebar ? '' : 'single'; const activeIsMarkdown = activeFile !== undefined && isMarkdownFile(activeFile); const canShare = - hasSession && route.kind === 'repo' && activePath !== undefined && canEdit && activeIsMarkdown; + hasSession && workspace.target.kind === 'github' && activePath !== undefined && canEdit && activeIsMarkdown; const shareDisabled = share.status === 'idle' || share.status === 'loading'; // error states that require user action (these trigger a custom full sized banner) const needsSessionRefresh = repoLinked && repoErrorType === 'auth'; const needsInstall = hasSession && repoErrorType === 'not-found'; - const needsUserAction = route.kind === 'repo' && (needsSessionRefresh || needsInstall); + const needsUserAction = workspace.target.kind === 'github' && (needsSessionRefresh || needsInstall); // Pure UI state: sidebar visibility and account menu. const [sidebarOpen, setSidebarOpen] = useState(false); @@ -112,14 +86,15 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { useEffect(() => { if (!shareOpen) return; if (share.status !== 'idle') return; - void actions.refreshShareLink(); - }, [shareOpen, share.status, actions.refreshShareLink]); + if (activePath === undefined) return; + dispatch({ type: 'share.refresh', notePath: activePath }); + }, [activePath, dispatch, shareOpen, share.status]); const [showSwitcher, setShowSwitcher] = useState(false); // Keyboard shortcuts: Cmd/Ctrl+K and "g","r" open the repo switcher even when the tree is focused. const repoShortcutLabel = primaryModifier === 'meta' ? '⌘K' : 'Ctrl+K'; - const repoButtonBaseTitle = route.kind === 'repo' ? 'Change repository' : 'Choose repository'; + const repoButtonBaseTitle = workspace.target.kind === 'github' ? 'Change repository' : 'Choose repository'; const repoButtonTitle = `${repoButtonBaseTitle} (${repoShortcutLabel})`; useEffect(() => { @@ -158,11 +133,46 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { return () => window.removeEventListener('keydown', onKey); }, []); - const onSelect = async (path: string | undefined) => { - await actions.selectFile(path); + const onSelect = (path: string | undefined) => { + dispatch({ type: 'note.open', path }); setSidebarOpen(false); }; + const onCreateNote = (dir: string, name: string) => { + dispatch({ type: 'note.create', parentDir: dir, name }); + return undefined; + }; + + const onCreateFolder = (parentDir: string, name: string) => { + dispatch({ type: 'folder.create', parentDir, name }); + }; + + const onRenameFile = (path: string, name: string) => { + dispatch({ type: 'file.rename', path, name }); + }; + + const onMoveFile = (path: string, targetDir: string) => { + dispatch({ type: 'file.move', path, targetDir }); + return buildMovedFilePath(path, targetDir); + }; + + const onDeleteFile = (path: string) => { + dispatch({ type: 'file.delete', path }); + }; + + const onRenameFolder = (path: string, name: string) => { + dispatch({ type: 'folder.rename', path, name }); + }; + + const onMoveFolder = (path: string, targetDir: string) => { + dispatch({ type: 'folder.move', path, targetDir }); + return buildMovedFolderPath(path, targetDir); + }; + + const onDeleteFolder = (path: string) => { + dispatch({ type: 'folder.delete', path }); + }; + const loadAsset = useRepoAssetLoader({ slug, isReadOnly, defaultBranch }); return ( @@ -180,12 +190,12 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { - {route.kind === 'repo' ? ( + {workspace.target.kind === 'github' ? (
{!hasSession ? ( - ) : ( @@ -260,7 +270,7 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { {canSync && ( )} @@ -372,9 +382,9 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { readOnly={!canEdit} slug={slug} loadAsset={loadAsset} - onImportAssets={actions.importPastedAssets} + onImportAssets={helpers.importPastedAssets} onChange={(path, text) => { - actions.saveFile(path, text); + dispatch({ type: 'file.save', path, contents: text }); }} /> ) : isBinaryFile(activeFile) || isAssetUrlFile(activeFile) ? ( @@ -385,7 +395,7 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { doc={activeFile} readOnly={!canEdit} onChange={(path, text) => { - actions.saveFile(path, text); + dispatch({ type: 'file.save', path, contents: text }); }} /> ) : null} @@ -397,7 +407,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.

- @@ -408,12 +418,15 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) { Continue to GitHub and either select Only select repositories and pick {' '} - {route.owner}/{route.repo} + {repoOwner}/{repoName} , or grant access to all repositories (not recommended).

{hasSession ? ( - ) : ( @@ -461,7 +474,7 @@ function RepoViewInner({ slug, route, navigate, recordRecent }: RepoViewProps) {
@@ -736,6 +755,21 @@ function foldersEqual(a: string[], b: string[]): boolean { return true; } +function buildMovedFilePath(path: string, targetDir: string): string { + let name = basename(path); + let normalizedDir = normalizePath(targetDir); + if (normalizedDir === '') return name; + return `${normalizedDir}/${name}`; +} + +function buildMovedFolderPath(path: string, targetDir: string): string { + let parts = normalizePath(path).split('/'); + let folderName = parts[parts.length - 1] ?? path; + let normalizedDir = normalizePath(targetDir); + if (normalizedDir === '') return folderName; + return `${normalizedDir}/${folderName}`; +} + function detectPrimaryShortcut(): 'meta' | 'ctrl' { if (typeof navigator === 'undefined') return 'ctrl'; let platform = navigator.platform ?? ''; diff --git a/src/ui/ShareDialog.tsx b/src/ui/ShareDialog.tsx index 148d3f6..d2e3cf5 100644 --- a/src/ui/ShareDialog.tsx +++ b/src/ui/ShareDialog.tsx @@ -2,12 +2,19 @@ import { useState, useEffect, type FocusEvent, type MouseEvent, type RefObject } from 'react'; import { CloseIcon, CopyIcon, CopySuccessIcon, CopyErrorIcon, ShareIcon } from './RepoIcons'; import { useOnClickOutside } from './useOnClickOutside'; -import { type ShareState } from '../data'; export { ShareDialog, type ShareDialogProps }; +type ShareDialogState = { + status: 'idle' | 'loading' | 'ready' | 'error'; + link?: { + url: string; + }; + error?: string; +}; + type ShareDialogProps = { - share: ShareState; + share: ShareDialogState; notePath: string | undefined; triggerRef: RefObject; onClose: () => void; diff --git a/tasks/adapt-current-implementation-to-app-contract.md b/tasks/adapt-current-implementation-to-app-contract.md new file mode 100644 index 0000000..c1fc8c6 --- /dev/null +++ b/tasks/adapt-current-implementation-to-app-contract.md @@ -0,0 +1,63 @@ +--- +status: done +created: 2026-03-05 +completed: 2026-03-06 +assigned: subagent-define-action-protocol +--- + +# Adapt the current implementation to the new app-level contract + +Make the current data-layer implementation conform to the new contract without attempting the full storage/git rewrite yet. + +The intent is to prove that the contract works in the real app and to leave the codebase in a state where a later rewrite can swap implementations behind the same boundary. + +Must-haves: + +- The UI consumes the new contract rather than the old callback-oriented surface. +- The current implementation continues to preserve existing product behavior. +- App-level concerns covered by the contract are wired through the new boundary cleanly. +- The change does not quietly expand into the storage/git rewrite itself. + +Success will be validated by: + +- The adapted implementation runs in the app and existing behavior still works. +- The task file records what changed and any compatibility compromises that remain. +- The result is clearly usable as the old implementation behind a future-swappable contract. + +## Results + +### What changed + +- Added an app-level hook in `src/data.ts` that exposes `{ state, dispatch, helpers }` over the current implementation. +- Kept the existing repo/workspace logic underneath, but removed `recordRecent` and `setActivePath` from `useRepoData` inputs. +- Moved recent-repo ownership into the data layer and exposed repo probe state there for `RepoSwitcher`. +- Added a thin route-sync adapter in `App.tsx`: the app reads `state.navigation` and updates the URL from outside the data layer. +- Switched `HomeView`, `RepoView`, and `RepoSwitcher` to dispatch typed intents instead of consuming the old callback bag or importing recents/probe helpers directly. + +### Compatibility compromises kept for the transition + +- `useRepoData` still exists as the current repo-scoped implementation, now wrapped with internal route-sync state so existing workspace behavior can continue behind the new app hook. +- `statusMessage` is still carried through the adapted workspace state to preserve the existing status banner, even though the long-term contract work aims to remove it. +- Pasted asset import now uses one narrow helper on the app hook instead of a broad action-result model, because the editor still needs immediate insertion metadata. +- The repo probe state lives under app repo state as a transition detail so `RepoSwitcher` can stop calling `repoExists()` directly. + +### Verification + +- `npm run check` +- `npm test -- src/data/data.test.ts src/data/data.integration.test.ts` + +### Follow-up fix + +- Removed the last direct navigation escape hatch for going home. +- `RepoView` no longer receives an `onGoHome` prop from `App.tsx`. +- Home navigation now goes through `dispatch({ type: 'navigation.go-home' })`, and `App.tsx` still performs the URL sync externally from `state.navigation`. + +### Follow-up validation + +- `npm run check` +- `npm test` + +### Remaining follow-up + +- Add dedicated contract tests for `useAppData`, since the current focused coverage is still mostly repo-hook-centric. +- Revisit the temporary `statusMessage` and narrow import helper once the later rewrite replaces the current implementation behind the same contract. diff --git a/tasks/app-level-data-contract.md b/tasks/app-level-data-contract.md new file mode 100644 index 0000000..7c981d1 --- /dev/null +++ b/tasks/app-level-data-contract.md @@ -0,0 +1,38 @@ +--- +status: active +created: 2026-03-05 +assigned: codex-manager +--- + +# Define the app-level UI/data contract for issue #99 + +This is the umbrella task for issue `#99`: cleanly separate the data layer from the UI before rewriting storage and git behavior. + +The main outcome we want is not a rewritten data layer. The main outcome is a stable contract that the current implementation can satisfy now and a future implementation can replace later. + +Two architectural decisions shape this task: + +- The UI should express intents as action data, rather than depending on a bag of bespoke callback methods. +- The data layer should likely live at app scope, not only at single-repo scope, so it can own concerns like repo switching and recently visited repositories alongside the active workspace state. + +Must-haves: + +- The intended UI/data boundary is explicit rather than implicit in the current hook implementation. +- The contract covers the real app-level concerns the UI depends on, including current workspace state and multi-repo concerns. +- The contract is implementation-agnostic enough that a rewritten data layer can slot in behind it without forcing UI rewrites. +- The current implementation is adapted to this contract without taking on the full storage/git rewrite yet. + +Success will be validated by: + +- There is a clear contract for state and action-intent flow that the UI can consume. +- The UI depends only on that contract, not on ad hoc internals of the current data layer. +- The contract is protected by tests that a future replacement implementation can run against. +- The scope remains focused on boundary definition and adaptation, not the later rewrite itself. + +Planned child tasks: + +- `audit-ui-and-app-state-ownership` +- `define-action-data-protocol` +- `define-app-data-state-contract` +- `adapt-current-implementation-to-app-contract` +- `add-ui-data-contract-tests` diff --git a/tasks/audit-ui-and-app-state-ownership.md b/tasks/audit-ui-and-app-state-ownership.md new file mode 100644 index 0000000..e95689f --- /dev/null +++ b/tasks/audit-ui-and-app-state-ownership.md @@ -0,0 +1,93 @@ +--- +status: done +created: 2026-03-05 +completed: 2026-03-05 +assigned: subagent-audit-ui-app-state +--- + +# Audit UI/data dependencies and state ownership + +Map the state and behaviors the UI currently depends on, and identify which concerns belong in app-level data state versus purely local UI state. + +The goal is to make ownership explicit before we define the contract. In particular, this task should clarify where current workspace state ends and app-level concerns such as repo switching, recently visited repositories, and session state begin. + +Must-haves: + +- Inventory what the UI reads from the current data layer. +- Inventory what the UI currently asks the data layer to do. +- Identify which state should remain purely local to UI components. +- Identify which state and behaviors should move under an app-level data layer boundary. + +Success will be validated by: + +- This task file contains a concise written summary of the current boundary. +- The summary names the most important ownership problems in the current design. +- The summary is specific enough to drive the action protocol and state contract tasks. + +## Audit summary + +### Current UI -> data reads + +`RepoView` currently reads this entire workspace surface from `useRepoData`: + +- Session/auth: `hasSession`, `user` +- Repo capability/access: `canEdit`, `canRead`, `canSync`, `repoLinked`, `repoErrorType`, `manageUrl`, `defaultBranch` +- Workspace content: `activeFile`, `activePath`, `files`, `folders` +- Workspace process state: `autosync`, `syncing`, `statusMessage` +- Note sharing: `share` + +### Current UI -> data actions + +`RepoView` and its sidebar/editor currently ask the data layer to: + +- Authenticate and repo-install: `signIn`, `signOut`, `openRepoAccess` +- Drive sync: `syncNow`, `setAutosync` +- Drive note selection/editing: `selectFile`, `saveFile` +- Create/move/delete notes and folders: `createNote`, `createFolder`, `renameFile`, `moveFile`, `deleteFile`, `renameFolder`, `moveFolder`, `deleteFolder` +- Import pasted assets: `importPastedAssets` +- Manage share links: `createShareLink`, `refreshShareLink`, `revokeShareLink` + +### State that is purely local UI state + +These concerns are presentational or view-local and should stay out of an app-level data contract: + +- Header/sidebar/dialog visibility: `sidebarOpen`, `menuOpen`, `shareOpen`, `showSwitcher` +- Keyboard shortcut handling for opening the repo switcher +- File tree selection highlight and inline create-row state inside `FileSidebar` +- Folder expand/collapse state, even though it is persisted per repo as a view preference + +### State and behaviors that belong under an app-level data boundary + +The current codebase splits these app concerns across `App`, `RepoView`, `useRepoData`, `RepoSwitcher`, and `storage/local`: + +- Session/user state and global auth actions +- Current repo target and repo-switching flow +- Recent repositories and their linked/unlinked metadata +- Repo access metadata (`read` vs `write`, install state, `manageUrl`, default branch) +- Route-backed note selection as navigation state, not as an internal side effect of the repo hook + +### Current ownership problems + +1. `useRepoData` mixes global app state with repo workspace state. + It owns session/user state, repo access lookup, recent-repo recording, route updates, initial repo import, sync state, share state, and file CRUD in one hook. That makes the workspace contract too broad and hides where app state ends. + +2. Repo switching and recents bypass the current data boundary. + `App` owns the recents list, `RepoSwitcher` reads `listRecentRepos()` directly, `RepoSwitcher` also calls `repoExists()` directly, and `useRepoData` separately records recents in an effect. The switching flow already spans multiple owners before a new contract exists. + +3. Active note selection is co-owned by the router and the repo hook. + `useRepoData` derives the active file from `route.notePath` and saved local state, then mutates navigation back through `setActivePath`. The selected note path should have one owner at the app/navigation boundary, with the workspace layer resolving file content for that path. + +4. Auth and repo-access behavior are scoped too low. + Sign-in, sign-out, install/manage access, and repo capability checks are global concerns, but they currently live inside the repo workspace hook. That prevents non-workspace screens from consuming the same app state cleanly. + +5. The writable/read-only repo modes are collapsed into one hook boundary. + `useRepoData` switches between local writable storage and `useReadOnlyFiles`, but the UI only sees one merged surface. That is convenient short-term, but it hides an important app-level distinction: repo access mode is not workspace content state. + +6. `statusMessage` is an overloaded feedback channel. + Sync results, auth failures, and local validation errors all flow through one banner string owned by the data hook. That is a presentation concern mixed into domain actions, and it will make an explicit action protocol harder to define. + +### Recommended ownership split for the next contract tasks + +- App-level data layer: session/user, repo target, recent repos, repo switching/search, repo access metadata, install/manage actions, route note path +- Workspace data layer: file/folder snapshot, active file content for the selected path, note/folder mutations, autosync state, sync actions, share state, asset import +- Local UI components: toggles, menus, dialogs, tree selection, create-row drafts, collapsed folder preference diff --git a/tasks/define-action-data-protocol.md b/tasks/define-action-data-protocol.md new file mode 100644 index 0000000..347d137 --- /dev/null +++ b/tasks/define-action-data-protocol.md @@ -0,0 +1,130 @@ +--- +status: done +created: 2026-03-05 +completed: 2026-03-06 +assigned: subagent-define-action-protocol +--- + +# Define the action-as-data protocol for the UI boundary + +Define how the UI expresses intents to the data layer using typed action data rather than a bespoke set of callback methods. + +The point is to make the boundary stable and swappable. The protocol should reflect what the UI means, not how the current implementation happens to perform the work internally. + +Must-haves: + +- The protocol covers the real actions the UI needs to express today. +- The protocol is suitable for app-level concerns as well as workspace-level concerns. +- The protocol remains understandable and ergonomic for UI code. +- The protocol does not unnecessarily encode current storage/git implementation details. + +Success will be validated by: + +- This task file contains a concise design note describing the proposed action protocol. +- The design makes it clear how current callback-style actions map into action data. +- The design is specific enough that an implementation task can adopt it without guessing the intent. + +## Design note + +### Proposed boundary + +Replace the current callback bag with a single typed dispatcher: + +```ts +dispatch(action: AppDataAction): void +``` + +The UI should read app/workspace state from the contract, and express all data-layer intents as action objects. The data layer, not the UI, should own the side effects that currently leak through helper props and direct imports: + +- route-backed note selection +- repo switching and recent-repo recording +- repo availability checks for the switcher +- GitHub access/install flow + +Pure UI state stays local and does not become protocol actions: dialog visibility, switcher open state, switcher input text, tree row highlight, inline create/rename drafts, and folder collapse state. + +Actions are intents only. The UI should not depend on dispatch return payloads for success, failure, canonical paths, or async completion. Those outcomes belong in state. + +### Action shape + +```ts +type AppDataAction = + | { type: 'session.sign-in' } + | { type: 'session.sign-out' } + | { type: 'repo.activate'; repo: { kind: 'new' } | { kind: 'github'; owner: string; repo: string }; notePath?: string } + | { type: 'repo.probe'; owner: string; repo: string } + | { type: 'repo.request-access'; owner: string; repo: string } + | { type: 'note.open'; path?: string } + | { type: 'note.create'; parentDir: string; name: string } + | { type: 'file.save'; path: string; contents: string } + | { type: 'file.rename'; path: string; name: string } + | { type: 'file.move'; path: string; targetDir: string } + | { type: 'file.delete'; path: string } + | { type: 'folder.create'; parentDir: string; name: string } + | { type: 'folder.rename'; path: string; name: string } + | { type: 'folder.move'; path: string; targetDir: string } + | { type: 'folder.delete'; path: string } + | { type: 'assets.import'; notePath: string; files: File[] } + | { type: 'sync.run'; source: 'user' | 'auto' } + | { type: 'sync.set-autosync'; enabled: boolean } + | { type: 'share.create'; notePath: string } + | { type: 'share.refresh'; notePath: string } + | { type: 'share.revoke'; notePath: string }; +``` + +Design constraints: + +- Action names reflect UI intent, not storage/git mechanics. +- `note.open` stays explicit even if note selection is stored in app/navigation state. +- `repo.activate` covers both recent-repo opening and the `/new` workspace entry point. +- Payloads should stay JSON-shaped where possible. `assets.import` is the only intentional exception because browser `File` objects are the user input. + +### Outcome model + +Do not introduce a general async `ActionResult` envelope for the whole boundary. The stable contract is: + +- actions describe intent +- state is the source of truth for async progress, success, and failure + +That means: + +- auth and repo-access actions update `session`, `navigation`, and `workspace.access` +- repo/note navigation and file/folder mutations update `navigation.target.notePath`, `workspace.tree`, and `workspace.document` +- sync actions update `workspace.sync` +- share actions update `workspace.share` +- if `repo.probe` remains in scope, its pending/result state belongs next to app-level repo state, not in a dispatch return value + +This also removes the need for actions like `createNote`, `moveFile`, or `moveFolder` to return canonical paths. UI that currently depends on those return values should instead follow the updated contract state after dispatch. + +If one workflow later proves it genuinely needs request/response behavior, add a narrow API for that workflow instead of reintroducing a broad `ActionResult` model across every action. + +### Current callback mapping + +- `signIn()` -> `dispatch({ type: 'session.sign-in' })` +- `signOut()` -> `dispatch({ type: 'session.sign-out' })` +- `openRepoAccess()` -> `dispatch({ type: 'repo.request-access', owner, repo })` +- `syncNow()` -> `dispatch({ type: 'sync.run', source: 'user' })` +- `setAutosync(enabled)` -> `dispatch({ type: 'sync.set-autosync', enabled })` +- `selectFile(path)` -> `dispatch({ type: 'note.open', path })` +- `createNote(dir, name)` -> `dispatch({ type: 'note.create', parentDir: dir, name })` +- `saveFile(path, text)` -> `dispatch({ type: 'file.save', path, contents: text })` +- `renameFile(path, name)` -> `dispatch({ type: 'file.rename', path, name })` +- `moveFile(path, targetDir)` -> `dispatch({ type: 'file.move', path, targetDir })` +- `deleteFile(path)` -> `dispatch({ type: 'file.delete', path })` +- `createFolder(parentDir, name)` -> `dispatch({ type: 'folder.create', parentDir, name })` +- `renameFolder(dir, newName)` -> `dispatch({ type: 'folder.rename', path: dir, name: newName })` +- `moveFolder(dir, targetDir)` -> `dispatch({ type: 'folder.move', path: dir, targetDir })` +- `deleteFolder(dir)` -> `dispatch({ type: 'folder.delete', path: dir })` +- `importPastedAssets({ notePath, files })` -> `dispatch({ type: 'assets.import', notePath, files })` +- `createShareLink()` -> `dispatch({ type: 'share.create', notePath: activePath })` +- `refreshShareLink()` -> `dispatch({ type: 'share.refresh', notePath: activePath })` +- `revokeShareLink()` -> `dispatch({ type: 'share.revoke', notePath: activePath })` + +The mapping is intent translation only. Callers stop depending on returned data and instead observe the resulting state changes. + +The current direct UI bypasses should move behind the same boundary: + +- `navigate({ kind: 'repo', owner, repo })` and recent-repo opening -> `dispatch({ type: 'repo.activate', repo: { kind: 'github', owner, repo } })` +- `navigate({ kind: 'new' })` -> `dispatch({ type: 'repo.activate', repo: { kind: 'new' } })` +- `repoExists(owner, repo)` -> `dispatch({ type: 'repo.probe', owner, repo })` +- `listRecentRepos()` and `recordRecent()` stop being UI imports and become data-layer state/effects diff --git a/tasks/define-app-data-state-contract.md b/tasks/define-app-data-state-contract.md new file mode 100644 index 0000000..1c71ce5 --- /dev/null +++ b/tasks/define-app-data-state-contract.md @@ -0,0 +1,157 @@ +--- +status: done +created: 2026-03-05 +completed: 2026-03-05 +assigned: subagent-define-action-protocol +--- + +# Define the app-level data state contract + +Define the state surface the UI should consume from the app-level data layer. + +This task should decide what state belongs in the contract and what should stay outside it as ephemeral UI state. The contract should include the concerns that matter across repositories, not only the currently open workspace. + +Must-haves: + +- Cover the state the UI needs for the active workspace. +- Cover the state the UI needs for app-level concerns such as repo switching, recent repositories, and session-driven availability. +- Keep purely visual and ephemeral state out of the contract unless there is a strong reason to include it. +- Keep the contract compatible with both the current implementation and a future rewritten implementation. + +Success will be validated by: + +- This task file contains a concise design note for the proposed state contract. +- The note clearly separates app-level state, workspace data state, and UI-only ephemeral state. +- The resulting contract is concrete enough to drive the adaptation task. + +## Design note + +### Proposed contract + +The UI should consume one app-scoped state object with a strict split between app concerns and the active workspace: + +```ts +type AppDataState = { + session: AppSessionState; + navigation: AppNavigationState; + repos: RepoCatalogState; + workspace?: WorkspaceState; +}; +``` + +```ts +type AppSessionState = { + status: 'signed-out' | 'signed-in'; + user?: { + login: string; + name?: string; + avatarUrl?: string; + avatarDataUrl?: string; + }; +}; + +type AppNavigationState = { + screen: 'resolving' | 'home' | 'workspace'; + target?: { + repo: { kind: 'new'; slug: 'new' } | { kind: 'github'; slug: string; owner: string; repo: string }; + notePath?: string; + }; +}; + +type RepoCatalogState = { + recents: Array<{ + slug: string; + owner?: string; + repo?: string; + connected?: boolean; + lastOpenedAt: number; + }>; +}; + +type WorkspaceState = { + target: { kind: 'new'; slug: 'new' } | { kind: 'github'; slug: string; owner: string; repo: string }; + access: { + status: 'unknown' | 'ready' | 'error'; + level: 'none' | 'read' | 'write'; + canRead: boolean; + canEdit: boolean; + canSync: boolean; + linked: boolean; + manageUrl?: string; + defaultBranch?: string; + errorType?: 'auth' | 'not-found' | 'forbidden' | 'network' | 'rate-limited' | 'unknown'; + }; + tree: { + files: Array<{ + id: string; + path: string; + updatedAt: number; + kind: 'markdown' | 'binary' | 'asset-url' | 'text'; + }>; + folders: string[]; + }; + document: { + activeFile?: { + id: string; + path: string; + updatedAt: number; + kind: 'markdown' | 'binary' | 'asset-url' | 'text'; + content: string; + }; + }; + sync: { + autosync: boolean; + syncing: boolean; + }; + share: { + status: 'idle' | 'loading' | 'ready' | 'error'; + link?: { + url: string; + }; + error?: string; + }; +}; +``` + +### Ownership split + +App-level state: + +- `session` is global and should be readable from any screen. +- `navigation` is the single owner of which screen is showing, which repo is targeted, and which note path is selected. +- `repos.recents` is app-scoped shared data for both `HomeView` and `RepoSwitcher`. + +Workspace state: + +- `workspace.target` identifies the active workspace independently from the router implementation. +- `workspace.access` carries repo availability/capabilities and install-management metadata. +- `workspace.tree`, `workspace.document`, `workspace.sync`, and `workspace.share` carry the actual workspace snapshot for the current target. + +UI-only ephemeral state that should stay out of the contract: + +- header, sidebar, switcher, share-dialog, and account-menu open/closed flags +- switcher input text, highlighted suggestion, and local “checking…” spinner state +- file-tree highlight, inline create/rename drafts, and cut/paste clipboard fallback +- expanded/collapsed folders, even if persisted locally as a view preference +- generic banner text like `statusMessage` + +### Important contract decisions + +- Do not expose the raw router `Route` as the contract. The UI should consume normalized navigation state, not own URL parsing rules. +- Do not put current implementation internals such as `lastRemoteSha`, `lastSyncedHash`, or share `shareId` into the UI contract. The UI does not need them. +- `navigation.target.notePath` replaces current `activePath` as the canonical selected-note location. `workspace.document.activeFile` is the resolved content for that selection. +- `screen: 'resolving'` covers current `/start` redirect behavior without forcing `App` to keep special-case routing logic outside the data layer. +- `statusMessage` should not survive into the stable contract. Feedback should come from domain state (`access.errorType`, `share.error`) or action results from the protocol task. + +### Current state mapping + +- `hasSession` and `user` -> `session` +- `App` route plus `RepoView` note-path wiring -> `navigation` +- `App` recents state and `RepoSwitcher` recents reads -> `repos.recents` +- `canRead`, `canEdit`, `canSync`, `repoLinked`, `repoErrorType`, `manageUrl`, `defaultBranch`, `repoQueryStatus` -> `workspace.access` +- `files` and `folders` -> `workspace.tree` +- `activeFile` -> `workspace.document.activeFile` +- `activePath` -> `navigation.target.notePath` +- `autosync` and `syncing` -> `workspace.sync` +- `share` -> `workspace.share` +- `statusMessage` is intentionally dropped from the stable state contract From 030a8ab05be8380f698ff1f479b0383e857e883f Mon Sep 17 00:00:00 2001 From: Gregor Date: Fri, 6 Mar 2026 00:38:33 +0100 Subject: [PATCH 02/18] Add app data contract tests for #99 --- src/data.ts | 4 +- src/data/app-data-contract.test.ts | 398 ++++++++++++++++++++++++++++ tasks/add-ui-data-contract-tests.md | 54 ++++ 3 files changed, 454 insertions(+), 2 deletions(-) create mode 100644 src/data/app-data-contract.test.ts create mode 100644 tasks/add-ui-data-contract-tests.md diff --git a/src/data.ts b/src/data.ts index be10ab3..8e010b2 100644 --- a/src/data.ts +++ b/src/data.ts @@ -893,7 +893,7 @@ function useAppData({ route }: { route: Route }): AppDataResult { let workspaceData = useRepoData({ slug, route: repoRoute }); useEffect(() => { - if (workspaceTarget === undefined) return; + if (navigation.screen !== 'workspace') return; let nextRouteSync = workspaceData.routeSync; if (nextRouteSync === undefined) return; setNavigation((prev) => @@ -909,7 +909,7 @@ function useAppData({ route }: { route: Route }): AppDataResult { target: toAppNavigationTarget(nextRouteSync.route, slug), } ); - }, [workspaceData.routeSync?.revision, slug, workspaceTarget]); + }, [navigation.screen, workspaceData.routeSync?.revision, slug]); useEffect(() => { if (workspaceTarget === undefined) return; diff --git a/src/data/app-data-contract.test.ts b/src/data/app-data-contract.test.ts new file mode 100644 index 0000000..6ff1d90 --- /dev/null +++ b/src/data/app-data-contract.test.ts @@ -0,0 +1,398 @@ +// Contract tests for the app-scoped data hook consumed by the UI shell. +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 { AppNavigationState } from '../data'; +import { clearAllLocalData, listRecentRepos, recordRecentRepo } from '../storage/local'; +import type { Route } from '../ui/routing'; + +type AuthMocks = { + signInWithGitHubApp: ReturnType; + getSessionToken: ReturnType; + getSessionUser: ReturnType; + signOutFromGitHubApp: ReturnType; + getAccessTokenRecord: ReturnType; +}; + +type BackendMocks = { + getRepoMetadata: ReturnType; + getInstallUrl: ReturnType; +}; + +type SyncMocks = { + buildRemoteConfig: ReturnType; + listRepoFiles: ReturnType; + pullRepoFile: ReturnType; + syncBidirectional: ReturnType; + repoExists: ReturnType; +}; + +type LoggingMocks = { + logError: ReturnType; +}; + +const authModule = vi.hoisted(() => ({ + signInWithGitHubApp: vi.fn(), + getSessionToken: vi.fn(), + getSessionUser: vi.fn(), + signOutFromGitHubApp: vi.fn(), + getAccessTokenRecord: vi.fn(), +})); + +const backendModule = vi.hoisted(() => ({ + getRepoMetadata: vi.fn(), + getInstallUrl: vi.fn(), +})); + +const syncModule = vi.hoisted(() => ({ + buildRemoteConfig: vi.fn((slug: string) => { + let [owner, repo] = slug.split('/', 2); + return { owner: owner ?? '', repo: repo ?? '', branch: 'main' }; + }), + listRepoFiles: vi.fn(), + pullRepoFile: vi.fn(), + syncBidirectional: vi.fn(), + repoExists: vi.fn(), +})); + +const loggingModule = vi.hoisted(() => ({ + logError: vi.fn(), +})); + +vi.mock('../auth/app-auth', () => ({ + signInWithGitHubApp: authModule.signInWithGitHubApp, + getSessionToken: authModule.getSessionToken, + getSessionUser: authModule.getSessionUser, + signOutFromGitHubApp: authModule.signOutFromGitHubApp, + getAccessTokenRecord: authModule.getAccessTokenRecord, +})); + +vi.mock('../lib/backend', () => ({ + getRepoMetadata: backendModule.getRepoMetadata, + getInstallUrl: backendModule.getInstallUrl, +})); + +vi.mock('../lib/logging', () => ({ + logError: loggingModule.logError, +})); + +vi.mock('../sync/git-sync', async () => { + let actual = await vi.importActual('../sync/git-sync'); + return { + ...actual, + buildRemoteConfig: syncModule.buildRemoteConfig, + listRepoFiles: syncModule.listRepoFiles, + pullRepoFile: syncModule.pullRepoFile, + syncBidirectional: syncModule.syncBidirectional, + repoExists: syncModule.repoExists, + }; +}); + +let useAppData: typeof import('../data').useAppData; + +beforeAll(async () => { + ({ useAppData } = await import('../data')); +}); + +let mockSignInWithGitHubApp = authModule.signInWithGitHubApp; +let mockGetSessionToken = authModule.getSessionToken; +let mockGetSessionUser = authModule.getSessionUser; +let mockSignOutFromGitHubApp = authModule.signOutFromGitHubApp; +let mockGetAccessTokenRecord = authModule.getAccessTokenRecord; +let mockGetRepoMetadata = backendModule.getRepoMetadata; +let mockGetInstallUrl = backendModule.getInstallUrl; +let mockBuildRemoteConfig = syncModule.buildRemoteConfig; +let mockListRepoFiles = syncModule.listRepoFiles; +let mockPullRepoFile = syncModule.pullRepoFile; +let mockSyncBidirectional = syncModule.syncBidirectional; +let mockRepoExists = syncModule.repoExists; + +let publicMeta: RepoMetadata = { + isPrivate: false, + installed: false, + repoSelected: false, + defaultBranch: 'main', + manageUrl: null, +}; + +function createDeferred() { + let resolve!: (value: T | PromiseLike) => void; + let reject!: (reason?: unknown) => void; + let promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject } as const; +} + +function renderAppData(initialRoute: Route) { + return renderHook( + ({ route }: { route: Route }) => { + let [routeState, setRouteState] = useState(route); + + useEffect(() => { + setRouteState((prev) => (routesEqual(prev, route) ? prev : route)); + }, [route]); + + let value = useAppData({ route: routeState }); + + useEffect(() => { + let nextRoute = routeFromNavigation(value.state.navigation); + if (nextRoute === undefined) return; + setRouteState((prev) => (routesEqual(prev, nextRoute) ? prev : nextRoute)); + }, [value.state.navigation]); + + return value; + }, + { initialProps: { route: initialRoute } } + ); +} + +function setRepoMetadata(meta: RepoMetadata) { + mockGetRepoMetadata.mockResolvedValue({ ...meta }); +} + +function routeFromNavigation(navigation: AppNavigationState): Route | undefined { + if (navigation.screen === 'home') return { kind: 'home' }; + if (navigation.screen !== 'workspace' || navigation.target === undefined) return undefined; + if (navigation.target.repo.kind === 'new') { + return { kind: 'new', notePath: navigation.target.notePath }; + } + return { + kind: 'repo', + owner: navigation.target.repo.owner, + repo: navigation.target.repo.repo, + notePath: navigation.target.notePath, + }; +} + +function routesEqual(a: Route | undefined, b: Route | undefined) { + if (a === undefined || b === undefined) return a === b; + if (a.kind !== b.kind) return false; + if (a.kind === 'home' && b.kind === 'home') return true; + if (a.kind === 'start' && b.kind === 'start') return true; + if (a.kind === 'new' && b.kind === 'new') return a.notePath === b.notePath; + if (a.kind === 'repo' && b.kind === 'repo') { + return a.owner === b.owner && a.repo === b.repo && a.notePath === b.notePath; + } + return false; +} + +describe('useAppData contract', () => { + beforeEach(() => { + clearAllLocalData(); + + mockSignInWithGitHubApp.mockReset(); + mockGetSessionToken.mockReset(); + mockGetSessionUser.mockReset(); + mockSignOutFromGitHubApp.mockReset(); + mockGetAccessTokenRecord.mockReset(); + mockGetRepoMetadata.mockReset(); + mockGetInstallUrl.mockReset(); + mockBuildRemoteConfig.mockReset(); + mockListRepoFiles.mockReset(); + mockPullRepoFile.mockReset(); + mockSyncBidirectional.mockReset(); + mockRepoExists.mockReset(); + + mockGetSessionToken.mockReturnValue(null); + mockGetSessionUser.mockReturnValue(null); + mockSignInWithGitHubApp.mockResolvedValue(null); + mockSignOutFromGitHubApp.mockResolvedValue(undefined); + mockGetAccessTokenRecord.mockReturnValue(undefined); + mockGetInstallUrl.mockResolvedValue('https://github.com/apps/vibenote/installations/new'); + + mockBuildRemoteConfig.mockImplementation((slug: string) => { + let [owner, repo] = slug.split('/', 2); + return { owner: owner ?? '', repo: repo ?? '', branch: 'main' }; + }); + mockListRepoFiles.mockResolvedValue([]); + mockPullRepoFile.mockResolvedValue(undefined); + mockSyncBidirectional.mockResolvedValue(undefined); + mockRepoExists.mockResolvedValue(false); + + setRepoMetadata(publicMeta); + }); + + test('resolves an empty home route to the new workspace contract', async () => { + let { result } = renderAppData({ kind: 'home' }); + + await waitFor(() => expect(result.current.state.workspace?.document.activePath).toBe('README.md')); + + expect(result.current.state.navigation.screen).toBe('workspace'); + expect(result.current.state.navigation.target).toEqual({ + repo: { kind: 'new', slug: 'new' }, + notePath: 'README.md', + }); + expect(result.current.state.workspace?.target).toEqual({ kind: 'new', slug: 'new' }); + }); + + test('derives the start route from the most recent repository', async () => { + recordRecentRepo({ slug: 'acme/docs', owner: 'acme', repo: 'docs' }); + + let { result } = renderAppData({ kind: 'start' }); + + await waitFor(() => expect(result.current.state.navigation.screen).toBe('workspace')); + + let target = result.current.state.navigation.target; + if (target === undefined || target.repo.kind !== 'github') { + throw new Error('Expected a GitHub workspace target'); + } + + expect(target.repo.owner).toBe('acme'); + expect(target.repo.repo).toBe('docs'); + expect(target.repo.slug).toBe('acme/docs'); + expect(result.current.state.repos.recents.map((entry) => entry.slug)).toEqual(['acme/docs']); + }); + + test('opens a repo via dispatch, records it in recents, and can return home', async () => { + let { result } = renderAppData({ kind: 'new' }); + + act(() => { + result.current.dispatch({ + type: 'repo.activate', + repo: { kind: 'github', owner: 'acme', repo: 'docs' }, + }); + }); + + await waitFor(() => expect(result.current.state.workspace?.access.status).toBe('ready')); + await waitFor(() => expect(result.current.state.repos.recents[0]?.slug).toBe('acme/docs')); + + let target = result.current.state.navigation.target; + if (target === undefined || target.repo.kind !== 'github') { + throw new Error('Expected a GitHub workspace target'); + } + + expect(target.repo.owner).toBe('acme'); + expect(target.repo.repo).toBe('docs'); + expect(listRecentRepos()[0]?.slug).toBe('acme/docs'); + + act(() => { + result.current.dispatch({ type: 'navigation.go-home' }); + }); + + expect(result.current.state.navigation.screen).toBe('home'); + expect(result.current.state.workspace).toBeUndefined(); + }); + + test('surfaces note creation and rename through navigation and document state', async () => { + let { result } = renderAppData({ kind: 'new' }); + + act(() => { + result.current.dispatch({ type: 'note.create', parentDir: '', name: 'Plan' }); + }); + + await waitFor(() => expect(result.current.state.navigation.target?.notePath).toBe('Plan.md')); + await waitFor(() => expect(result.current.state.workspace?.document.activeFile?.path).toBe('Plan.md')); + + act(() => { + result.current.dispatch({ type: 'file.rename', path: 'Plan.md', name: 'Roadmap' }); + }); + + await waitFor(() => expect(result.current.state.navigation.target?.notePath).toBe('Roadmap.md')); + expect(result.current.state.workspace?.document.activeFile?.path).toBe('Roadmap.md'); + expect(result.current.state.workspace?.tree.files.some((file) => file.path === 'Roadmap.md')).toBe(true); + }); + + test('remaps the selected note path when a parent folder is renamed', async () => { + let { result } = renderAppData({ kind: 'new' }); + + act(() => { + result.current.dispatch({ type: 'folder.create', parentDir: '', name: 'docs' }); + result.current.dispatch({ type: 'note.create', parentDir: 'docs', name: 'Guide' }); + }); + + await waitFor(() => expect(result.current.state.navigation.target?.notePath).toBe('docs/Guide.md')); + + act(() => { + result.current.dispatch({ type: 'folder.rename', path: 'docs', name: 'guides' }); + }); + + await waitFor(() => expect(result.current.state.navigation.target?.notePath).toBe('guides/Guide.md')); + expect(result.current.state.workspace?.document.activeFile?.path).toBe('guides/Guide.md'); + expect(result.current.state.workspace?.tree.files.some((file) => file.path === 'guides/Guide.md')).toBe(true); + }); + + test('tracks repo probe state and ignores stale probe results', async () => { + let firstProbe = createDeferred(); + let secondProbe = createDeferred(); + mockRepoExists + .mockImplementationOnce(() => firstProbe.promise) + .mockImplementationOnce(() => secondProbe.promise); + + let { result } = renderAppData({ kind: 'home' }); + + act(() => { + result.current.dispatch({ type: 'repo.probe', owner: 'acme', repo: 'docs' }); + }); + + expect(result.current.state.repos.probe).toEqual({ + status: 'checking', + owner: 'acme', + repo: 'docs', + }); + + act(() => { + result.current.dispatch({ type: 'repo.probe', owner: 'space', repo: 'wiki' }); + }); + + expect(result.current.state.repos.probe).toEqual({ + status: 'checking', + owner: 'space', + repo: 'wiki', + }); + + await act(async () => { + firstProbe.resolve(true); + await firstProbe.promise; + }); + + expect(result.current.state.repos.probe).toEqual({ + status: 'checking', + owner: 'space', + repo: 'wiki', + }); + + await act(async () => { + secondProbe.resolve(false); + await secondProbe.promise; + }); + + await waitFor(() => + expect(result.current.state.repos.probe).toEqual({ + status: 'ready', + owner: 'space', + repo: 'wiki', + exists: false, + }) + ); + }); + + test('reflects sign-in and sign-out outcomes through session state', async () => { + mockSignInWithGitHubApp.mockResolvedValue({ + token: 'session-token', + user: { + login: 'mona', + name: 'Mona Lisa', + avatarUrl: 'https://example.com/mona.png', + }, + }); + + let { result } = renderAppData({ kind: 'new' }); + + act(() => { + result.current.dispatch({ type: 'session.sign-in' }); + }); + + await waitFor(() => expect(result.current.state.session.status).toBe('signed-in')); + expect(result.current.state.session.user?.login).toBe('mona'); + + act(() => { + result.current.dispatch({ type: 'session.sign-out' }); + }); + + await waitFor(() => expect(result.current.state.session.status).toBe('signed-out')); + expect(result.current.state.session.user).toBeUndefined(); + }); +}); diff --git a/tasks/add-ui-data-contract-tests.md b/tasks/add-ui-data-contract-tests.md new file mode 100644 index 0000000..3259d44 --- /dev/null +++ b/tasks/add-ui-data-contract-tests.md @@ -0,0 +1,54 @@ +--- +status: done +created: 2026-03-05 +completed: 2026-03-06 +assigned: subagent-app-contract-tests +--- + +# Add tests that protect the UI/data contract + +Add tests that lock in the contract the UI depends on, so a future rewritten data layer can prove compatibility without reusing the current internals. + +The purpose is not to test every implementation detail. The purpose is to protect the boundary that issue `#99` is creating. + +Must-haves: + +- The tests cover the most important state and action flows the UI contract promises. +- The tests focus on contract behavior, not incidental internal structure. +- The test coverage is strong enough to support the later data-layer rewrite with confidence. + +Success will be validated by: + +- The repo gains runnable tests that assert the new boundary behavior. +- The tests would catch meaningful regressions in the contract even if internals were rewritten. +- The task file records what is covered and what remains intentionally out of scope. + +## Results + +Added a dedicated `useAppData` contract suite in `src/data/app-data-contract.test.ts`. + +### Covered + +- Route-to-state derivation at app scope: + - empty `home` routes resolve to the new-workspace contract + - `start` routes resolve from the most recent repo entry +- App-level navigation dispatch: + - `repo.activate` opens the targeted workspace + - `navigation.go-home` returns to the home screen and clears `workspace` +- App-owned recent-repo behavior: + - activating a readable repo updates `state.repos.recents` +- Workspace contract outcomes observed through app state: + - `note.create` updates selected note state without relying on callback return values + - `file.rename` updates both navigation state and the resolved document path + - `folder.rename` remaps the selected note path through contract state +- App-level probe behavior: + - `repo.probe` exposes `checking` and final result state + - stale probe responses are ignored +- Session contract behavior: + - `session.sign-in` and `session.sign-out` are asserted via `state.session` + +### Intentional gaps + +- The suite does not try to exhaustively test every action mapping from `dispatch`; low-level file/folder mutation details are still covered better by existing repo-store tests. +- `helpers.importPastedAssets` is intentionally out of scope here because the contract focus is `state` plus `dispatch`, and the helper remains a temporary transition escape hatch. +- Sync and share flows are not deeply re-covered here; existing tests already exercise those paths closer to the current implementation, and this suite is meant to lock the app-facing boundary first. From fc81c497db0bf446b9443ac0b4025bd261b2c07b Mon Sep 17 00:00:00 2001 From: Gregor Date: Fri, 6 Mar 2026 02:09:18 +0100 Subject: [PATCH 03/18] renaming --- src/App.tsx | 8 ++--- src/data.ts | 57 ++++++++++++++---------------- src/data/app-data-contract.test.ts | 20 ++++++----- src/data/data.test.ts | 33 ++++++++++------- src/ui/RepoView.tsx | 34 +++++++++++++----- src/ui/routing.test.ts | 12 +++---- src/ui/routing.ts | 28 +++++++-------- 7 files changed, 108 insertions(+), 84 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index 7646516..0e995c6 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -37,13 +37,13 @@ function routeFromNavigation(navigation: AppNavigationState): Route | undefined if (navigation.screen === 'home') return { kind: 'home' } as const; if (navigation.screen !== 'workspace' || navigation.target === undefined) return undefined; if (navigation.target.repo.kind === 'new') { - return { kind: 'new', notePath: navigation.target.notePath } as const; + return { kind: 'new', filePath: navigation.target.filePath } as const; } return { kind: 'repo', owner: navigation.target.repo.owner, repo: navigation.target.repo.repo, - notePath: navigation.target.notePath, + filePath: navigation.target.filePath, } as const; } @@ -51,9 +51,9 @@ function routesEqual(a: Route | undefined, b: Route | undefined) { if (a === undefined || b === undefined) return a === b; if (a.kind !== b.kind) return false; if (a.kind === 'home' && b.kind === 'home') return true; - if (a.kind === 'new' && b.kind === 'new') return a.notePath === b.notePath; + if (a.kind === 'new' && b.kind === 'new') return a.filePath === b.filePath; if (a.kind === 'repo' && b.kind === 'repo') { - return a.owner === b.owner && a.repo === b.repo && a.notePath === b.notePath; + return a.owner === b.owner && a.repo === b.repo && a.filePath === b.filePath; } return false; } diff --git a/src/data.ts b/src/data.ts index 8e010b2..2c1a354 100644 --- a/src/data.ts +++ b/src/data.ts @@ -32,12 +32,7 @@ import { getInstallUrl as apiGetInstallUrl, type RepoMetadata, } from './lib/backend'; -import { - createGitShare, - revokeGitShare, - lookupCachedShare, - type GitShareLink, -} from './lib/git-share-ops'; +import { createGitShare, revokeGitShare, lookupCachedShare, type GitShareLink } from './lib/git-share-ops'; import { buildRemoteConfig, syncBidirectional, @@ -163,8 +158,8 @@ type RepoDataInputs = { }; type AppNavigationTarget = - | { repo: { kind: 'new'; slug: 'new' }; notePath?: string } - | { repo: { kind: 'github'; slug: string; owner: string; repo: string }; notePath?: string }; + | { repo: { kind: 'new'; slug: 'new' }; filePath?: string } + | { repo: { kind: 'github'; slug: string; owner: string; repo: string }; filePath?: string }; type AppNavigationState = { screen: 'resolving' | 'home' | 'workspace'; @@ -229,7 +224,11 @@ type AppDataAction = | { type: 'navigation.go-home' } | { type: 'session.sign-in' } | { type: 'session.sign-out' } - | { type: 'repo.activate'; repo: { kind: 'new' } | { kind: 'github'; owner: string; repo: string }; notePath?: string } + | { + type: 'repo.activate'; + repo: { kind: 'new' } | { kind: 'github'; owner: string; repo: string }; + notePath?: string; + } | { type: 'repo.probe'; owner: string; repo: string } | { type: 'repo.request-access'; owner: string; repo: string } | { type: 'note.open'; path?: string } @@ -300,7 +299,7 @@ function useRepoWorkspaceData({ slug, route }: RepoDataInputs): { let accessStatusReady = repoAccess.status === 'ready' || repoAccess.status === 'error'; let accessStatusUnknown = !accessStatusReady || repoAccess.errorType === 'network'; - let desiredPath = normalizePath(route.notePath); + let desiredPath = normalizePath(route.filePath); // in readonly mode, we store nothing locally and just fetch content from github no demand let isReadOnly = repoAccess.level === 'read'; @@ -484,7 +483,7 @@ function useRepoWorkspaceData({ slug, route }: RepoDataInputs): { // CLICK HANDLERS const ensureActivePath = (nextPath: string | undefined, options?: { replace?: boolean }) => { - if (pathsEqual(route.notePath, nextPath)) return; + if (pathsEqual(route.filePath, nextPath)) return; // Keep path changes as data so the app-level adapter can sync the router. setTimeout(() => { setRouteSync({ @@ -939,11 +938,7 @@ function useAppData({ route }: { route: Route }): AppDataResult { target: workspaceTarget.repo, access: { status: workspaceData.state.repoQueryStatus, - level: workspaceData.state.canEdit - ? 'write' - : workspaceData.state.canRead - ? 'read' - : 'none', + level: workspaceData.state.canEdit ? 'write' : workspaceData.state.canRead ? 'read' : 'none', canRead: workspaceData.state.canRead, canEdit: workspaceData.state.canEdit, canSync: workspaceData.state.canSync, @@ -992,7 +987,7 @@ function useAppData({ route }: { route: Route }): AppDataResult { if (action.type === 'repo.activate') { let nextTarget: AppNavigationTarget = action.repo.kind === 'new' - ? { repo: { kind: 'new', slug: 'new' }, notePath: action.notePath } + ? { repo: { kind: 'new', slug: 'new' }, filePath: action.notePath } : { repo: { kind: 'github', @@ -1000,7 +995,7 @@ function useAppData({ route }: { route: Route }): AppDataResult { repo: action.repo.repo, slug: `${action.repo.owner}/${action.repo.repo}`, }, - notePath: action.notePath, + filePath: action.notePath, }; setNavigation({ screen: 'workspace', target: nextTarget }); return; @@ -1385,18 +1380,18 @@ function pathsEqual(a: string | undefined, b: string | undefined): boolean { function areRepoRoutesEqual(a: RepoRoute, b: RepoRoute): boolean { if (a.kind !== b.kind) return false; - if (a.kind === 'new' && b.kind === 'new') return pathsEqual(a.notePath, b.notePath); + if (a.kind === 'new' && b.kind === 'new') return pathsEqual(a.filePath, b.filePath); if (a.kind === 'repo' && b.kind === 'repo') { - return a.owner === b.owner && a.repo === b.repo && pathsEqual(a.notePath, b.notePath); + return a.owner === b.owner && a.repo === b.repo && pathsEqual(a.filePath, b.filePath); } return false; } function updateRepoRouteNotePath(route: RepoRoute, notePath: string | undefined): RepoRoute { if (route.kind === 'repo') { - return { kind: 'repo', owner: route.owner, repo: route.repo, notePath }; + return { kind: 'repo', owner: route.owner, repo: route.repo, filePath: notePath }; } - return { kind: 'new', notePath }; + return { kind: 'new', filePath: notePath }; } function deriveAppNavigation(route: Route, recents: RecentRepo[]): AppNavigationState { @@ -1425,7 +1420,7 @@ function deriveAppNavigation(route: Route, recents: RecentRepo[]): AppNavigation replace: true, target: { repo: { kind: 'new', slug: 'new' }, - notePath: 'README.md', + filePath: 'README.md', }, }; } @@ -1436,7 +1431,7 @@ function deriveAppNavigation(route: Route, recents: RecentRepo[]): AppNavigation screen: 'workspace', target: { repo: { kind: 'new', slug: 'new' }, - notePath: route.notePath, + filePath: route.filePath, }, }; } @@ -1449,7 +1444,7 @@ function deriveAppNavigation(route: Route, recents: RecentRepo[]): AppNavigation repo: route.repo, slug: `${route.owner}/${route.repo}`, }, - notePath: route.notePath, + filePath: route.filePath, }, }; } @@ -1460,13 +1455,13 @@ function areAppNavigationsEqual(a: AppNavigationState, b: AppNavigationState): b if (a.target === undefined || b.target === undefined) return a.target === b.target; if (a.target.repo.kind !== b.target.repo.kind) return false; if (a.target.repo.kind === 'new' && b.target.repo.kind === 'new') { - return pathsEqual(a.target.notePath, b.target.notePath); + return pathsEqual(a.target.filePath, b.target.filePath); } if (a.target.repo.kind === 'github' && b.target.repo.kind === 'github') { return ( a.target.repo.owner === b.target.repo.owner && a.target.repo.repo === b.target.repo.repo && - pathsEqual(a.target.notePath, b.target.notePath) + pathsEqual(a.target.filePath, b.target.filePath) ); } return false; @@ -1474,19 +1469,19 @@ function areAppNavigationsEqual(a: AppNavigationState, b: AppNavigationState): b function toRepoRoute(target: AppNavigationTarget | undefined): RepoRoute { if (target === undefined || target.repo.kind === 'new') { - return { kind: 'new', notePath: target?.notePath }; + return { kind: 'new', filePath: target?.filePath }; } return { kind: 'repo', owner: target.repo.owner, repo: target.repo.repo, - notePath: target.notePath, + filePath: target.filePath, }; } function toAppNavigationTarget(route: RepoRoute, slug: string): AppNavigationTarget { if (route.kind === 'new') { - return { repo: { kind: 'new', slug: 'new' }, notePath: route.notePath }; + return { repo: { kind: 'new', slug: 'new' }, filePath: route.filePath }; } return { repo: { @@ -1495,7 +1490,7 @@ function toAppNavigationTarget(route: RepoRoute, slug: string): AppNavigationTar repo: route.repo, slug, }, - notePath: route.notePath, + filePath: route.filePath, }; } diff --git a/src/data/app-data-contract.test.ts b/src/data/app-data-contract.test.ts index 6ff1d90..50ac509 100644 --- a/src/data/app-data-contract.test.ts +++ b/src/data/app-data-contract.test.ts @@ -157,13 +157,13 @@ function routeFromNavigation(navigation: AppNavigationState): Route | undefined if (navigation.screen === 'home') return { kind: 'home' }; if (navigation.screen !== 'workspace' || navigation.target === undefined) return undefined; if (navigation.target.repo.kind === 'new') { - return { kind: 'new', notePath: navigation.target.notePath }; + return { kind: 'new', filePath: navigation.target.filePath }; } return { kind: 'repo', owner: navigation.target.repo.owner, repo: navigation.target.repo.repo, - notePath: navigation.target.notePath, + filePath: navigation.target.filePath, }; } @@ -172,9 +172,9 @@ function routesEqual(a: Route | undefined, b: Route | undefined) { if (a.kind !== b.kind) return false; if (a.kind === 'home' && b.kind === 'home') return true; if (a.kind === 'start' && b.kind === 'start') return true; - if (a.kind === 'new' && b.kind === 'new') return a.notePath === b.notePath; + if (a.kind === 'new' && b.kind === 'new') return a.filePath === b.filePath; if (a.kind === 'repo' && b.kind === 'repo') { - return a.owner === b.owner && a.repo === b.repo && a.notePath === b.notePath; + return a.owner === b.owner && a.repo === b.repo && a.filePath === b.filePath; } return false; } @@ -283,14 +283,14 @@ describe('useAppData contract', () => { result.current.dispatch({ type: 'note.create', parentDir: '', name: 'Plan' }); }); - await waitFor(() => expect(result.current.state.navigation.target?.notePath).toBe('Plan.md')); + await waitFor(() => expect(result.current.state.navigation.target?.filePath).toBe('Plan.md')); await waitFor(() => expect(result.current.state.workspace?.document.activeFile?.path).toBe('Plan.md')); act(() => { result.current.dispatch({ type: 'file.rename', path: 'Plan.md', name: 'Roadmap' }); }); - await waitFor(() => expect(result.current.state.navigation.target?.notePath).toBe('Roadmap.md')); + await waitFor(() => expect(result.current.state.navigation.target?.filePath).toBe('Roadmap.md')); expect(result.current.state.workspace?.document.activeFile?.path).toBe('Roadmap.md'); expect(result.current.state.workspace?.tree.files.some((file) => file.path === 'Roadmap.md')).toBe(true); }); @@ -303,15 +303,17 @@ describe('useAppData contract', () => { result.current.dispatch({ type: 'note.create', parentDir: 'docs', name: 'Guide' }); }); - await waitFor(() => expect(result.current.state.navigation.target?.notePath).toBe('docs/Guide.md')); + await waitFor(() => expect(result.current.state.navigation.target?.filePath).toBe('docs/Guide.md')); act(() => { result.current.dispatch({ type: 'folder.rename', path: 'docs', name: 'guides' }); }); - await waitFor(() => expect(result.current.state.navigation.target?.notePath).toBe('guides/Guide.md')); + await waitFor(() => expect(result.current.state.navigation.target?.filePath).toBe('guides/Guide.md')); expect(result.current.state.workspace?.document.activeFile?.path).toBe('guides/Guide.md'); - expect(result.current.state.workspace?.tree.files.some((file) => file.path === 'guides/Guide.md')).toBe(true); + expect(result.current.state.workspace?.tree.files.some((file) => file.path === 'guides/Guide.md')).toBe( + true + ); }); test('tracks repo probe state and ignores stale probe results', async () => { diff --git a/src/data/data.test.ts b/src/data/data.test.ts index 105510c..7c57335 100644 --- a/src/data/data.test.ts +++ b/src/data/data.test.ts @@ -4,7 +4,13 @@ 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, listRecentRepos, markRepoLinked, recordAutoSyncRun, setLastActiveFileId } from '../storage/local'; +import { + LocalStore, + listRecentRepos, + markRepoLinked, + recordAutoSyncRun, + setLastActiveFileId, +} from '../storage/local'; import type { RemoteFile } from '../sync/git-sync'; import type { RepoDataState, ImportedAsset } from '../data'; @@ -236,7 +242,7 @@ describe('useRepoData', () => { if (!welcome) throw new Error('Missing welcome note'); const { result } = renderHook(() => { - const [routeState, setRouteState] = useState({ kind: 'new', notePath: alpha.path }); + const [routeState, setRouteState] = useState({ kind: 'new', filePath: alpha.path }); const data = useRepoData({ slug: 'new', route: routeState }); useEffect(() => { if (data.routeSync === undefined) return; @@ -247,14 +253,14 @@ describe('useRepoData', () => { 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); + expect(result.current.routeState.filePath).toBe(alpha.path); await act(async () => { await result.current.data.actions.selectFile(welcome.path); }); await waitFor(() => expect(result.current.data.state.activePath).toBe(welcome.path)); - expect(result.current.routeState.notePath).toBe(welcome.path); + expect(result.current.routeState.filePath).toBe(welcome.path); }); test('activates the route note path when the file exists locally', async () => { @@ -275,7 +281,7 @@ describe('useRepoData', () => { setRepoMetadata(writableMeta); const recordRecent = vi.fn(); - const route: RepoRoute = { kind: 'repo', owner: 'acme', repo: 'docs', notePath: target.path }; + const route: RepoRoute = { kind: 'repo', owner: 'acme', repo: 'docs', filePath: target.path }; const { result } = renderRepoData({ slug, route, recordRecent }); await waitFor(() => expect(result.current.state.activePath).toBe(target.path)); @@ -319,7 +325,7 @@ describe('useRepoData', () => { }); const recordRecent = vi.fn(); - const route: RepoRoute = { kind: 'repo', owner: 'acme', repo: 'docs', notePath: 'guides/Intro.md' }; + const route: RepoRoute = { kind: 'repo', owner: 'acme', repo: 'docs', filePath: 'guides/Intro.md' }; const { result } = renderRepoData({ slug, route, recordRecent }); await waitFor(() => expect(result.current.state.activePath).toBe('guides/Intro.md')); @@ -570,9 +576,9 @@ describe('useRepoData', () => { expect(entry.altText.startsWith('Pasted image ')).toBe(true); await waitFor(() => - expect(result.current.state.files.some((meta) => meta.path === entry.assetPath && meta.kind === 'binary')).toBe( - true - ) + expect( + result.current.state.files.some((meta) => meta.path === entry.assetPath && meta.kind === 'binary') + ).toBe(true) ); let refreshedStore = new LocalStore(slug); @@ -1126,8 +1132,8 @@ describe('useRepoData', () => { const { result } = renderHook(() => { const value = useRepoData({ slug, route: { kind: 'repo', owner: 'octo', repo: 'public' } }); - if (value.routeSync?.route.notePath !== undefined) { - activePathHistory.push(value.routeSync.route.notePath); + if (value.routeSync?.route.filePath !== undefined) { + activePathHistory.push(value.routeSync.route.filePath); } return value; }); @@ -1147,7 +1153,10 @@ describe('useRepoData', () => { // An oscillating history would be: ['docs/guide.md', 'README.md', 'docs/guide.md', ...] let oscillations = 0; for (let i = 2; i < activePathHistory.length; i++) { - if (activePathHistory[i] === activePathHistory[i - 2] && activePathHistory[i] !== activePathHistory[i - 1]) { + if ( + activePathHistory[i] === activePathHistory[i - 2] && + activePathHistory[i] !== activePathHistory[i - 1] + ) { oscillations++; } } diff --git a/src/ui/RepoView.tsx b/src/ui/RepoView.tsx index 62d2edc..3f665ab 100644 --- a/src/ui/RepoView.tsx +++ b/src/ui/RepoView.tsx @@ -46,10 +46,17 @@ function RepoViewInner({ state, dispatch, helpers }: RepoViewProps) { let slug = workspace.target.slug; let hasSession = state.session.status === 'signed-in'; let user = state.session.user; - let { canEdit, canRead, canSync, linked: repoLinked, manageUrl, defaultBranch, errorType: repoErrorType } = - workspace.access; + let { + canEdit, + canRead, + canSync, + linked: repoLinked, + manageUrl, + defaultBranch, + errorType: repoErrorType, + } = workspace.access; let activeFile = workspace.document.activeFile; - let activePath = state.navigation.target?.notePath ?? workspace.document.activePath; + let activePath = state.navigation.target?.filePath ?? workspace.document.activePath; let files = workspace.tree.files; let folders = workspace.tree.folders; let autosync = workspace.sync.autosync; @@ -66,7 +73,11 @@ function RepoViewInner({ state, dispatch, helpers }: RepoViewProps) { const activeIsMarkdown = activeFile !== undefined && isMarkdownFile(activeFile); const canShare = - hasSession && workspace.target.kind === 'github' && activePath !== undefined && canEdit && activeIsMarkdown; + hasSession && + workspace.target.kind === 'github' && + activePath !== undefined && + canEdit && + activeIsMarkdown; const shareDisabled = share.status === 'idle' || share.status === 'loading'; // error states that require user action (these trigger a custom full sized banner) @@ -257,8 +268,8 @@ function RepoViewInner({ state, dispatch, helpers }: RepoViewProps) { share.link ? 'Manage share link' : shareDisabled - ? 'Checking share status' - : 'Create share link' + ? 'Checking share status' + : 'Create share link' } aria-label={share.link ? 'Manage share link' : 'Create share link'} disabled={shareDisabled} @@ -367,7 +378,12 @@ function RepoViewInner({ state, dispatch, helpers }: RepoViewProps) { Read-only You can view, but not edit files in this repository. {hasSession && ( - )} @@ -425,7 +441,9 @@ function RepoViewInner({ state, dispatch, helpers }: RepoViewProps) { {hasSession ? ( diff --git a/src/ui/routing.test.ts b/src/ui/routing.test.ts index 0f0f1bc..d5d7e10 100644 --- a/src/ui/routing.test.ts +++ b/src/ui/routing.test.ts @@ -10,14 +10,14 @@ describe('routing helpers', () => { it('parses /new with nested note path', () => { expect(parseRoute('/new/docs/setup.md')).toEqual({ kind: 'new', - notePath: 'docs/setup.md', + filePath: 'docs/setup.md', }); }); it('round-trips /new with encoded segments', () => { const path = '/new/docs/My%20Note.md'; const route = parseRoute(path); - expect(route).toEqual({ kind: 'new', notePath: 'docs/My Note.md' }); + expect(route).toEqual({ kind: 'new', filePath: 'docs/My Note.md' }); expect(routeToPath(route)).toBe(path); }); @@ -26,13 +26,13 @@ describe('routing helpers', () => { kind: 'repo', owner: 'acme', repo: 'docs', - notePath: 'guides/intro.md', + filePath: 'guides/intro.md', }); }); it('builds repo paths with nested note path', () => { - expect( - routeToPath({ kind: 'repo', owner: 'acme', repo: 'docs', notePath: 'guides/intro.md' }) - ).toBe('/acme/docs/guides/intro.md'); + expect(routeToPath({ kind: 'repo', owner: 'acme', repo: 'docs', filePath: 'guides/intro.md' })).toBe( + '/acme/docs/guides/intro.md' + ); }); }); diff --git a/src/ui/routing.ts b/src/ui/routing.ts index dcc7fef..317132b 100644 --- a/src/ui/routing.ts +++ b/src/ui/routing.ts @@ -6,13 +6,13 @@ export { useRoute, parseRoute, routeToPath }; type Route = | { kind: 'home' } - | { kind: 'new'; notePath?: string } + | { kind: 'new'; filePath?: string } | { kind: 'start' } - | { kind: 'repo'; owner: string; repo: string; notePath?: string }; + | { kind: 'repo'; owner: string; repo: string; filePath?: string }; type RepoRoute = - | { kind: 'new'; notePath?: string } - | { kind: 'repo'; owner: string; repo: string; notePath?: string }; + | { kind: 'new'; filePath?: string } + | { kind: 'repo'; owner: string; repo: string; filePath?: string }; const HOME_ROUTE: Route = { kind: 'home' }; const NEW_ROUTE: Route = { kind: 'new' }; @@ -30,17 +30,17 @@ function parseRoute(pathname: string): Route { if (clean === '/new') return NEW_ROUTE; let segments = clean.replace(/^\//, '').split('/'); if (segments.length >= 1 && segments[0] === 'new') { - let noteSegments = segments.slice(1).map((segment) => decodeURIComponent(segment ?? '')); - let notePath = noteSegments.length > 0 ? noteSegments.join('/') : undefined; - return { kind: 'new', notePath }; + let fileSegments = segments.slice(1).map((segment) => decodeURIComponent(segment ?? '')); + let filePath = fileSegments.length > 0 ? fileSegments.join('/') : undefined; + return { kind: 'new', filePath }; } if (segments.length >= 2) { let owner = decodeURIComponent(segments[0] ?? ''); let repo = decodeURIComponent(segments[1] ?? ''); if (owner && repo) { - let noteSegments = segments.slice(2).map((segment) => decodeURIComponent(segment ?? '')); - let notePath = noteSegments.length > 0 ? noteSegments.join('/') : undefined; - return { kind: 'repo', owner, repo, notePath }; + let fileSegments = segments.slice(2).map((segment) => decodeURIComponent(segment ?? '')); + let filePath = fileSegments.length > 0 ? fileSegments.join('/') : undefined; + return { kind: 'repo', owner, repo, filePath }; } } return HOME_ROUTE; @@ -49,8 +49,8 @@ function parseRoute(pathname: string): Route { function routeToPath(route: Route): string { if (route.kind === 'home') return '/'; if (route.kind === 'new') { - if (!route.notePath || route.notePath === '') return '/new'; - let segments = route.notePath + if (!route.filePath || route.filePath === '') return '/new'; + let segments = route.filePath .split('/') .filter((segment) => segment !== '') .map((segment) => encodeURIComponent(segment)); @@ -60,10 +60,10 @@ function routeToPath(route: Route): string { if (route.kind === 'start') return '/start'; let owner = encodeURIComponent(route.owner); let repo = encodeURIComponent(route.repo); - if (!route.notePath) { + if (!route.filePath) { return `/${owner}/${repo}`; } - let segments = route.notePath + let segments = route.filePath .split('/') .filter((segment) => segment !== '') .map((segment) => encodeURIComponent(segment)); From 3c3fc0c65b2a1955dd7ae74f584466356157fb2a Mon Sep 17 00:00:00 2001 From: Gregor Date: Fri, 6 Mar 2026 02:14:55 +0100 Subject: [PATCH 04/18] renaming --- src/data.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/data.ts b/src/data.ts index 2c1a354..c48930c 100644 --- a/src/data.ts +++ b/src/data.ts @@ -227,7 +227,7 @@ type AppDataAction = | { type: 'repo.activate'; repo: { kind: 'new' } | { kind: 'github'; owner: string; repo: string }; - notePath?: string; + filePath?: string; } | { type: 'repo.probe'; owner: string; repo: string } | { type: 'repo.request-access'; owner: string; repo: string } @@ -987,7 +987,7 @@ function useAppData({ route }: { route: Route }): AppDataResult { if (action.type === 'repo.activate') { let nextTarget: AppNavigationTarget = action.repo.kind === 'new' - ? { repo: { kind: 'new', slug: 'new' }, filePath: action.notePath } + ? { repo: { kind: 'new', slug: 'new' }, filePath: action.filePath } : { repo: { kind: 'github', @@ -995,7 +995,7 @@ function useAppData({ route }: { route: Route }): AppDataResult { repo: action.repo.repo, slug: `${action.repo.owner}/${action.repo.repo}`, }, - filePath: action.notePath, + filePath: action.filePath, }; setNavigation({ screen: 'workspace', target: nextTarget }); return; From e8d1f8c8eecc11ef6eba5e4131b9f273c8fd8f8f Mon Sep 17 00:00:00 2001 From: Gregor Date: Fri, 6 Mar 2026 02:42:00 +0100 Subject: [PATCH 05/18] Polish app data contract review feedback --- src/App.tsx | 8 +- src/data.ts | 163 ++++++++++++++--------------- src/data/app-data-contract.test.ts | 26 ++--- src/ui/RepoView.tsx | 33 +++--- 4 files changed, 113 insertions(+), 117 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index 0e995c6..bb1bc0a 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -12,7 +12,7 @@ export function App() { useEffect(() => { let target = state.workspace?.target; document.title = - target !== undefined && target.kind === 'github' ? `${target.owner}/${target.repo}` : 'VibeNote'; + target !== undefined && target.kind === 'repo' ? `${target.owner}/${target.repo}` : 'VibeNote'; }, [state.workspace?.target]); useEffect(() => { @@ -36,13 +36,13 @@ export function App() { function routeFromNavigation(navigation: AppNavigationState): Route | undefined { if (navigation.screen === 'home') return { kind: 'home' } as const; if (navigation.screen !== 'workspace' || navigation.target === undefined) return undefined; - if (navigation.target.repo.kind === 'new') { + if (navigation.target.kind === 'new') { return { kind: 'new', filePath: navigation.target.filePath } as const; } return { kind: 'repo', - owner: navigation.target.repo.owner, - repo: navigation.target.repo.repo, + owner: navigation.target.owner, + repo: navigation.target.repo, filePath: navigation.target.filePath, } as const; } diff --git a/src/data.ts b/src/data.ts index c48930c..72d3422 100644 --- a/src/data.ts +++ b/src/data.ts @@ -51,13 +51,12 @@ import { prepareClipboardImage } from './lib/image-processing'; import { relativePathBetween, COMMON_ASSET_DIR } from './lib/pathing'; import type { Route, RepoRoute } from './ui/routing'; -export { useAppData, useRepoData }; +export { useAppData, useRepoData, repoRouteToSlug }; export type { AppDataAction, AppDataResult, AppDataState, AppNavigationState, - AppNavigationTarget, RepoAccessState, RepoDataInputs, RepoDataState, @@ -157,35 +156,49 @@ type RepoDataInputs = { route: RepoRoute; }; -type AppNavigationTarget = - | { repo: { kind: 'new'; slug: 'new' }; filePath?: string } - | { repo: { kind: 'github'; slug: string; owner: string; repo: string }; filePath?: string }; - type AppNavigationState = { + /** Which top-level screen the app should currently render. */ screen: 'resolving' | 'home' | 'workspace'; - target?: AppNavigationTarget; + /** Active workspace target when the app is on the workspace screen. */ + target?: RepoRoute; + /** Whether the next route sync should replace browser history. */ replace?: boolean; }; type RepoProbeState = { + /** Lifecycle of the latest repo probe request from the switcher UI. */ status: 'idle' | 'checking' | 'ready'; + /** Owner currently being probed, if any. */ owner?: string; + /** Repo currently being probed, if any. */ repo?: string; + /** Whether the probed repo appears reachable from the current session. */ exists?: boolean; }; +/** App-level contract consumed by the UI shell. */ type AppDataState = { + /** Current auth/session state for GitHub-backed features. */ session: { status: 'signed-out' | 'signed-in'; user: AppUser | undefined; }; + + /** Canonical app location, synced to the URL by App.tsx. */ navigation: AppNavigationState; + + /** Cross-workspace repo state that should survive switching between repos. */ repos: { recents: RecentRepo[]; probe: RepoProbeState; }; + + /** State for the active repo/new-note workspace, if the user is currently in one. */ workspace?: { - target: AppNavigationTarget['repo']; + /** The repo/new route this workspace represents. */ + target: RepoRoute; + + /** Access and GitHub integration status for the active target. */ access: { status: RepoQueryStatus; level: RepoAccessLevel; @@ -197,19 +210,27 @@ type AppDataState = { defaultBranch: string | undefined; errorType: RepoAccessErrorType | undefined; }; + + /** Tree data rendered by the file sidebar. */ tree: { files: FileMeta[]; folders: string[]; }; + + /** Currently opened file within the workspace. */ document: { activeFile: RepoFile | undefined; activePath: string | undefined; }; + + /** Sync-related state surfaced to the header and status banner. */ sync: { autosync: boolean; syncing: boolean; statusMessage: string | undefined; }; + + /** Share-link state for the active markdown note. */ share: { status: ShareState['status']; link?: { @@ -220,17 +241,27 @@ type AppDataState = { }; }; +// Action protocol emitted by the UI. +// Actions are intents, not imperative callbacks: the UI asks for something to +// happen and then observes the resulting state update. type AppDataAction = + // App-level navigation and session lifecycle. | { type: 'navigation.go-home' } | { type: 'session.sign-in' } | { type: 'session.sign-out' } + + // Repo selection and access checks. + // Open a workspace target and optionally seed the desired file path. | { type: 'repo.activate'; repo: { kind: 'new' } | { kind: 'github'; owner: string; repo: string }; filePath?: string; } + // Check whether an owner/repo appears reachable from the current session. | { type: 'repo.probe'; owner: string; repo: string } | { type: 'repo.request-access'; owner: string; repo: string } + + // File/folder selection and local edits within the active workspace. | { type: 'note.open'; path?: string } | { type: 'note.create'; parentDir: string; name: string } | { type: 'file.save'; path: string; contents: string } @@ -241,10 +272,18 @@ type AppDataAction = | { type: 'folder.rename'; path: string; name: string } | { type: 'folder.move'; path: string; targetDir: string } | { type: 'folder.delete'; path: string } + + // Editor-specific file imports that create assets plus markdown references. + // Import pasted files into repo storage and attach them to the current note. | { type: 'assets.import'; notePath: string; files: File[] } + + // Sync controls for the active workspace. | { type: 'sync.run'; source: 'user' | 'auto' } | { type: 'sync.set-autosync'; enabled: boolean } + + // Share-link lifecycle for the active markdown note. | { type: 'share.create'; notePath: string } + // Reload the cached share-link status for the active note target. | { type: 'share.refresh'; notePath: string } | { type: 'share.revoke'; notePath: string }; @@ -485,6 +524,7 @@ function useRepoWorkspaceData({ slug, route }: RepoDataInputs): { const ensureActivePath = (nextPath: string | undefined, options?: { replace?: boolean }) => { if (pathsEqual(route.filePath, nextPath)) return; // Keep path changes as data so the app-level adapter can sync the router. + // hack: we navigate on the next event loop task to give React state time to update active doc setTimeout(() => { setRouteSync({ revision: ++routeRevisionRef.current, @@ -835,6 +875,17 @@ function useRepoWorkspaceData({ slug, route }: RepoDataInputs): { return { state, actions, routeSync }; } +/** + * Data layer entry point for the repo-scoped implementation. + * + * Invariants for callers: + * - `slug` and `route` are always in sync, and never change throughout the component lifetime + * - callers can treat the hook as owning the current repo route after mount + * - `routeSync` is the only supported way for this layer to request route updates + * + * The wrapper keeps those routing mechanics localized so the workspace hook can + * stay focused on repo state, sync, and file operations. + */ function useRepoData({ slug, route }: RepoDataInputs): { state: RepoDataState; actions: RepoDataActions; @@ -853,6 +904,9 @@ function useRepoData({ slug, route }: RepoDataInputs): { setRouteState((prev) => (areRepoRoutesEqual(prev, routeSync.route) ? prev : routeSync.route)); }, [routeSync?.revision]); + // Remember recently opened repos once we know the current repo is reachable. + // TODO this shouldn't be a useEffect, the only place a repo ever becomes reachable is after + // fetching metadata, so just record it there useEffect(() => { if (routeState.kind !== 'repo') return; if (!state.canRead) return; @@ -887,8 +941,8 @@ function useAppData({ route }: { route: Route }): AppDataResult { }, [route, recents]); let workspaceTarget = navigation.screen === 'workspace' ? navigation.target : undefined; - let repoRoute = toRepoRoute(workspaceTarget); - let slug = workspaceTarget?.repo.slug ?? 'new'; + let repoRoute = workspaceTarget ?? { kind: 'new' }; + let slug = repoRouteToSlug(repoRoute); let workspaceData = useRepoData({ slug, route: repoRoute }); useEffect(() => { @@ -899,13 +953,13 @@ function useAppData({ route }: { route: Route }): AppDataResult { areAppNavigationsEqual(prev, { screen: 'workspace', replace: nextRouteSync.replace, - target: toAppNavigationTarget(nextRouteSync.route, slug), + target: nextRouteSync.route, }) ? prev : { screen: 'workspace', replace: nextRouteSync.replace, - target: toAppNavigationTarget(nextRouteSync.route, slug), + target: nextRouteSync.route, } ); }, [navigation.screen, workspaceData.routeSync?.revision, slug]); @@ -935,7 +989,7 @@ function useAppData({ route }: { route: Route }): AppDataResult { workspaceTarget === undefined ? undefined : { - target: workspaceTarget.repo, + target: workspaceTarget, access: { status: workspaceData.state.repoQueryStatus, level: workspaceData.state.canEdit ? 'write' : workspaceData.state.canRead ? 'read' : 'none', @@ -985,16 +1039,13 @@ function useAppData({ route }: { route: Route }): AppDataResult { return; } if (action.type === 'repo.activate') { - let nextTarget: AppNavigationTarget = + let nextTarget: RepoRoute = action.repo.kind === 'new' - ? { repo: { kind: 'new', slug: 'new' }, filePath: action.filePath } + ? { kind: 'new', filePath: action.filePath } : { - repo: { - kind: 'github', - owner: action.repo.owner, - repo: action.repo.repo, - slug: `${action.repo.owner}/${action.repo.repo}`, - }, + kind: 'repo', + owner: action.repo.owner, + repo: action.repo.repo, filePath: action.filePath, }; setNavigation({ screen: 'workspace', target: nextTarget }); @@ -1401,14 +1452,7 @@ function deriveAppNavigation(route: Route, recents: RecentRepo[]): AppNavigation return { screen: 'workspace', replace: true, - target: { - repo: { - kind: 'github', - owner: candidate.owner, - repo: candidate.repo, - slug: candidate.slug, - }, - }, + target: { kind: 'repo', owner: candidate.owner, repo: candidate.repo }, }; } return { screen: 'home', replace: true }; @@ -1418,10 +1462,7 @@ function deriveAppNavigation(route: Route, recents: RecentRepo[]): AppNavigation return { screen: 'workspace', replace: true, - target: { - repo: { kind: 'new', slug: 'new' }, - filePath: 'README.md', - }, + target: { kind: 'new', filePath: 'README.md' }, }; } return { screen: 'home' }; @@ -1429,23 +1470,12 @@ function deriveAppNavigation(route: Route, recents: RecentRepo[]): AppNavigation if (route.kind === 'new') { return { screen: 'workspace', - target: { - repo: { kind: 'new', slug: 'new' }, - filePath: route.filePath, - }, + target: { kind: 'new', filePath: route.filePath }, }; } return { screen: 'workspace', - target: { - repo: { - kind: 'github', - owner: route.owner, - repo: route.repo, - slug: `${route.owner}/${route.repo}`, - }, - filePath: route.filePath, - }, + target: { kind: 'repo', owner: route.owner, repo: route.repo, filePath: route.filePath }, }; } @@ -1453,45 +1483,12 @@ function areAppNavigationsEqual(a: AppNavigationState, b: AppNavigationState): b if (a.screen !== b.screen) return false; if (a.replace !== b.replace) return false; if (a.target === undefined || b.target === undefined) return a.target === b.target; - if (a.target.repo.kind !== b.target.repo.kind) return false; - if (a.target.repo.kind === 'new' && b.target.repo.kind === 'new') { - return pathsEqual(a.target.filePath, b.target.filePath); - } - if (a.target.repo.kind === 'github' && b.target.repo.kind === 'github') { - return ( - a.target.repo.owner === b.target.repo.owner && - a.target.repo.repo === b.target.repo.repo && - pathsEqual(a.target.filePath, b.target.filePath) - ); - } - return false; + return areRepoRoutesEqual(a.target, b.target); } -function toRepoRoute(target: AppNavigationTarget | undefined): RepoRoute { - if (target === undefined || target.repo.kind === 'new') { - return { kind: 'new', filePath: target?.filePath }; - } - return { - kind: 'repo', - owner: target.repo.owner, - repo: target.repo.repo, - filePath: target.filePath, - }; -} - -function toAppNavigationTarget(route: RepoRoute, slug: string): AppNavigationTarget { - if (route.kind === 'new') { - return { repo: { kind: 'new', slug: 'new' }, filePath: route.filePath }; - } - return { - repo: { - kind: 'github', - owner: route.owner, - repo: route.repo, - slug, - }, - filePath: route.filePath, - }; +function repoRouteToSlug(route: RepoRoute): string { + if (route.kind === 'new') return 'new'; + return `${route.owner}/${route.repo}`; } function isPathInsideDir(path: string, dir: string): boolean { diff --git a/src/data/app-data-contract.test.ts b/src/data/app-data-contract.test.ts index 50ac509..cd34323 100644 --- a/src/data/app-data-contract.test.ts +++ b/src/data/app-data-contract.test.ts @@ -156,13 +156,13 @@ function setRepoMetadata(meta: RepoMetadata) { function routeFromNavigation(navigation: AppNavigationState): Route | undefined { if (navigation.screen === 'home') return { kind: 'home' }; if (navigation.screen !== 'workspace' || navigation.target === undefined) return undefined; - if (navigation.target.repo.kind === 'new') { + if (navigation.target.kind === 'new') { return { kind: 'new', filePath: navigation.target.filePath }; } return { kind: 'repo', - owner: navigation.target.repo.owner, - repo: navigation.target.repo.repo, + owner: navigation.target.owner, + repo: navigation.target.repo, filePath: navigation.target.filePath, }; } @@ -221,11 +221,8 @@ describe('useAppData contract', () => { await waitFor(() => expect(result.current.state.workspace?.document.activePath).toBe('README.md')); expect(result.current.state.navigation.screen).toBe('workspace'); - expect(result.current.state.navigation.target).toEqual({ - repo: { kind: 'new', slug: 'new' }, - notePath: 'README.md', - }); - expect(result.current.state.workspace?.target).toEqual({ kind: 'new', slug: 'new' }); + expect(result.current.state.navigation.target).toEqual({ kind: 'new', filePath: 'README.md' }); + expect(result.current.state.workspace?.target).toEqual({ kind: 'new', filePath: 'README.md' }); }); test('derives the start route from the most recent repository', async () => { @@ -236,13 +233,12 @@ describe('useAppData contract', () => { await waitFor(() => expect(result.current.state.navigation.screen).toBe('workspace')); let target = result.current.state.navigation.target; - if (target === undefined || target.repo.kind !== 'github') { + if (target === undefined || target.kind !== 'repo') { throw new Error('Expected a GitHub workspace target'); } - expect(target.repo.owner).toBe('acme'); - expect(target.repo.repo).toBe('docs'); - expect(target.repo.slug).toBe('acme/docs'); + expect(target.owner).toBe('acme'); + expect(target.repo).toBe('docs'); expect(result.current.state.repos.recents.map((entry) => entry.slug)).toEqual(['acme/docs']); }); @@ -260,12 +256,12 @@ describe('useAppData contract', () => { await waitFor(() => expect(result.current.state.repos.recents[0]?.slug).toBe('acme/docs')); let target = result.current.state.navigation.target; - if (target === undefined || target.repo.kind !== 'github') { + if (target === undefined || target.kind !== 'repo') { throw new Error('Expected a GitHub workspace target'); } - expect(target.repo.owner).toBe('acme'); - expect(target.repo.repo).toBe('docs'); + expect(target.owner).toBe('acme'); + expect(target.repo).toBe('docs'); expect(listRecentRepos()[0]?.slug).toBe('acme/docs'); act(() => { diff --git a/src/ui/RepoView.tsx b/src/ui/RepoView.tsx index 3f665ab..98d0a1a 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 type { AppDataAction, AppDataResult, AppDataState } from '../data'; +import { repoRouteToSlug, type AppDataAction, type AppDataResult, type AppDataState } from '../data'; import type { FileMeta } from '../storage/local'; import { getExpandedFolders, @@ -36,14 +36,21 @@ const primaryModifier = detectPrimaryShortcut(); export function RepoView({ state, dispatch, helpers }: RepoViewProps) { let workspace = state.workspace; if (workspace === undefined) return null; - return ; + return ( + + ); } function RepoViewInner({ state, dispatch, helpers }: RepoViewProps) { let workspace = state.workspace; if (workspace === undefined) return null; - let slug = workspace.target.slug; + let slug = repoRouteToSlug(workspace.target); let hasSession = state.session.status === 'signed-in'; let user = state.session.user; let { @@ -65,25 +72,21 @@ function RepoViewInner({ state, dispatch, helpers }: RepoViewProps) { let share = workspace.share; const userAvatarSrc = user?.avatarDataUrl ?? user?.avatarUrl ?? undefined; - let repoOwner = workspace.target.kind === 'github' ? workspace.target.owner : undefined; - let repoName = workspace.target.kind === 'github' ? workspace.target.repo : undefined; + let repoOwner = workspace.target.kind === 'repo' ? workspace.target.owner : undefined; + let repoName = workspace.target.kind === 'repo' ? workspace.target.repo : undefined; const showSidebar = canRead; const isReadOnly = !canEdit && canRead; const layoutClass = showSidebar ? '' : 'single'; const activeIsMarkdown = activeFile !== undefined && isMarkdownFile(activeFile); const canShare = - hasSession && - workspace.target.kind === 'github' && - activePath !== undefined && - canEdit && - activeIsMarkdown; + hasSession && workspace.target.kind === 'repo' && activePath !== undefined && canEdit && activeIsMarkdown; const shareDisabled = share.status === 'idle' || share.status === 'loading'; // error states that require user action (these trigger a custom full sized banner) const needsSessionRefresh = repoLinked && repoErrorType === 'auth'; const needsInstall = hasSession && repoErrorType === 'not-found'; - const needsUserAction = workspace.target.kind === 'github' && (needsSessionRefresh || needsInstall); + const needsUserAction = workspace.target.kind === 'repo' && (needsSessionRefresh || needsInstall); // Pure UI state: sidebar visibility and account menu. const [sidebarOpen, setSidebarOpen] = useState(false); @@ -105,7 +108,7 @@ function RepoViewInner({ state, dispatch, helpers }: RepoViewProps) { // Keyboard shortcuts: Cmd/Ctrl+K and "g","r" open the repo switcher even when the tree is focused. const repoShortcutLabel = primaryModifier === 'meta' ? '⌘K' : 'Ctrl+K'; - const repoButtonBaseTitle = workspace.target.kind === 'github' ? 'Change repository' : 'Choose repository'; + const repoButtonBaseTitle = workspace.target.kind === 'repo' ? 'Change repository' : 'Choose repository'; const repoButtonTitle = `${repoButtonBaseTitle} (${repoShortcutLabel})`; useEffect(() => { @@ -206,7 +209,7 @@ function RepoViewInner({ state, dispatch, helpers }: RepoViewProps) { > VibeNote - {workspace.target.kind === 'github' ? ( + {workspace.target.kind === 'repo' ? ( diff --git a/src/ui/RepoView.tsx b/src/ui/RepoView.tsx index bf02b59..88c8d35 100644 --- a/src/ui/RepoView.tsx +++ b/src/ui/RepoView.tsx @@ -510,7 +510,6 @@ function RepoViewInner({ state, dispatch, helpers }: RepoViewProps) { )} {showSwitcher && ( Date: Fri, 6 Mar 2026 09:54:22 +0100 Subject: [PATCH 15/18] Align repo activation with route shape --- src/data.ts | 23 ++++++----------------- src/data/app-data-contract.test.ts | 10 +++++----- src/ui/HomeView.tsx | 6 +++--- src/ui/RepoSwitcher.tsx | 2 +- 4 files changed, 15 insertions(+), 26 deletions(-) diff --git a/src/data.ts b/src/data.ts index 50c3b6d..a6470ce 100644 --- a/src/data.ts +++ b/src/data.ts @@ -246,11 +246,7 @@ type AppAction = // Repo selection and access checks. // Open a workspace target and optionally seed the desired file path. - | { - type: 'repo.activate'; - repo: { kind: 'new' } | { kind: 'github'; owner: string; repo: string }; - filePath?: string; - } + | { type: 'repo.activate'; target: RepoRoute } // Check whether an owner/repo appears reachable from the current session. | { type: 'repo.probe'; owner: string; repo: string } | { type: 'repo.request-access'; owner: string; repo: string } @@ -1008,21 +1004,14 @@ function useAppShellData({ route }: { route: Route }): AppShellDataResult { } if (action.type === 'repo.activate') { let currentTarget = navigation.screen === 'workspace' ? navigation.target : undefined; - if (action.repo.kind === 'github' && currentTarget?.kind === 'repo') { - let sameRepo = currentTarget.owner === action.repo.owner && currentTarget.repo === action.repo.repo; - if (sameRepo && action.filePath === undefined) { + if (action.target.kind === 'repo' && currentTarget?.kind === 'repo') { + let sameRepo = + currentTarget.owner === action.target.owner && currentTarget.repo === action.target.repo; + if (sameRepo && action.target.filePath === undefined) { return; } } - let nextTarget: RepoRoute = - action.repo.kind === 'new' - ? { kind: 'new', filePath: action.filePath } - : { - kind: 'repo', - owner: action.repo.owner, - repo: action.repo.repo, - filePath: action.filePath, - }; + let nextTarget = action.target; if (currentTarget !== undefined && areRepoRoutesEqual(currentTarget, nextTarget)) { return; } diff --git a/src/data/app-data-contract.test.ts b/src/data/app-data-contract.test.ts index c90dae7..d3a5b6c 100644 --- a/src/data/app-data-contract.test.ts +++ b/src/data/app-data-contract.test.ts @@ -340,7 +340,7 @@ describe('useAppData contract', () => { act(() => { result.current.dispatch({ type: 'repo.activate', - repo: { kind: 'github', owner: 'acme', repo: 'docs' }, + target: { kind: 'repo', owner: 'acme', repo: 'docs' }, }); }); @@ -380,7 +380,7 @@ describe('useAppData contract', () => { act(() => { result.current.dispatch({ type: 'repo.activate', - repo: { kind: 'github', owner: 'acme', repo: 'docs' }, + target: { kind: 'repo', owner: 'acme', repo: 'docs' }, }); }); @@ -447,7 +447,7 @@ describe('useAppData contract', () => { act(() => { result.current.dispatch({ type: 'repo.activate', - repo: { kind: 'github', owner: 'acme', repo: 'docs' }, + target: { kind: 'repo', owner: 'acme', repo: 'docs' }, }); }); @@ -483,7 +483,7 @@ describe('useAppData contract', () => { act(() => { result.current.dispatch({ type: 'repo.activate', - repo: { kind: 'github', owner: 'acme', repo: 'docs' }, + target: { kind: 'repo', owner: 'acme', repo: 'docs' }, }); }); @@ -528,7 +528,7 @@ describe('useAppData contract', () => { act(() => { result.current.dispatch({ type: 'repo.activate', - repo: { kind: 'github', owner: 'octocat', repo: 'Hello-World' }, + target: { kind: 'repo', owner: 'octocat', repo: 'Hello-World' }, }); }); diff --git a/src/ui/HomeView.tsx b/src/ui/HomeView.tsx index 6215305..5a4eebe 100644 --- a/src/ui/HomeView.tsx +++ b/src/ui/HomeView.tsx @@ -15,12 +15,12 @@ export function HomeView({ recents, dispatch }: HomeViewProps) { const openEntry = (entry: RecentRepo) => { if (entry.owner && entry.repo) { - dispatch({ type: 'repo.activate', repo: { kind: 'github', owner: entry.owner, repo: entry.repo } }); + dispatch({ type: 'repo.activate', target: { kind: 'repo', owner: entry.owner, repo: entry.repo } }); return; } const [owner, repo] = entry.slug.split('/', 2); if (owner && repo) { - dispatch({ type: 'repo.activate', repo: { kind: 'github', owner, repo } }); + dispatch({ type: 'repo.activate', target: { kind: 'repo', owner, repo } }); } }; @@ -30,7 +30,7 @@ export function HomeView({ recents, dispatch }: HomeViewProps) { }; const goCreateRepo = () => { - dispatch({ type: 'repo.activate', repo: { kind: 'new' } }); + dispatch({ type: 'repo.activate', target: { kind: 'new' } }); }; return ( diff --git a/src/ui/RepoSwitcher.tsx b/src/ui/RepoSwitcher.tsx index ba2c643..c9b2721 100644 --- a/src/ui/RepoSwitcher.tsx +++ b/src/ui/RepoSwitcher.tsx @@ -67,7 +67,7 @@ export function RepoSwitcher({ dispatch, probe, recents, onClose, triggerRef }: }, [dispatch, input]); const goTo = (owner: string, repo: string) => { - dispatch({ type: 'repo.activate', repo: { kind: 'github', owner, repo } }); + dispatch({ type: 'repo.activate', target: { kind: 'repo', owner, repo } }); onClose(); }; From 8d21cab834b0a7c4167f7a981f18c45e99b816fc Mon Sep 17 00:00:00 2001 From: Gregor Date: Fri, 6 Mar 2026 10:23:16 +0100 Subject: [PATCH 16/18] Move data contexts out of core data module --- src/App.tsx | 39 +++++++++++++++++++++++++++------------ src/data-context.tsx | 38 ++++++++++++++++++++++++++++++++++++++ src/ui/RepoView.tsx | 16 +--------------- 3 files changed, 66 insertions(+), 27 deletions(-) create mode 100644 src/data-context.tsx diff --git a/src/App.tsx b/src/App.tsx index 37b1491..2f4c2d2 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,12 +1,27 @@ import React, { useEffect } from 'react'; -import { repoRouteToSlug, useAppShellData, useWorkspaceAppData, type AppNavigationState } from './data'; +import { repoRouteToSlug, type AppNavigationState } from './data'; +import { AppShellProvider, RepoDataProvider, useAppDataContext, useAppShellContext } from './data-context'; import { useRoute, type Route } from './ui/routing'; import { RepoView } from './ui/RepoView'; import { HomeView } from './ui/HomeView'; export function App() { const { route, navigate } = useRoute(); - let app = useAppShellData({ route }); + return ( + + + + ); +} + +function AppScreens({ + route, + navigate, +}: { + route: Route; + navigate: (route: Route, options?: { replace?: boolean }) => void; +}) { + let app = useAppShellContext(); // Adjust page title based on route useEffect(() => { @@ -31,22 +46,22 @@ export function App() { // Mount repo state behind a slug key so repo-local hooks can assume owner/repo stay fixed. if (app.state.navigation.screen === 'workspace' && app.state.navigation.target !== undefined) { let target = app.state.navigation.target; - return ; + return ( + + + + ); } return null; } -function RepoWorkspaceScreen({ - route, - app, -}: { - route: NonNullable; - app: ReturnType; -}) { +function RepoWorkspaceScreen() { // Combine app-level shell state with the repo-scoped workspace data for RepoView. - let data = useWorkspaceAppData({ app, route }); - return ; + let data = useAppDataContext(); + let workspace = data.state.workspace; + if (workspace === undefined) return null; + return ; } function routeFromNavigation(navigation: AppNavigationState): Route | undefined { diff --git a/src/data-context.tsx b/src/data-context.tsx new file mode 100644 index 0000000..c17ae73 --- /dev/null +++ b/src/data-context.tsx @@ -0,0 +1,38 @@ +// React context bridge for the app-level and repo-level data hooks. +import { createContext, useContext, type ReactNode } from 'react'; +import { useAppShellData, useWorkspaceAppData, type AppDataResult } from './data'; +import type { Route, RepoRoute } from './ui/routing'; + +export { AppShellProvider, RepoDataProvider, useAppShellContext, useAppDataContext }; + +const AppShellContext = createContext | undefined>(undefined); +const AppDataContext = createContext(undefined); + +function AppShellProvider({ route, children }: { route: Route; children: ReactNode }) { + // App-lifetime provider: route parsing, recents, session shell state, and repo probe state. + let app = useAppShellData({ route }); + return {children}; +} + +function useAppShellContext(): ReturnType { + let value = useContext(AppShellContext); + if (value === undefined) { + throw new Error('useAppShellContext must be used inside AppShellProvider'); + } + return value; +} + +function RepoDataProvider({ route, children }: { route: RepoRoute; children: ReactNode }) { + // Repo-lifetime provider: mount this behind a repo key so repo-local hooks get a fresh lifetime. + let app = useAppShellContext(); + let data = useWorkspaceAppData({ app, route }); + return {children}; +} + +function useAppDataContext(): AppDataResult { + let value = useContext(AppDataContext); + if (value === undefined) { + throw new Error('useAppDataContext must be used inside RepoDataProvider'); + } + return value; +} diff --git a/src/ui/RepoView.tsx b/src/ui/RepoView.tsx index 88c8d35..239a9e1 100644 --- a/src/ui/RepoView.tsx +++ b/src/ui/RepoView.tsx @@ -26,7 +26,7 @@ import { ShareDialog } from './ShareDialog'; import { useOnClickOutside } from './useOnClickOutside'; type RepoViewProps = { - state: AppState; + state: AppState & { workspace: NonNullable }; dispatch: (action: AppAction) => void; helpers: AppDataResult['helpers']; }; @@ -35,20 +35,6 @@ const primaryModifier = detectPrimaryShortcut(); export function RepoView({ state, dispatch, helpers }: RepoViewProps) { let workspace = state.workspace; - if (workspace === undefined) return null; - return ( - - ); -} - -function RepoViewInner({ state, dispatch, helpers }: RepoViewProps) { - let workspace = state.workspace; - if (workspace === undefined) return null; let slug = repoRouteToSlug(workspace.target); let hasSession = state.session.status === 'signed-in'; From 978bdf07da06a9ac3e8d4a151b374a45abc4a7f6 Mon Sep 17 00:00:00 2001 From: Gregor Date: Fri, 6 Mar 2026 11:05:34 +0100 Subject: [PATCH 17/18] Fix repo switcher probe behavior --- src/data.ts | 60 +++- src/data/app-data-contract.test.ts | 48 ++- src/sync/git-sync.test.ts | 453 ++++------------------------- src/sync/git-sync.ts | 3 +- src/ui/RepoSwitcher.test.tsx | 61 ++++ src/ui/RepoSwitcher.tsx | 40 +-- 6 files changed, 240 insertions(+), 425 deletions(-) create mode 100644 src/ui/RepoSwitcher.test.tsx diff --git a/src/data.ts b/src/data.ts index a6470ce..979a83b 100644 --- a/src/data.ts +++ b/src/data.ts @@ -54,6 +54,7 @@ import type { Route, RepoRoute } from './ui/routing'; export { useAppShellData, useWorkspaceAppData, useRepoData, repoRouteToSlug }; export type { AppAction, + AppQueries, AppDataResult, AppState, AppNavigationState, @@ -190,7 +191,6 @@ type AppState = { /** Cross-workspace repo state that should survive switching between repos. */ repos: { recents: RecentRepo[]; - probe: RepoProbeState; }; /** State for the active repo/new-note workspace, if the user is currently in one. */ @@ -280,6 +280,7 @@ type AppAction = type AppDataResult = { state: AppState; dispatch: (action: AppAction) => void; + queries: AppQueries; helpers: { importPastedAssets: (params: { notePath: string; files: File[] }) => Promise; }; @@ -291,9 +292,14 @@ type AppShellState = { repos: AppState['repos']; }; +type AppQueries = { + getRepoProbe: (owner: string, repo: string) => RepoProbeState | undefined; +}; + type AppShellDataResult = { state: AppShellState; dispatch: (action: AppAction) => void; + queries: AppQueries; setWorkspaceNavigation: (route: RepoRoute, options?: { replace?: boolean }) => void; syncSession: (session: AppShellState['session']) => void; refreshRecents: () => void; @@ -929,11 +935,11 @@ function useAppShellData({ route }: { route: Route }): AppShellDataResult { // App-lifetime state: routing, recents, probe state, and coarse session info. let [session, setSession] = useState(() => readAppSessionState()); let [recents, setRecents] = useState(() => listRecentRepos()); - let [probe, setProbe] = useState({ status: 'idle' }); + let [probeCache, setProbeCache] = useState>({}); let [navigation, setNavigation] = useState(() => deriveAppNavigation(route, listRecentRepos()) ); - let probeRevisionRef = useRef(0); + let probeRevisionRef = useRef>({}); let refreshSession = useCallback(() => { let next = readAppSessionState(); @@ -954,6 +960,13 @@ function useAppShellData({ route }: { route: Route }): AppShellDataResult { setNavigation((prev) => (areAppNavigationsEqual(prev, next) ? prev : next)); }, []); + let queries = useMemo( + () => ({ + getRepoProbe: (owner, repo) => probeCache[repoProbeKey(owner, repo)], + }), + [probeCache] + ); + useEffect(() => { let onStorage = () => { refreshRecents(); @@ -1019,11 +1032,28 @@ function useAppShellData({ route }: { route: Route }): AppShellDataResult { return; } if (action.type === 'repo.probe') { - let revision = ++probeRevisionRef.current; - setProbe({ status: 'checking', owner: action.owner, repo: action.repo }); + let key = repoProbeKey(action.owner, action.repo); + let revision = (probeRevisionRef.current[key] ?? 0) + 1; + probeRevisionRef.current[key] = revision; + setProbeCache((prev) => { + let nextProbe: RepoProbeState = { status: 'checking', owner: action.owner, repo: action.repo }; + let current = prev[key]; + if (current !== undefined && areRepoProbesEqual(current, nextProbe)) return prev; + return { ...prev, [key]: nextProbe }; + }); void repoExists(action.owner, action.repo).then((exists) => { - if (probeRevisionRef.current !== revision) return; - setProbe({ status: 'ready', owner: action.owner, repo: action.repo, exists }); + if (probeRevisionRef.current[key] !== revision) return; + setProbeCache((prev) => { + let nextProbe: RepoProbeState = { + status: 'ready', + owner: action.owner, + repo: action.repo, + exists, + }; + let current = prev[key]; + if (current !== undefined && areRepoProbesEqual(current, nextProbe)) return prev; + return { ...prev, [key]: nextProbe }; + }); }); } }, @@ -1036,10 +1066,10 @@ function useAppShellData({ route }: { route: Route }): AppShellDataResult { navigation, repos: { recents, - probe, }, }, dispatch, + queries, setWorkspaceNavigation, syncSession: useCallback((next) => { setSession((prev) => (areSessionStatesEqual(prev, next) ? prev : next)); @@ -1219,6 +1249,7 @@ function useWorkspaceAppData({ app, route }: { app: AppShellDataResult; route: R return { state, dispatch, + queries: app.queries, helpers: { importPastedAssets: (params) => workspaceData.actions.importPastedAssets(params), }, @@ -1594,6 +1625,19 @@ function areSessionStatesEqual(a: AppShellState['session'], b: AppShellState['se ); } +function repoProbeKey(owner: string, repo: string) { + return `${owner.toLowerCase()}/${repo.toLowerCase()}`; +} + +function areRepoProbesEqual(a: RepoProbeState, b: RepoProbeState) { + return ( + a.status === b.status && + a.owner === b.owner && + a.repo === b.repo && + a.exists === b.exists + ); +} + function repoRouteToSlug(route: RepoRoute): string { if (route.kind === 'new') return 'new'; return `${route.owner}/${route.repo}`; diff --git a/src/data/app-data-contract.test.ts b/src/data/app-data-contract.test.ts index d3a5b6c..5650307 100644 --- a/src/data/app-data-contract.test.ts +++ b/src/data/app-data-contract.test.ts @@ -215,6 +215,7 @@ function HomeDataHarness({ workspace: undefined, }, dispatch: app.dispatch, + queries: app.queries, helpers: { importPastedAssets: async () => [], }, @@ -593,12 +594,14 @@ describe('useAppData contract', () => { ); }); - test('tracks repo probe state and ignores stale probe results', async () => { + test('tracks probe state per repo and ignores stale results for the same repo', async () => { let firstProbe = createDeferred(); let secondProbe = createDeferred(); + let thirdProbe = createDeferred(); mockRepoExists .mockImplementationOnce(() => firstProbe.promise) - .mockImplementationOnce(() => secondProbe.promise); + .mockImplementationOnce(() => secondProbe.promise) + .mockImplementationOnce(() => thirdProbe.promise); let { result } = renderAppData({ kind: 'home' }); @@ -606,7 +609,7 @@ describe('useAppData contract', () => { result.current.dispatch({ type: 'repo.probe', owner: 'acme', repo: 'docs' }); }); - expect(result.current.state.repos.probe).toEqual({ + expect(result.current.queries.getRepoProbe('acme', 'docs')).toEqual({ status: 'checking', owner: 'acme', repo: 'docs', @@ -616,7 +619,7 @@ describe('useAppData contract', () => { result.current.dispatch({ type: 'repo.probe', owner: 'space', repo: 'wiki' }); }); - expect(result.current.state.repos.probe).toEqual({ + expect(result.current.queries.getRepoProbe('space', 'wiki')).toEqual({ status: 'checking', owner: 'space', repo: 'wiki', @@ -627,7 +630,16 @@ describe('useAppData contract', () => { await firstProbe.promise; }); - expect(result.current.state.repos.probe).toEqual({ + await waitFor(() => + expect(result.current.queries.getRepoProbe('acme', 'docs')).toEqual({ + status: 'ready', + owner: 'acme', + repo: 'docs', + exists: true, + }) + ); + + expect(result.current.queries.getRepoProbe('space', 'wiki')).toEqual({ status: 'checking', owner: 'space', repo: 'wiki', @@ -639,13 +651,37 @@ describe('useAppData contract', () => { }); await waitFor(() => - expect(result.current.state.repos.probe).toEqual({ + expect(result.current.queries.getRepoProbe('space', 'wiki')).toEqual({ status: 'ready', owner: 'space', repo: 'wiki', exists: false, }) ); + + act(() => { + result.current.dispatch({ type: 'repo.probe', owner: 'space', repo: 'wiki' }); + }); + + expect(result.current.queries.getRepoProbe('space', 'wiki')).toEqual({ + status: 'checking', + owner: 'space', + repo: 'wiki', + }); + + await act(async () => { + thirdProbe.resolve(true); + await thirdProbe.promise; + }); + + await waitFor(() => + expect(result.current.queries.getRepoProbe('space', 'wiki')).toEqual({ + status: 'ready', + owner: 'space', + repo: 'wiki', + exists: true, + }) + ); }); test('reflects sign-in and sign-out outcomes through session state', async () => { diff --git a/src/sync/git-sync.test.ts b/src/sync/git-sync.test.ts index d57388f..892cd8a 100644 --- a/src/sync/git-sync.test.ts +++ b/src/sync/git-sync.test.ts @@ -1,408 +1,81 @@ -import { Buffer } from 'node:buffer'; -import { beforeEach, describe, expect, test, vi } from 'vitest'; -import { LocalStore, listTombstones, findBySyncedHash } from '../storage/local'; -import { MockRemoteRepo } from '../test/mock-remote'; +// Unit tests for git-sync repo reachability helpers. +import { beforeEach, describe, expect, it, vi } from 'vitest'; -const authModule = vi.hoisted(() => ({ - ensureFreshAccessToken: vi.fn().mockResolvedValue('test-token'), -})); - -vi.mock('../auth/app-auth', () => authModule); +import type { RepoMetadata } from '../lib/backend'; -const globalAny = globalThis as { - fetch?: typeof fetch; +type BackendMocks = { + getRepoMetadata: ReturnType; }; -const remoteScenarios: Array<{ - label: string; - configure(remote: MockRemoteRepo): void; -}> = [ - { - label: 'fresh remote responses', - configure(remote) { - remote.enableStaleReads({ enabled: false }); - }, - }, - { - label: 'stale remote responses with random delay', - configure(remote) { - const windowMs = Math.floor(Math.random() * 901) + 100; - remote.enableStaleReads({ enabled: true, windowMs }); - }, - }, -]; - -describe.each(remoteScenarios)('syncBidirectional: $label', ({ configure }) => { - let store: LocalStore; - let remote: MockRemoteRepo; - let syncBidirectional: typeof import('./git-sync').syncBidirectional; - - beforeEach(async () => { - authModule.ensureFreshAccessToken.mockReset(); - authModule.ensureFreshAccessToken.mockResolvedValue('test-token'); - remote = new MockRemoteRepo(); - remote.configure('user', 'repo'); - remote.allowToken('test-token'); - configure(remote); - const fetchMock = vi.fn((input: RequestInfo | URL, init?: RequestInit) => - remote.handleFetch(input, init) - ); - globalAny.fetch = fetchMock as unknown as typeof fetch; - const mod = await import('./git-sync'); - syncBidirectional = mod.syncBidirectional; - store = new LocalStore('user/repo'); - }); - - test('pushes new notes and remains stable', async () => { - const firstId = store.createFile('First.md', 'first note'); - const secondId = store.createFile('Second.md', 'second note'); - await syncBidirectional(store, 'user/repo'); - await syncBidirectional(store, 'user/repo'); - expectParity(store, remote); - expect(listTombstones(store.slug)).toHaveLength(0); - const firstDoc = store.loadFileById(firstId); - const secondDoc = store.loadFileById(secondId); - expect(firstDoc?.path).toBe('First.md'); - expect(secondDoc?.path).toBe('Second.md'); - }); - - test('applies local deletions to remote without resurrection', async () => { - store.createFile('Ghost.md', 'haunt me'); - await syncBidirectional(store, 'user/repo'); - store.deleteFile('Ghost.md'); - await syncBidirectional(store, 'user/repo'); - expectParity(store, remote); - expect(store.listFiles()).toHaveLength(0); - expect(listTombstones(store.slug)).toHaveLength(0); - }); - - test('renames move files remotely', async () => { - store.createFile('Original.md', 'rename me'); - await syncBidirectional(store, 'user/repo'); - store.renameFile('Original.md', 'Renamed'); - await syncBidirectional(store, 'user/repo'); - expectParity(store, remote); - const notes = store.listFiles(); - expect(notes).toHaveLength(1); - expect(notes[0]?.path).toBe('Renamed.md'); - expect([...remote.snapshot().keys()]).toEqual(['Renamed.md']); - }); - - test('rename removes old remote path after prior sync', async () => { - store.createFile('test.md', 'body'); - await syncBidirectional(store, 'user/repo'); - expect([...remote.snapshot().keys()]).toEqual(['test.md']); - store.renameFile('test.md', 'test2'); - await syncBidirectional(store, 'user/repo'); - const remoteFiles = [...remote.snapshot().keys()].sort(); - expect(remoteFiles).toEqual(['test2.md']); - expectParity(store, remote); - }); - - test('rename with remote edits keeps both copies in sync', async () => { - store.createFile('draft.md', 'original body'); - await syncBidirectional(store, 'user/repo'); - remote.setFile('draft.md', 'remote update'); - store.renameFile('draft.md', 'draft-renamed'); - await syncBidirectional(store, 'user/repo'); - const paths = [...remote.snapshot().keys()].sort(); - expect(paths).toEqual(['draft-renamed.md', 'draft.md']); - expectParity(store, remote); - const localPaths = store - .listFiles() - .map((n) => n.path) - .sort(); - expect(localPaths).toEqual(['draft-renamed.md', 'draft.md']); - }); - - test('rename revert does not push redundant commits', async () => { - store.createFile('first-name.md', 'body'); - await syncBidirectional(store, 'user/repo'); - const headBeforeRename = await getRemoteHeadSha(remote); - - store.renameFile('first-name.md', 'second-name'); - store.renameFile('second-name.md', 'first-name'); - - await syncBidirectional(store, 'user/repo'); - - const headAfterSync = await getRemoteHeadSha(remote); - expect(headAfterSync).toBe(headBeforeRename); - expectParity(store, remote); - }); - - test('rename followed by local edit pushes updated content under new path', async () => { - store.createFile('Draft.md', 'initial body'); - await syncBidirectional(store, 'user/repo'); - expect([...remote.snapshot().keys()]).toEqual(['Draft.md']); - - const nextPath = store.renameFile('Draft.md', 'Ready'); - expect(nextPath).toBe('Ready.md'); - store.saveFile('Ready.md', 'edited after rename'); - - await syncBidirectional(store, 'user/repo'); - - const remoteFiles = [...remote.snapshot().entries()]; - expect(remoteFiles).toEqual([['Ready.md', 'edited after rename']]); - const readyMeta = store.listFiles().find((file) => file.path === 'Ready.md'); - const readyDoc = readyMeta ? store.loadFileById(readyMeta.id) : null; - expect(readyDoc?.content).toBe('edited after rename'); - expect(listTombstones(store.slug)).toHaveLength(0); - }); - - test('surface 422 when branch head advances during push', async () => { - store.createFile('Lonely.md', 'seed text'); - await syncBidirectional(store, 'user/repo'); - - store.saveFile('Lonely.md', 'edited locally'); - remote.advanceHeadOnNextUpdate(); - - await expect(syncBidirectional(store, 'user/repo')).rejects.toMatchObject({ - status: 422, - path: expect.stringContaining('/git/refs/heads/'), - }); - }); - - test('pulls new remote notes', async () => { - remote.setFile('Remote.md', '# remote'); - await syncBidirectional(store, 'user/repo'); - expectParity(store, remote); - const notes = store.listFiles(); - expect(notes).toHaveLength(1); - const doc = store.loadFileById(notes[0]?.id ?? ''); - expect(doc?.content).toBe('# remote'); - }); - - test('removes notes when deleted remotely', async () => { - store.createFile('Shared.md', 'shared text'); - await syncBidirectional(store, 'user/repo'); - remote.deleteDirect('Shared.md'); - await syncBidirectional(store, 'user/repo'); - expectParity(store, remote); - expect(store.listFiles()).toHaveLength(0); - }); +type PublicMocks = { + fetchPublicRepoInfo: ReturnType; +}; - test('syncs tracked image files while ignoring unrelated blobs', async () => { - // .xyz is an unknown extension — should be ignored by the sync - remote.setFile('data.xyz', 'ignored'); - remote.setFile('image.png', 'asset'); - store.createFile('OnlyNote.md', '# hello'); - await syncBidirectional(store, 'user/repo'); - const snapshot = remote.snapshot(); - // Unknown extension stays on remote but is not pulled to local store - expect(snapshot.get('data.xyz')).toBe('ignored'); - expect(store.listFiles().find((f) => f.path === 'data.xyz')).toBeUndefined(); - expect(snapshot.get('image.png')).toBe('asset'); - expect(snapshot.get('OnlyNote.md')).toBe('# hello'); - const files = store.listFiles(); - const imageMeta = files.find((f) => f.path === 'image.png'); - expect(imageMeta).toBeDefined(); - if (imageMeta) { - const imageDoc = store.loadFileById(imageMeta.id); - expect(imageDoc?.kind).toBe('asset-url'); - expect(imageDoc?.content).toMatch(/^gh-blob:/); - } - expectParity(store, remote); - }); +const backendModule = vi.hoisted(() => ({ + getRepoMetadata: vi.fn(), +})); - test('pulls nested Markdown files', async () => { - remote.setFile('nested/Nested.md', '# nested'); - await syncBidirectional(store, 'user/repo'); - const notes = store.listFiles(); - expect(notes).toHaveLength(1); - const doc = store.loadFileById(notes[0]?.id ?? ''); - expect(doc?.path).toBe('nested/Nested.md'); - expect(doc?.content).toBe('# nested'); - }); +const publicModule = vi.hoisted(() => ({ + fetchPublicRepoInfo: vi.fn(), +})); - test('pulls binary image assets from remote', async () => { - remote.setFile('assets/logo.png', 'image-data'); - await syncBidirectional(store, 'user/repo'); - const files = store.listFiles(); - const asset = files.find((f) => f.path === 'assets/logo.png'); - expect(asset).toBeDefined(); - if (!asset) return; - const doc = store.loadFileById(asset.id); - expect(doc?.kind).toBe('asset-url'); - expect(doc?.content).toMatch(/^gh-blob:/); - expectParity(store, remote); - }); +vi.mock('../lib/backend', async () => { + let actual = await vi.importActual('../lib/backend'); + return { + ...actual, + getRepoMetadata: backendModule.getRepoMetadata, + }; +}); - test('locally created binary assets convert to blob placeholders after the subsequent sync', async () => { - const base64 = Buffer.from('clipboard-image').toString('base64'); - const id = store.createFile('assets/paste.png', base64); +vi.mock('../lib/github-public', async () => { + let actual = await vi.importActual('../lib/github-public'); + return { + ...actual, + fetchPublicRepoInfo: publicModule.fetchPublicRepoInfo, + }; +}); - await syncBidirectional(store, 'user/repo'); - const afterFirstSync = store.loadFileById(id); - expect(afterFirstSync?.kind).toBe('asset-url'); - expect(afterFirstSync?.content).toMatch(/^gh-blob:/); - const firstPlaceholder = afterFirstSync?.content; +let repoExists: typeof import('./git-sync').repoExists; - await syncBidirectional(store, 'user/repo'); - const afterSecondSync = store.loadFileById(id); - expect(afterSecondSync?.kind).toBe('asset-url'); - expect(afterSecondSync?.content).toBe(firstPlaceholder); - }); +beforeEach(async () => { + ({ repoExists } = await import('./git-sync')); +}); - test('pulls binary assets via blob fallback when contents payload is empty', async () => { - const payload = 'high-res-image'; - const expectedBase64 = Buffer.from(payload, 'utf8').toString('base64'); - remote.setFile('assets/large.png', payload); - const originalFetch = globalAny.fetch!; - let capturedSha: string | null = null; - const interceptFetch = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { - const request = input instanceof Request ? input : new Request(input, init); - const url = new URL(request.url); - if ( - request.method.toUpperCase() === 'GET' && - url.pathname === '/repos/user/repo/contents/assets/large.png' - ) { - const upstream = await originalFetch(input, init); - const json = await upstream.json(); - capturedSha = typeof json?.sha === 'string' ? json.sha : null; - return new Response(JSON.stringify({ ...json, content: '' }), { - status: 200, - headers: { 'Content-Type': 'application/json' }, - }); - } - if ( - request.method.toUpperCase() === 'GET' && - capturedSha && - url.pathname === `/repos/user/repo/git/blobs/${capturedSha}` - ) { - return new Response( - JSON.stringify({ sha: capturedSha, content: expectedBase64, encoding: 'base64' }), - { status: 200, headers: { 'Content-Type': 'application/json' } } - ); - } - return originalFetch(input, init); +describe('repoExists', () => { + beforeEach(() => { + backendModule.getRepoMetadata.mockReset(); + publicModule.fetchPublicRepoInfo.mockReset(); + }); + + it('does not treat owner-level installation as proof that a repo exists', async () => { + let metadata: RepoMetadata = { + isPrivate: null, + installed: true, + repoSelected: false, + defaultBranch: null, + manageUrl: null, + errorKind: 'not-found', + }; + backendModule.getRepoMetadata.mockResolvedValue(metadata); + publicModule.fetchPublicRepoInfo.mockResolvedValue({ + ok: false, + notFound: true, + status: 404, }); - globalAny.fetch = interceptFetch as unknown as typeof fetch; - try { - await syncBidirectional(store, 'user/repo'); - } finally { - globalAny.fetch = originalFetch; - } - const files = store.listFiles(); - const asset = files.find((f) => f.path === 'assets/large.png'); - expect(asset).toBeDefined(); - if (!asset) return; - const doc = store.loadFileById(asset.id); - expect(doc?.kind).toBe('asset-url'); - expect(doc?.content).toMatch(/^gh-blob:/); - expect(capturedSha).toBeTruthy(); - expectParity(store, remote); - }); - - test('tracks remote binary renames by sha/hash', async () => { - const payload = Buffer.from('asset', 'utf8').toString('base64'); - const id = store.createFile('logo.png', payload); - await syncBidirectional(store, 'user/repo'); - const before = store.loadFileById(id); - expect(before?.lastSyncedHash).toBeDefined(); - remote.deleteDirect('logo.png'); - remote.setFile('assets/logo.png', payload); - if (before?.lastSyncedHash) { - const lookup = findBySyncedHash(store.slug, before.lastSyncedHash); - expect(lookup?.id).toBe(id); - } - - await syncBidirectional(store, 'user/repo'); - - const paths = store - .listFiles() - .map((f) => f.path) - .sort(); - expect(paths).toContain('assets/logo.png'); - expect(paths).not.toContain('logo.png'); - const renamedFile = store.listFiles().find((f) => f.path === 'assets/logo.png'); - expect(renamedFile).toBeDefined(); - expectParity(store, remote); - }); - - test('listRepoFiles includes nested markdown', async () => { - const mod = await import('./git-sync'); - remote.setFile('nested/Nested.md', '# nested'); - let cfg = mod.buildRemoteConfig('user/repo'); - let entries = await mod.listRepoFiles(cfg); - const paths = entries.map((e) => e.path).sort(); - expect(paths).toEqual(['nested/Nested.md']); + await expect(repoExists('mitschabaude', 'montgom')).resolves.toBe(false); }); - test('listRepoFiles returns markdown and image entries', async () => { - const mod = await import('./git-sync'); - remote.setFile('docs/Doc.md', '# hi'); - remote.setFile('assets/logo.png', 'img'); - let cfg = mod.buildRemoteConfig('user/repo'); - let entries = await mod.listRepoFiles(cfg); - const byPath = new Map(entries.map((entry) => [entry.path, entry.kind])); - expect(byPath.get('docs/Doc.md')).toBe('markdown'); - expect(byPath.get('assets/logo.png')).toBe('binary'); - }); + it('accepts repos that are selected in the current installation', async () => { + let metadata: RepoMetadata = { + isPrivate: true, + installed: true, + repoSelected: true, + defaultBranch: 'main', + manageUrl: null, + }; + backendModule.getRepoMetadata.mockResolvedValue(metadata); - test('includes README.md files from the repository', async () => { - remote.setFile('README.md', 'root readme'); - remote.setFile('sub/README.md', 'sub readme'); - await syncBidirectional(store, 'user/repo'); - const paths = store - .listFiles() - .map((n) => n.path) - .sort(); - expect(paths).toEqual(['README.md', 'sub/README.md']); + await expect(repoExists('acme', 'private-notes')).resolves.toBe(true); }); }); - -type RemoteHeadPayload = { - object?: { sha?: string }; -}; - -async function getRemoteHeadSha(remote: MockRemoteRepo, branch = 'main'): Promise { - const response = await remote.handleFetch( - `https://api.github.com/repos/user/repo/git/ref/heads/${branch}`, - { method: 'GET' } - ); - const payload = (await response.json()) as RemoteHeadPayload; - if (!response.ok) { - throw new Error(`remote head lookup failed with status ${response.status}`); - } - const sha = typeof payload.object?.sha === 'string' ? payload.object.sha : ''; - expect(sha).not.toBe(''); - return sha; -} - -function expectParity(store: LocalStore, remote: MockRemoteRepo) { - const localDocs = new Map>(); - for (const meta of store.listFiles()) { - const doc = store.loadFileById(meta.id); - if (!doc) continue; - localDocs.set(meta.path, doc); - } - const remoteMap = remote.snapshot(); - const trackedRemoteKeys = [...remoteMap.keys()].filter(isTrackedPath).sort(); - expect(trackedRemoteKeys).toEqual([...localDocs.keys()].sort()); - for (const [path, doc] of localDocs.entries()) { - const remoteContent = remoteMap.get(path); - if (doc?.kind === 'markdown') { - expect(remoteContent).toBe(doc.content); - } else if (doc?.kind === 'binary') { - const decoded = Buffer.from(doc.content, 'base64').toString('utf8'); - expect(remoteContent).toBe(decoded); - } else if (doc?.kind === 'asset-url') { - expect(remoteContent).toBeDefined(); - } - } -} - -function isTrackedPath(path: string): boolean { - const lower = path.toLowerCase(); - return ( - lower.endsWith('.md') || - lower.endsWith('.png') || - lower.endsWith('.jpg') || - lower.endsWith('.jpeg') || - lower.endsWith('.gif') || - lower.endsWith('.webp') || - lower.endsWith('.svg') || - lower.endsWith('.avif') - ); -} diff --git a/src/sync/git-sync.ts b/src/sync/git-sync.ts index 2c3292a..211e8a9 100644 --- a/src/sync/git-sync.ts +++ b/src/sync/git-sync.ts @@ -52,7 +52,8 @@ export function buildRemoteConfig(slug: string, branch?: string): RemoteConfig { export async function repoExists(owner: string, repo: string): Promise { try { let meta = await getRepoMetadata(owner, repo); - if (meta.installed) return true; + // Installation alone is owner-scoped and does not prove the repo itself exists. + if (meta.repoSelected) return true; if (meta.isPrivate === false) return true; if (meta.isPrivate === true) return false; } catch { diff --git a/src/ui/RepoSwitcher.test.tsx b/src/ui/RepoSwitcher.test.tsx new file mode 100644 index 0000000..06e5eaf --- /dev/null +++ b/src/ui/RepoSwitcher.test.tsx @@ -0,0 +1,61 @@ +// Unit tests for RepoSwitcher probe behavior. +import React from 'react'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { AppQueries } from '../data'; +import { RepoSwitcher } from './RepoSwitcher'; + +function renderSwitcher({ + queries, + dispatch = vi.fn(), +}: { + queries: AppQueries; + dispatch?: ReturnType; +}) { + cleanup(); + return { + dispatch, + ...render(), + }; +} + +describe('RepoSwitcher', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + cleanup(); + }); + + it('does not re-probe the same repo on rerender once a probe exists', () => { + let probeStatus: AppQueries['getRepoProbe'] = () => undefined; + let queries: AppQueries = { + getRepoProbe: (owner, repo) => probeStatus(owner, repo), + }; + let dispatch = vi.fn(); + let view = renderSwitcher({ dispatch, queries }); + + fireEvent.change(screen.getByPlaceholderText('owner/repo'), { + target: { value: 'acme/docs' }, + }); + + vi.advanceTimersByTime(300); + expect(dispatch).toHaveBeenCalledTimes(1); + expect(dispatch).toHaveBeenCalledWith({ type: 'repo.probe', owner: 'acme', repo: 'docs' }); + + probeStatus = () => ({ status: 'checking', owner: 'acme', repo: 'docs' }); + view.rerender(); + + vi.advanceTimersByTime(600); + expect(dispatch).toHaveBeenCalledTimes(1); + + probeStatus = () => ({ status: 'ready', owner: 'acme', repo: 'docs', exists: true }); + view.rerender(); + + vi.advanceTimersByTime(600); + expect(dispatch).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/ui/RepoSwitcher.tsx b/src/ui/RepoSwitcher.tsx index c9b2721..3c1e7e5 100644 --- a/src/ui/RepoSwitcher.tsx +++ b/src/ui/RepoSwitcher.tsx @@ -1,16 +1,11 @@ import { useEffect, useMemo, useRef, useState, type RefObject } from 'react'; -import type { AppAction } from '../data'; +import type { AppAction, AppQueries } from '../data'; import type { RecentRepo } from '../storage/local'; import { useOnClickOutside } from './useOnClickOutside'; type Props = { dispatch: (action: AppAction) => void; - probe: { - status: 'idle' | 'checking' | 'ready'; - owner?: string; - repo?: string; - exists?: boolean; - }; + queries: AppQueries; recents: RecentRepo[]; onClose: () => void; triggerRef?: RefObject; @@ -25,11 +20,18 @@ function parseOwnerRepo(input: string): Parsed { return { owner, repo }; } -export function RepoSwitcher({ dispatch, probe, recents, onClose, triggerRef }: Props) { +export function RepoSwitcher({ dispatch, queries, recents, onClose, triggerRef }: Props) { const [input, setInput] = useState(''); const [selectedIndex, setSelectedIndex] = useState(0); const panelRef = useOnClickOutside(onClose, { trigger: triggerRef }); const inputRef = useRef(null); + const dispatchRef = useRef(dispatch); + const parsed = parseOwnerRepo(input); + const probe = parsed === null ? undefined : queries.getRepoProbe(parsed.owner, parsed.repo); + + useEffect(() => { + dispatchRef.current = dispatch; + }, [dispatch]); useEffect(() => { inputRef.current?.focus(); @@ -52,19 +54,22 @@ export function RepoSwitcher({ dispatch, probe, recents, onClose, triggerRef }: setSelectedIndex(0); }, [suggestions.length]); - // Debounced existence check for precise owner/repo inputs + // Debounced existence check for precise owner/repo inputs. + // Once a matching probe is already in-flight or cached, do not re-dispatch it. useEffect(() => { - const parsed = parseOwnerRepo(input); if (!parsed) { return; } - const t = setTimeout(async () => { - dispatch({ type: 'repo.probe', owner: parsed.owner, repo: parsed.repo }); + if (probe?.status === 'checking' || probe?.status === 'ready') { + return; + } + const t = setTimeout(() => { + dispatchRef.current({ type: 'repo.probe', owner: parsed.owner, repo: parsed.repo }); }, 300); return () => { clearTimeout(t); }; - }, [dispatch, input]); + }, [input, parsed?.owner, parsed?.repo, probe?.status]); const goTo = (owner: string, repo: string) => { dispatch({ type: 'repo.activate', target: { kind: 'repo', owner, repo } }); @@ -84,13 +89,8 @@ export function RepoSwitcher({ dispatch, probe, recents, onClose, triggerRef }: if (parsed) goTo(parsed.owner, parsed.repo); }; - const parsed = parseOwnerRepo(input); - const probeMatchesInput = - parsed !== null && - probe.owner?.toLowerCase() === parsed.owner.toLowerCase() && - probe.repo?.toLowerCase() === parsed.repo.toLowerCase(); - const checking = probeMatchesInput && probe.status === 'checking'; - const exists = probeMatchesInput && probe.status === 'ready' ? (probe.exists ?? null) : null; + const checking = probe?.status === 'checking'; + const exists = probe?.status === 'ready' ? (probe.exists ?? null) : null; const statusText = checking ? 'Checking repository…' From 059f8c96e2abfed931fb06e97c117c9a53977323 Mon Sep 17 00:00:00 2001 From: Gregor Date: Fri, 6 Mar 2026 11:06:33 +0100 Subject: [PATCH 18/18] Document data layer query direction --- docs/data-layer-notes.md | 23 +++++++++++++++++++++++ src/App.tsx | 9 ++++++++- src/ui/RepoView.tsx | 5 +++-- 3 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 docs/data-layer-notes.md diff --git a/docs/data-layer-notes.md b/docs/data-layer-notes.md new file mode 100644 index 0000000..0b36ebe --- /dev/null +++ b/docs/data-layer-notes.md @@ -0,0 +1,23 @@ +# Data Layer Notes + +Short notes on directions we want to preserve while evolving the UI/data boundary. + +## Richer Communication + +The current model of `dispatch(action)` plus one returned `state` object is useful, but not rich enough for every UI pattern. + +Two expansions we want to keep in mind: + +- add synchronous `queries` for cached/looked-up data that should not be forced into one durable state object +- consider an outbound effect/event channel for ephemeral async results that are not naturally durable state + +## Example: Repo Probe + +`repo.probe` is a good example of data that feels awkward as a single "latest probe" field in app state. + +Prefer: + +- `dispatch({ type: 'repo.probe', ... })` to request work +- `queries.getRepoProbe(owner, repo)` to read cached probe results synchronously + +This lets the data layer keep a cache of probe results without pretending the latest probe is durable app state. diff --git a/src/App.tsx b/src/App.tsx index 2f4c2d2..77ee0f8 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -61,7 +61,14 @@ function RepoWorkspaceScreen() { let data = useAppDataContext(); let workspace = data.state.workspace; if (workspace === undefined) return null; - return ; + return ( + + ); } function routeFromNavigation(navigation: AppNavigationState): Route | undefined { diff --git a/src/ui/RepoView.tsx b/src/ui/RepoView.tsx index 239a9e1..09a2ea8 100644 --- a/src/ui/RepoView.tsx +++ b/src/ui/RepoView.tsx @@ -28,12 +28,13 @@ import { useOnClickOutside } from './useOnClickOutside'; type RepoViewProps = { state: AppState & { workspace: NonNullable }; dispatch: (action: AppAction) => void; + queries: AppDataResult['queries']; helpers: AppDataResult['helpers']; }; const primaryModifier = detectPrimaryShortcut(); -export function RepoView({ state, dispatch, helpers }: RepoViewProps) { +export function RepoView({ state, dispatch, queries, helpers }: RepoViewProps) { let workspace = state.workspace; let slug = repoRouteToSlug(workspace.target); @@ -497,8 +498,8 @@ export function RepoView({ state, dispatch, helpers }: RepoViewProps) { {showSwitcher && ( setShowSwitcher(false)} triggerRef={repoButtonRef} />