From 47ba9dd7e9c2cabfdecc2cb41ae883bbce5fdb72 Mon Sep 17 00:00:00 2001 From: Sam Chau Date: Sat, 17 Jan 2026 15:25:24 +1030 Subject: [PATCH] feat: manual incoming changes handling - Enhanced Git class to include method for fetching incoming changes from remote repository. - Implemented logic to retrieve and display incoming commits in the changes page. - Updated API routes to handle incoming changes and pull requests. - Modified UI components to show incoming changes and allow users to pull updates. - Improved actions bar to disable commit actions when there are incoming changes. - Added sync button to refresh repository status and check for updates. --- docs/todo/2.manual-pull-handling.md | 302 ++++++++++ src/lib/server/utils/git/Git.ts | 3 +- src/lib/server/utils/git/status.ts | 57 +- src/lib/server/utils/git/types.ts | 6 + .../api/databases/[id]/changes/+server.ts | 22 +- src/routes/databases/[id]/+layout.svelte | 16 +- .../databases/[id]/changes/+page.server.ts | 41 +- .../databases/[id]/changes/+page.svelte | 546 ++++++++++++------ .../components/ChangesActionsBar.svelte | 36 +- .../[id]/changes/components/StatusCard.svelte | 27 +- 10 files changed, 832 insertions(+), 224 deletions(-) create mode 100644 docs/todo/2.manual-pull-handling.md diff --git a/docs/todo/2.manual-pull-handling.md b/docs/todo/2.manual-pull-handling.md new file mode 100644 index 0000000..4ab660b --- /dev/null +++ b/docs/todo/2.manual-pull-handling.md @@ -0,0 +1,302 @@ +# Manual Pull Handling for Databases + +**Status: Complete** + +## Summary + +When `auto_pull = 0`, users receive notifications that updates are available but +have no way to review or pull them. This document plans extending the existing +`/databases/[id]/changes` page to support incoming changes (pull) alongside +outgoing changes (push). + +--- + +## Current State + +### The `/databases/[id]/changes` Page + +Currently this page: + +- Only accessible to developers (requires `personal_access_token`) +- Shows **outgoing changes** (uncommitted local ops) +- Allows: select files, write commit message, push to remote +- Allows: discard local changes +- Allows: switch branches + +### The Gap + +When `auto_pull = 0` and updates are found: + +1. User receives notification "Updates available for X" +2. User has no way to see what those updates contain +3. User has no way to pull them from the UI + +--- + +## Proposed Solution + +Extend the changes page to show both directions: + +| Section | Who Sees It | Description | +| ----------------- | ----------- | ---------------------------------- | +| Incoming Changes | Everyone | Commits available to pull | +| Outgoing Changes | Developers | Uncommitted local ops to push | + +### User Flow + +**For regular users (no PAT):** + +1. Navigate to `/databases/[id]/changes` +2. See "Incoming Changes" section with commits behind +3. Review the changes (files modified in each commit) +4. Click "Pull Updates" to sync + +**For developers (with PAT):** + +1. Same as above, plus... +2. See "Outgoing Changes" section with uncommitted ops +3. Full commit/push/discard functionality + +--- + +## Implementation Plan + +### 1. Remove PAT Requirement for Page Access + +**File:** `src/routes/databases/[id]/changes/+page.server.ts` + +```typescript +export const load: PageServerLoad = async ({ parent }) => { + const { database } = await parent(); + // Remove the PAT check - page is now accessible to everyone + // PAT only needed for push actions + return { + isDeveloper: !!database.personal_access_token + }; +}; +``` + +### 2. Update API to Return Incoming Changes + +**File:** `src/routes/api/databases/[id]/changes/+server.ts` + +Remove PAT requirement for GET. Add incoming changes data: + +```typescript +export const GET: RequestHandler = async ({ params }) => { + const database = databaseInstancesQueries.getById(id); + const git = new Git(database.local_path); + + // Fetch for everyone + const [status, incomingChanges, branches, repoInfo] = await Promise.all([ + git.status(), + git.getIncomingChanges(), + git.getBranches(), + getRepoInfo(database.repository_url, database.personal_access_token) + ]); + + // Only fetch outgoing changes for developers + let uncommittedOps = null; + if (database.personal_access_token) { + uncommittedOps = await git.getUncommittedOps(); + } + + return json({ + status, + incomingChanges, + branches, + repoInfo, + uncommittedOps + }); +}; +``` + +### 3. Add Git Function for Incoming Changes + +**File:** `src/lib/server/utils/git/status.ts` + +```typescript +export interface IncomingCommit { + hash: string; + shortHash: string; + message: string; + author: string; + date: string; + files: string[]; +} + +export interface IncomingChanges { + hasUpdates: boolean; + commitsBehind: number; + commits: IncomingCommit[]; +} + +export async function getIncomingChanges(repoPath: string): Promise { + // Fetch latest from remote + await execGitSafe(['fetch'], repoPath); + + const branch = await getBranch(repoPath); + const remoteBranch = `origin/${branch}`; + + // Count commits behind + const countOutput = await execGitSafe( + ['rev-list', '--count', `HEAD..${remoteBranch}`], + repoPath + ); + const commitsBehind = parseInt(countOutput || '0') || 0; + + if (commitsBehind === 0) { + return { hasUpdates: false, commitsBehind: 0, commits: [] }; + } + + // Get commit details for incoming commits + const logOutput = await execGitSafe( + ['log', '--format=%H|%h|%s|%an|%aI', `HEAD..${remoteBranch}`], + repoPath + ); + + const commits: IncomingCommit[] = []; + for (const line of logOutput.split('\n').filter(Boolean)) { + const [hash, shortHash, message, author, date] = line.split('|'); + + // Get files changed in this commit + const filesOutput = await execGitSafe( + ['diff-tree', '--no-commit-id', '--name-only', '-r', hash], + repoPath + ); + const files = filesOutput.split('\n').filter(Boolean); + + commits.push({ hash, shortHash, message, author, date, files }); + } + + return { hasUpdates: true, commitsBehind, commits }; +} +``` + +### 4. Add Pull Action + +**File:** `src/routes/databases/[id]/changes/+page.server.ts` + +```typescript +export const actions: Actions = { + // ... existing actions ... + + pull: async ({ params }) => { + const id = parseInt(params.id || '', 10); + const database = databaseInstancesQueries.getById(id); + + if (!database) { + return { success: false, error: 'Database not found' }; + } + + try { + const result = await pcdManager.sync(id); + return result; + } catch (err) { + return { + success: false, + error: err instanceof Error ? err.message : 'Failed to pull' + }; + } + } +}; +``` + +### 5. Update Page UI + +**File:** `src/routes/databases/[id]/changes/+page.svelte` + +```svelte + + + +
+

