forked from farhoodlabs/paperclip
[codex] Workspace diff polish (#6383)
## Thinking Path > - Paperclip gives operators a workspace diff plugin so they can inspect agent changes before review > - The diff view needs reliable base-ref defaults and controls that stay usable while scrolling large diffs > - The working branch mixed those plugin improvements with unrelated server and cloud work > - Keeping the workspace diff plugin changes isolated makes them easy to test and review > - This pull request polishes the workspace diff plugin controls, base-ref behavior, and sticky headers > - The benefit is a more predictable diff review surface for agent workspaces ## What Changed - Fixed workspace diff default base-ref resolution. - Improved split/unified and working-tree/against-ref pane controls. - Made workspace diff headers stay sticky while scrolling. - Added a review screenshot at `screenshots/PAP-9841-workspace-diff.png`. ## Verification - `pnpm install --frozen-lockfile --ignore-scripts` - `pnpm --filter @paperclipai/plugin-sdk build` - `pnpm --filter @paperclipai/plugin-workspace-diff exec vitest run tests/plugin.spec.ts` - Result: 9 tests passed. ## Risks - UI-only plugin branch with low data risk. - The default base-ref inference should be reviewed against unusual worktree/upstream combinations. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, GPT-5-based coding agent with local shell/git/tool use. Exact hosted model ID and context-window size are not exposed by the local Paperclip adapter runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -645,6 +645,57 @@ async function resolveHeadSha(cwd: string) {
|
||||
}
|
||||
}
|
||||
|
||||
async function resolveVerifiedGitRef(cwd: string, refName: string) {
|
||||
const trimmed = refName.trim();
|
||||
if (!trimmed) return null;
|
||||
try {
|
||||
await execFileAsync("git", ["-C", cwd, "rev-parse", "--verify", "--quiet", `${trimmed}^{commit}`], {
|
||||
cwd,
|
||||
timeout: GIT_TIMEOUT_MS,
|
||||
maxBuffer: 128 * 1024,
|
||||
});
|
||||
return trimmed;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
async function resolveGitUpstreamRef(cwd: string) {
|
||||
try {
|
||||
const upstream = (await execFileAsync(
|
||||
"git",
|
||||
["-C", cwd, "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}"],
|
||||
{
|
||||
cwd,
|
||||
timeout: GIT_TIMEOUT_MS,
|
||||
maxBuffer: 128 * 1024,
|
||||
},
|
||||
)).stdout.trim();
|
||||
return upstream ? await resolveVerifiedGitRef(cwd, upstream) : null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
async function resolveInferredDefaultBaseRef(cwd: string) {
|
||||
const upstream = await resolveGitUpstreamRef(cwd);
|
||||
if (upstream) return upstream;
|
||||
|
||||
const candidates = ["origin/master", "origin/main", "master", "main"];
|
||||
const resolvedCandidates = await Promise.all(
|
||||
candidates.map((candidate) => resolveVerifiedGitRef(cwd, candidate)),
|
||||
);
|
||||
for (const resolved of resolvedCandidates) {
|
||||
if (resolved) return resolved;
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
async function resolveDefaultDiffBaseRef(cwd: string, workspace: WorkspaceDiffTarget) {
|
||||
return workspace.baseRef?.trim() || await resolveInferredDefaultBaseRef(cwd);
|
||||
}
|
||||
|
||||
async function resolveBaseRef(cwd: string, baseRef: string | null, workspace: WorkspaceDiffTarget) {
|
||||
const resolvedBaseRef = baseRef ?? workspace.baseRef ?? null;
|
||||
if (!resolvedBaseRef) {
|
||||
@@ -705,9 +756,16 @@ export function workspaceDiffService() {
|
||||
return {
|
||||
async getDiff(workspace: WorkspaceDiffTarget, query: WorkspaceDiffQueryOptions): Promise<WorkspaceDiffResponse> {
|
||||
const { cwd, repoRoot } = await resolveWorkspacePaths(workspace);
|
||||
const defaultBaseRef = await resolveDefaultDiffBaseRef(cwd, workspace);
|
||||
const workspaceWithDefaultBaseRef = { ...workspace, baseRef: defaultBaseRef };
|
||||
const paths = normalizePathFilters(query.paths);
|
||||
const warnings: WorkspaceDiffWarning[] = [];
|
||||
const { files: filesByPath, baseRef } = await collectFiles({ cwd, workspace, query, paths });
|
||||
const { files: filesByPath, baseRef } = await collectFiles({
|
||||
cwd,
|
||||
workspace: workspaceWithDefaultBaseRef,
|
||||
query,
|
||||
paths,
|
||||
});
|
||||
const allFiles = Array.from(filesByPath.values()).sort((left, right) => left.path.localeCompare(right.path));
|
||||
const cappedFiles = allFiles.slice(0, WORKSPACE_DIFF_CAPS.maxFiles);
|
||||
if (allFiles.length > cappedFiles.length) {
|
||||
@@ -771,7 +829,7 @@ export function workspaceDiffService() {
|
||||
companyId: workspace.companyId,
|
||||
view: query.view,
|
||||
baseRef,
|
||||
defaultBaseRef: workspace.baseRef,
|
||||
defaultBaseRef,
|
||||
headSha: await resolveHeadSha(cwd),
|
||||
includeUntracked: query.includeUntracked,
|
||||
paths,
|
||||
|
||||
Reference in New Issue
Block a user