Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions apps/staged/src-tauri/src/prs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1080,8 +1080,16 @@ pub(crate) async fn has_unpushed_commits_impl(
.map_err(|e| e.to_string())?
.ok_or_else(|| format!("No worktree for branch: {branch_id}"))?;

git::has_unpushed_commits(Path::new(&workdir.path), &branch.branch_name)
.map_err(|e| e.to_string())
// Run the blocking git subprocesses on a background thread so a slow cold
// `git` invocation can't block the Tauri IPC thread and freeze the UI,
// matching the remote path above.
let path = workdir.path.clone();
let branch_name = branch.branch_name.clone();
tauri::async_runtime::spawn_blocking(move || {
git::has_unpushed_commits(Path::new(&path), &branch_name).map_err(|e| e.to_string())
})
.await
.map_err(|e| format!("has_unpushed_commits task failed: {e}"))?
}

/// Push a branch to its remote by kicking off an agent session.
Expand Down
99 changes: 78 additions & 21 deletions apps/staged/src/lib/features/projects/ProjectHome.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@
import { projectRunActionsStore } from '../../stores/projectRunActions.svelte';
import { repoBadgeStore } from '../../stores/repoBadges.svelte';
import { projectStateStore } from '../../stores/projectState.svelte';
import { canDeleteProjectWithoutConfirmation } from './projectDeleteSafety';
import {
canDeleteProjectWithoutConfirmation,
computeSafeToDeleteSignature,
} from './projectDeleteSafety';