Incoming Changes

+ + {#if incomingChanges?.hasUpdates} +

{incomingChanges.commitsBehind} commits available

+ + + + + + + + + {:else} +

Up to date

+ {/if} +
+ + +{#if isDeveloper} +
+

Outgoing Changes

+ + +
+{/if} +``` + +--- + +## Sync Job Integration + +When the sync job runs with `auto_pull = 0`: + +1. `checkForUpdates()` already fetches and counts commits behind +2. This data is already available via `git.status()` (the `behind` field) +3. The `/api/databases/[id]/changes` endpoint will show this when user visits + +No additional "store" needed - the git state IS the store. Each time the user +visits the changes page, we fetch fresh data from git. + +--- + +## Files to Create/Modify + +### Modified Files + +- `src/routes/databases/[id]/changes/+page.server.ts` - Remove PAT requirement + for load, add pull action +- `src/routes/databases/[id]/changes/+page.svelte` - Add incoming changes UI +- `src/routes/api/databases/[id]/changes/+server.ts` - Remove PAT requirement + for GET, add incoming changes data +- `src/lib/server/utils/git/status.ts` - Add `getIncomingChanges()` function +- `src/lib/server/utils/git/types.ts` - Add `IncomingChanges` types +- `src/lib/server/utils/git/Git.ts` - Expose `getIncomingChanges()` + +### New Components (optional) + +- `src/routes/databases/[id]/changes/components/IncomingChangesTable.svelte` +- `src/routes/databases/[id]/changes/components/OutgoingChangesTable.svelte` + +Could extract existing table into `OutgoingChangesTable` and create matching +`IncomingChangesTable` for consistency. + +--- + +## Edge Cases + +1. **No incoming changes**: Show "Up to date" message +2. **Pull fails**: Show error, allow retry +3. **Conflicts**: Shouldn't happen since user_ops are gitignored, but handle + gracefully if it does +4. **Large number of commits**: Paginate or limit to recent N commits + +--- + +## UI Considerations + +- StatusCard (repo info, branch switcher) visible for everyone +- Use consistent table styling between incoming/outgoing +- Incoming table is read-only (no checkboxes) +- Clear visual separation between sections +- Consider showing incoming changes count in the tab/nav if updates available diff --git a/src/lib/server/utils/git/Git.ts b/src/lib/server/utils/git/Git.ts index 3104422..2b9ba84 100644 --- a/src/lib/server/utils/git/Git.ts +++ b/src/lib/server/utils/git/Git.ts @@ -2,7 +2,7 @@ * Git class - wraps git operations for a repository */ -import type { GitStatus, OperationFile, CommitResult, UpdateInfo, Commit } from './types.ts'; +import type { GitStatus, OperationFile, CommitResult, UpdateInfo, Commit, IncomingChanges } from './types.ts'; import * as repo from './repo.ts'; import * as status from './status.ts'; import * as ops from './ops.ts'; @@ -22,6 +22,7 @@ export class Git { getBranches = () => status.getBranches(this.repoPath); status = (options?: status.GetStatusOptions): Promise => status.getStatus(this.repoPath, options); checkForUpdates = (): Promise => status.checkForUpdates(this.repoPath); + getIncomingChanges = (): Promise => status.getIncomingChanges(this.repoPath); getLastPushed = () => status.getLastPushed(this.repoPath); getCommits = (limit?: number): Promise => status.getCommits(this.repoPath, limit); getDiff = (filepaths?: string[]): Promise => status.getDiff(this.repoPath, filepaths); diff --git a/src/lib/server/utils/git/status.ts b/src/lib/server/utils/git/status.ts index f79a068..0a79161 100644 --- a/src/lib/server/utils/git/status.ts +++ b/src/lib/server/utils/git/status.ts @@ -4,7 +4,7 @@ import { execGit, execGitSafe } from './exec.ts'; import { fetch } from './repo.ts'; -import type { GitStatus, UpdateInfo, Commit } from './types.ts'; +import type { GitStatus, UpdateInfo, Commit, IncomingChanges } from './types.ts'; /** * Get current branch name @@ -241,3 +241,58 @@ export async function getCommits(repoPath: string, limit: number = 50): Promise< return commits; } + +/** + * Get incoming changes (commits available to pull from remote) + */ +export async function getIncomingChanges(repoPath: string): Promise { + await fetch(repoPath); + + const branch = await getBranch(repoPath); + const remoteBranch = `origin/${branch}`; + + // Count commits behind + const countOutput = await execGitSafe( + ['rev-list', '--count', `HEAD..${remoteBranch}`], + repoPath + ); + const commitsBehind = parseInt(countOutput || '0') || 0; + + if (commitsBehind === 0) { + return { hasUpdates: false, commitsBehind: 0, commits: [] }; + } + + // Get commit details for incoming commits + const format = '%H|%h|%s|%an|%ae|%cI'; + const output = await execGit( + ['log', `--format=${format}`, `HEAD..${remoteBranch}`], + repoPath + ); + + const commits: Commit[] = []; + + for (const line of output.split('\n')) { + if (!line.trim()) continue; + + const [hash, shortHash, message, author, authorEmail, date] = line.split('|'); + + // Get files changed for this commit + const filesOutput = await execGitSafe( + ['diff-tree', '--no-commit-id', '--name-only', '-r', hash], + repoPath + ); + const files = filesOutput ? filesOutput.split('\n').filter((f) => f.trim()) : []; + + commits.push({ + hash, + shortHash, + message, + author, + authorEmail, + date, + files + }); + } + + return { hasUpdates: true, commitsBehind, commits }; +} diff --git a/src/lib/server/utils/git/types.ts b/src/lib/server/utils/git/types.ts index bba2f07..4fa88af 100644 --- a/src/lib/server/utils/git/types.ts +++ b/src/lib/server/utils/git/types.ts @@ -55,3 +55,9 @@ export interface Commit { date: string; files: string[]; } + +export interface IncomingChanges { + hasUpdates: boolean; + commitsBehind: number; + commits: Commit[]; +} diff --git a/src/routes/api/databases/[id]/changes/+server.ts b/src/routes/api/databases/[id]/changes/+server.ts index 2872b46..ef10a29 100644 --- a/src/routes/api/databases/[id]/changes/+server.ts +++ b/src/routes/api/databases/[id]/changes/+server.ts @@ -11,25 +11,27 @@ export const GET: RequestHandler = async ({ params }) => { error(404, 'Database not found'); } - if (!database.personal_access_token) { - error(403, 'Changes page requires a personal access token'); - } - const git = new Git(database.local_path); - const [status, uncommittedOps, lastPushed, branches, repoInfo] = await Promise.all([ + // Fetch data for everyone + const [status, incomingChanges, branches, repoInfo] = await Promise.all([ git.status(), - git.getUncommittedOps(), - git.getLastPushed(), + git.getIncomingChanges(), git.getBranches(), getRepoInfo(database.repository_url, database.personal_access_token) ]); + // Only fetch outgoing changes for developers + let uncommittedOps = null; + if (database.personal_access_token) { + uncommittedOps = await git.getUncommittedOps(); + } + return json({ status, - uncommittedOps, - lastPushed, + incomingChanges, branches, - repoInfo + repoInfo, + uncommittedOps }); }; diff --git a/src/routes/databases/[id]/+layout.svelte b/src/routes/databases/[id]/+layout.svelte index 56ed10e..777b40f 100644 --- a/src/routes/databases/[id]/+layout.svelte +++ b/src/routes/databases/[id]/+layout.svelte @@ -7,16 +7,12 @@ $: currentPath = $page.url.pathname; $: tabs = database ? [ - ...(database.personal_access_token - ? [ - { - label: 'Changes', - href: `/databases/${database.id}/changes`, - icon: GitBranch, - active: currentPath.endsWith('/changes') - } - ] - : []), + { + label: 'Changes', + href: `/databases/${database.id}/changes`, + icon: GitBranch, + active: currentPath.endsWith('/changes') + }, { label: 'Commits', href: `/databases/${database.id}/commits`, diff --git a/src/routes/databases/[id]/changes/+page.server.ts b/src/routes/databases/[id]/changes/+page.server.ts index 71520c9..cbf7882 100644 --- a/src/routes/databases/[id]/changes/+page.server.ts +++ b/src/routes/databases/[id]/changes/+page.server.ts @@ -1,18 +1,16 @@ -import { error } from '@sveltejs/kit'; import type { PageServerLoad, Actions } from './$types'; import { databaseInstancesQueries } from '$db/queries/databaseInstances.ts'; import { Git } from '$utils/git/index.ts'; import { logger } from '$logger/logger.ts'; import { compile, startWatch } from '$lib/server/pcd/cache.ts'; +import { pcdManager } from '$pcd/pcd.ts'; export const load: PageServerLoad = async ({ parent }) => { const { database } = await parent(); - if (!database.personal_access_token) { - error(403, 'Changes page requires a personal access token'); - } - - return {}; + return { + isDeveloper: !!database.personal_access_token + }; }; export const actions: Actions = { @@ -102,5 +100,36 @@ export const actions: Actions = { } catch (err) { return { success: false, error: err instanceof Error ? err.message : 'Failed to switch branch' }; } + }, + + pull: async ({ params }) => { + const id = parseInt(params.id || '', 10); + const database = databaseInstancesQueries.getById(id); + + if (!database) { + return { success: false, error: 'Database not found' }; + } + + try { + const result = await pcdManager.sync(id); + + if (result.success) { + await logger.info('Database synced', { + source: 'changes', + meta: { databaseId: id, commitsPulled: result.commitsBehind } + }); + } + + return result; + } catch (err) { + await logger.error('Failed to pull changes', { + source: 'changes', + meta: { databaseId: id, error: String(err) } + }); + return { + success: false, + error: err instanceof Error ? err.message : 'Failed to pull' + }; + } } }; diff --git a/src/routes/databases/[id]/changes/+page.svelte b/src/routes/databases/[id]/changes/+page.svelte index e2e3ef2..7f3215c 100644 --- a/src/routes/databases/[id]/changes/+page.svelte +++ b/src/routes/databases/[id]/changes/+page.svelte @@ -1,16 +1,22 @@ @@ -176,137 +244,247 @@ {:else} - + {/if} - - {#if loading} -
-
-
-
-
+ +
+
+

+ + Incoming Changes +

+ {#if incomingChanges?.hasUpdates} + + {/if} +
+ + {#if loading} +
+
+
+
-
- {:else} - - {/if} - - - {#if loading} -
- - - - - - - - - - - - {#each Array(5) as _} - - - - - - - - {/each} - -
OperationEntityNameFile
-
-
-
- {:else if uncommittedOps.length === 0} -
-

No uncommitted changes

-
- {:else} -
- - - - - - - - - - - - {#each uncommittedOps as op} - toggleRow(op.filepath)} + {:else if !incomingChanges?.hasUpdates} +
+

+ {#if data.database.auto_pull} + Up to date — updates are pulled automatically + {:else} + Up to date + {/if} +

+
+ {:else} + row.hash} + emptyMessage="No incoming changes" + > + + {#if column.key === 'shortHash'} + - + + + + + + {/each} + +
- - - Operation - - Entity - - Name - - File -
-
+ + {:else if column.key === 'message'} + + {row.message} + + {:else if column.key === 'author'} + + {row.author} + + {:else if column.key === 'date'} + + {formatDate(row.date)} + + {/if} + + + +
+
+ + {row.files.length} file{row.files.length !== 1 ? 's' : ''} changed +
+ {#if row.files.length > 0} +
+ {#each row.files as file} + + {file} + + {/each} +
+ {/if} +
+ + + {/if} + + + + {#if isDeveloper} +
+

+ + Outgoing Changes +

+ + + {#if loading} +
+
+
+
+
+
+
+ {:else} +
+ +
+ {/if} + + + {#if loading} +
+ + + + + + + + + + + + {#each Array(5) as _} + + + + + + + + {/each} + +
OperationEntityNameFile
+
+
+
+ {:else if uncommittedOps.length === 0} +
+

No uncommitted changes

+
+ {:else} +
+ + + + + + + + + + + + {#each uncommittedOps as op} + toggleRow(op.filepath)} > - {#if selected.has(op.filepath)} - - {/if} - - - - - - - - {/each} - -
+ + + Operation + + Entity + + Name + + File +
- - {formatOperation(op.operation)} - - - {op.entity || '-'} - - {#if op.previousName && op.previousName !== op.name} - {op.previousName} - → - {op.name || '-'} - {:else} - {op.name || '-'} - {/if} - - - {op.filename} - -
-
+
+
+ {#if selected.has(op.filepath)} + + {/if} +
+ 		+ + {formatOperation(op.operation)} + + + {op.entity || '-'} + + {#if op.previousName && op.previousName !== op.name} + {op.previousName} + → + {op.name || '-'} + {:else} + {op.name || '-'} + {/if} + + + {op.filename} + +
+
+ {/if} + {/if}
diff --git a/src/routes/databases/[id]/changes/components/ChangesActionsBar.svelte b/src/routes/databases/[id]/changes/components/ChangesActionsBar.svelte index 3bb40b7..dc991c9 100644 --- a/src/routes/databases/[id]/changes/components/ChangesActionsBar.svelte +++ b/src/routes/databases/[id]/changes/components/ChangesActionsBar.svelte @@ -10,15 +10,18 @@ export let selectedFiles: string[] = []; export let commitMessage: string; export let aiEnabled: boolean = false; + export let hasIncomingChanges: boolean = false; + export let adding: boolean = false; + export let discarding: boolean = false; export let onDiscard: () => void; export let onAdd: () => void; let generating = false; - $: canDiscard = selectedCount > 0; - $: canAdd = selectedCount > 0 && commitMessage.trim().length > 0; - $: canGenerate = aiEnabled && selectedCount > 0 && !generating; + $: canDiscard = selectedCount > 0 && !discarding; + $: canAdd = selectedCount > 0 && commitMessage.trim().length > 0 && !hasIncomingChanges && !adding; + $: canGenerate = aiEnabled && selectedCount > 0 && !generating && !hasIncomingChanges; async function handleGenerate() { if (!canGenerate) return; @@ -49,13 +52,14 @@
@@ -71,7 +75,9 @@
- {#if generating} + {#if hasIncomingChanges} + Pull incoming changes first + {:else if generating} Generating... {:else if !selectedCount} Select changes first @@ -85,7 +91,8 @@ {/if}
- {#if !selectedCount} + {#if adding} + Pushing... + {:else if hasIncomingChanges} + Pull incoming changes first + {:else if !selectedCount} Select changes to add {:else if !commitMessage.trim()} Enter a commit message @@ -106,7 +117,8 @@
- {#if canDiscard} + {#if discarding} + Discarding... + {:else if selectedCount > 0} Discard {selectedCount} change{selectedCount === 1 ? '' : 's'} {:else} Select changes to discard diff --git a/src/routes/databases/[id]/changes/components/StatusCard.svelte b/src/routes/databases/[id]/changes/components/StatusCard.svelte index cd7fa09..edff22e 100644 --- a/src/routes/databases/[id]/changes/components/StatusCard.svelte +++ b/src/routes/databases/[id]/changes/components/StatusCard.svelte @@ -9,7 +9,8 @@ CircleDot, ChevronDown, Check, - Database + Database, + RefreshCw } from 'lucide-svelte'; import { invalidateAll } from '$app/navigation'; import type { GitStatus, RepoInfo } from '$utils/git/types'; @@ -19,9 +20,11 @@ export let repoInfo: RepoInfo | null; export let branches: string[]; export let database: DatabaseInstance; + export let onSync: (() => Promise) | undefined = undefined; let branchDropdownOpen = false; let switching = false; + let syncing = false; async function handleBranchSwitch(branch: string) { if (branch === status.branch || switching) return; @@ -52,6 +55,18 @@ branchDropdownOpen = false; } } + + async function handleSync() { + if (syncing) return; + + syncing = true; + try { + // Just refresh the data (fetches from remote to check for updates) + if (onSync) await onSync(); + } finally { + syncing = false; + } + } @@ -179,6 +194,16 @@ {/if}
+ +