/**
* Merge incoming branches with existing ones, preserving worktreePath when
Expand Down Expand Up @@ -375,35 +378,89 @@
// Only check visible projects — calling hasUnpushedCommits for every
// project wastes IPC round-trips (especially expensive for remote branches)
// and the result is only consumed in the visibleProjects render loop.
// Signature of the last inputs the safe-to-delete check actually ran against.
// Background hydration/pollers reassign visibleProjects/branchesByProject/
// repoCountsByProject many times per switch even when the fields this check
// depends on are unchanged; deduping on the signature keeps the expensive
// per-branch git work from re-firing on every reassignment.
let lastSafeSignature: string | null = null;
$effect(() => {
const updateSafeStatus = async () => {
const nextSafe = new Set<string>();
// Read reactive deps synchronously so the effect re-subscribes correctly.
const projectsSnapshot = visibleProjects;
const branches = branchesByProject;
const repoCounts = repoCountsByProject;

const signature = computeSafeToDeleteSignature(projectsSnapshot, branches, repoCounts);
if (signature === lastSafeSignature) {
// Inputs relevant to the result are unchanged — skip the git work.
return;
Comment on lines +393 to +396
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid spawning duplicate safe-delete checks while one is in flight

When branchesByProject or repoCountsByProject is reassigned with the same relevant fields while the first safe-to-delete check is already running, this comparison still misses the duplicate because lastSafeSignature is only assigned after the async Promise.all completes. The cleanup marks the old run stale, but it cannot cancel the already-started commands.hasUnpushedCommits IPC/git subprocesses, so background hydration can still launch overlapping checks for the same signature and reproduce the switch-time git storm this change is trying to avoid. Consider tracking a pending/in-flight signature before scheduling or before starting updateSafeStatus.

Useful? React with 👍 / 👎.

}

for (const project of visibleProjects) {
const branches = branchesByProject.get(project.id) || [];
const repoCount = repoCountsByProject.get(project.id) || 0;
// Bail out of stale work: if the effect re-fires (or the component tears
// down) before this run resolves, `stale` flips so we neither spawn the
// git loop's tail nor clobber newer state.
let stale = false;
let idleHandle: number | undefined;

// Don't show red styling for projects with no repos — there's nothing
// to call attention to when no repositories have been added yet.
if (repoCount === 0) {
continue;
}
const updateSafeStatus = async () => {
// Parallelize across projects so the project-home grid doesn't serialize
// every project's git work.
const results = await Promise.all(
projectsSnapshot.map(async (project) => {
const projectBranches = branches.get(project.id) || [];
const repoCount = repoCounts.get(project.id) || 0;

// Don't show red styling for projects with no repos — there's nothing
// to call attention to when no repositories have been added yet.
if (repoCount === 0) {
return { id: project.id, safe: false };
}

const safe = await canDeleteProjectWithoutConfirmation({
branches: projectBranches,
repoCount,
hasUnpushedCommits: commands.hasUnpushedCommits,
});
return { id: project.id, safe };
})
);

const safe = await canDeleteProjectWithoutConfirmation({
branches,
repoCount,
hasUnpushedCommits: commands.hasUnpushedCommits,
});
if (stale) return;

if (safe) {
nextSafe.add(project.id);
}
const nextSafe = new Set<string>();
for (const { id, safe } of results) {
if (safe) nextSafe.add(id);
}

safeToDeleteProjects = nextSafe;
// Only record the signature once the check actually completes. If this
// run is cancelled (re-fire/teardown) before it gets here, the signature
// stays unchanged so the next fire reschedules instead of dropping the
// check permanently for that signature.
lastSafeSignature = signature;
};

updateSafeStatus();
// Defer off the critical render path: let the switch's keyed-block swap
// flush first, then settle the cosmetic styling during idle. The timeout
// guarantees the check still runs even while the main thread stays busy
// through the post-switch hydration window (otherwise an idle callback can
// be deferred indefinitely under sustained load).
const schedule =
typeof requestIdleCallback === 'function'
? (cb: () => void) => requestIdleCallback(cb, { timeout: 2000 })
: (cb: () => void) => setTimeout(cb, 0) as unknown as number;
const cancel =
typeof cancelIdleCallback === 'function'
? (handle: number) => cancelIdleCallback(handle)
: (handle: number) => clearTimeout(handle);

idleHandle = schedule(() => {
void updateSafeStatus();
});

return () => {
stale = true;
if (idleHandle !== undefined) cancel(idleHandle);
};
});

$effect(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { describe, expect, it, vi } from 'vitest';
import type { Branch } from '../../types';
import { canDeleteProjectWithoutConfirmation } from './projectDeleteSafety';
import {
canDeleteProjectWithoutConfirmation,
computeSafeToDeleteSignature,
} from './projectDeleteSafety';

function branch(overrides: Partial<Branch> = {}): Branch {
return {
Expand Down Expand Up @@ -113,3 +116,41 @@ describe('canDeleteProjectWithoutConfirmation', () => {
expect(onCheckError).toHaveBeenCalledWith(error, checkedBranch);
});
});

describe('computeSafeToDeleteSignature', () => {
const projects = [{ id: 'project-1' }];

it('produces an identical signature for identical inputs', () => {
const branchesA = new Map([['project-1', [branch()]]]);
const branchesB = new Map([['project-1', [branch()]]]);
const repoCounts = new Map([['project-1', 1]]);

expect(computeSafeToDeleteSignature(projects, branchesA, repoCounts)).toBe(
computeSafeToDeleteSignature(projects, branchesB, repoCounts)
);
});

it('changes when a branch prHeadSha changes', () => {
const repoCounts = new Map([['project-1', 1]]);
const before = computeSafeToDeleteSignature(
projects,
new Map([['project-1', [branch({ prHeadSha: 'abc' })]]]),
repoCounts
);
const after = computeSafeToDeleteSignature(
projects,
new Map([['project-1', [branch({ prHeadSha: 'def' })]]]),
repoCounts
);

expect(before).not.toBe(after);
});

it('changes when the repo count changes', () => {
const branches = new Map([['project-1', [branch()]]]);
const before = computeSafeToDeleteSignature(projects, branches, new Map([['project-1', 1]]));
const after = computeSafeToDeleteSignature(projects, branches, new Map([['project-1', 2]]));

expect(before).not.toBe(after);
});
});
29 changes: 29 additions & 0 deletions apps/staged/src/lib/features/projects/projectDeleteSafety.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,32 @@ export async function canDeleteProjectWithoutConfirmation({

return branchSafety.every(Boolean);
}

/**
* Compute a cheap signature of only the inputs that affect the safe-to-delete
* result, so the eager `$effect` in ProjectHome can skip recomputation (and the
* expensive per-branch git work it spawns) when background hydration reassigns
* the source Maps without actually changing the relevant fields.
*
* Keys on exactly the fields `canDeleteProjectWithoutConfirmation` branches on
* (`prState`, `branchType`, `repoCount`) plus `prHeadSha`, which invalidates the
* cache when a PR head moves — the main case where unpushed-commit state
* changes in practice. This trades a small staleness window (a local commit not
* reflected in the cosmetic styling until the next genuine input change) for
* eliminating the per-switch freeze.
*/
export function computeSafeToDeleteSignature(
projects: Array<{ id: string }>,
branchesByProject: Map<string, Branch[]>,
repoCountsByProject: Map<string, number>
): string {
return projects
.map((p) => {
const repoCount = repoCountsByProject.get(p.id) || 0;
const branches = (branchesByProject.get(p.id) || [])
.map((b) => `${b.id}:${b.prState}:${b.branchType}:${b.prHeadSha ?? ''}`)
.join(',');
return `${p.id}|${repoCount}|${branches}`;
})
.join(';');
}