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
1 change: 1 addition & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ LOCAL_WORKSPACE_MAX_FILE_BYTES=1000000
LOCAL_WORKSPACE_SEARCH_MAX_TOTAL_BYTES=50000000
LOCAL_WORKSPACE_MAX_DIFF_BYTES=5000000
LOCAL_WORKSPACE_MIN_FREE_SPACE_BYTES=500000000
LOCAL_WORKSPACE_FULL_CLONE_MAX_REPO_KB=1000000
LOCAL_WORKSPACE_STALE_CLEANUP_AGE_SECONDS=86400

# --- Ask agent ---
Expand Down
1 change: 1 addition & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ Import convention: `import { … } from "../settings/index.js"` for constants; `
| Workspace search byte cap | `LOCAL_WORKSPACE_SEARCH_MAX_TOTAL_BYTES` | `50000000` | max bytes scanned per `searchWorkspace` call |
| Workspace diff cap | `LOCAL_WORKSPACE_MAX_DIFF_BYTES` | `5000000` | max local diff bytes returned to tools |
| Workspace free space min | `LOCAL_WORKSPACE_MIN_FREE_SPACE_BYTES` | `500000000` | fail setup below this free-space threshold |
| Full clone repo size cap | `LOCAL_WORKSPACE_FULL_CLONE_MAX_REPO_KB` | `1000000` | use sparse changed-file checkout above this repo size |
| Workspace stale cleanup | `LOCAL_WORKSPACE_STALE_CLEANUP_AGE_SECONDS` | `86400` | startup cleanup age for leaked temp dirs |
| Log level | `LOG_LEVEL` | `info` | |
| Max wide sub-events | `LOG_MAX_WIDE_EVENTS` | `128` | |
Expand Down
1 change: 1 addition & 0 deletions src/agentWork/executors/askExecutor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export async function executeAskJob(
prNumber: item.prNumber,
headSha,
installationToken: tokenState.installation.token,
repositorySizeKb: payload.repositorySizeKb,
},
async (repositoryView) => {
const result = await runAskRun({
Expand Down
1 change: 1 addition & 0 deletions src/agentWork/executors/descriptionExecutor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export async function executeDescriptionJob(
prNumber: item.prNumber,
headSha,
installationToken: tokenState.installation.token,
repositorySizeKb: payload.repositorySizeKb,
},
async (repositoryView) => {
const result = await runFullPrDescription({
Expand Down
1 change: 1 addition & 0 deletions src/agentWork/executors/reviewExecutor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ export async function executeReviewJob(
prNumber: item.prNumber,
headSha,
installationToken: tokenState.installation.token,
repositorySizeKb: payload.repositorySizeKb,
},
async (repositoryView) => {
const bot = await getAppBotIdentity(cfg);
Expand Down
2 changes: 2 additions & 0 deletions src/agentWork/intake/slashIntake.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export type SlashCommandInput = {
readonly installationId: number;
readonly owner: string;
readonly repo: string;
readonly repositorySizeKb?: number;
readonly prNumber: number;
readonly commentId: number;
readonly commenterId: number;
Expand Down Expand Up @@ -234,6 +235,7 @@ export async function applySlashCommandIntake(
prNumber: input.prNumber,
installationId: input.installationId,
headSha: DEFERRED_HEAD_SHA,
repositorySizeKb: input.repositorySizeKb,
};
const targets: AckTarget[] = [
{ kind: "pr", prNumber: input.prNumber },
Expand Down
3 changes: 3 additions & 0 deletions src/agentWork/intake/workItemRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export async function createReviewWorkItem(
payload: {
mode: params.lens,
source: params.source,
repositorySizeKb: params.ref.repositorySizeKb,
userSupplement: params.userSupplement,
commenterId: params.commenterId,
},
Expand Down Expand Up @@ -132,6 +133,7 @@ export async function createDescriptionWorkItem(
priority: params.source === "slash" ? 50 : 0,
payload: {
source: params.source,
repositorySizeKb: params.ref.repositorySizeKb,
userSupplement: params.userSupplement,
commenterId: params.commenterId,
},
Expand Down Expand Up @@ -164,6 +166,7 @@ export async function createAskWorkItem(
payload: {
question: params.question,
replyTarget: params.replyTarget,
repositorySizeKb: params.ref.repositorySizeKb,
commentId: params.commentId,
commenterId: params.commenterId,
codeAnchor: params.codeAnchor,
Expand Down
5 changes: 5 additions & 0 deletions src/agentWork/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ export type PrRef = {
readonly installationId: number;
/** Commit SHA, or DEFERRED_HEAD_SHA for worker-side pulls.get resolution */
readonly headSha: string;
/** GitHub webhook repository.size, in KB, captured at intake time when available. */
readonly repositorySizeKb?: number;
};

export type AckTarget =
Expand Down Expand Up @@ -70,6 +72,7 @@ export type DescriptionJobData = JobCorrelation & {
export type ReviewWorkPayload = {
readonly mode: ReviewMode;
readonly source: WorkSource;
readonly repositorySizeKb?: number;
readonly userSupplement?: string;
readonly commenterId?: number;
/** Set when the run finished but structured publish did not succeed */
Expand All @@ -85,13 +88,15 @@ export type ReviewWorkPayload = {
export type AskWorkPayload = {
readonly question: string;
readonly replyTarget: ReplyTarget;
readonly repositorySizeKb?: number;
readonly codeAnchor?: CodeAnchor;
readonly commenterId?: number;
readonly commentId: number;
};

export type DescriptionWorkPayload = {
readonly source: WorkSource;
readonly repositorySizeKb?: number;
readonly userSupplement?: string;
readonly commenterId?: number;
};
Expand Down
6 changes: 6 additions & 0 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
DEFAULT_LOCAL_WORKSPACE_FETCH_TIMEOUT_MS,
DEFAULT_LOCAL_WORKSPACE_MAX_DIFF_BYTES,
DEFAULT_LOCAL_WORKSPACE_MAX_FILE_BYTES,
DEFAULT_LOCAL_WORKSPACE_FULL_CLONE_MAX_REPO_KB,
DEFAULT_LOCAL_WORKSPACE_SEARCH_MAX_FILES,
DEFAULT_LOCAL_WORKSPACE_SEARCH_MAX_TOTAL_BYTES,
DEFAULT_LOCAL_WORKSPACE_MIN_FREE_SPACE_BYTES,
Expand Down Expand Up @@ -345,6 +346,10 @@ export function loadConfig() {
ENV.LOCAL_WORKSPACE_MIN_FREE_SPACE_BYTES,
DEFAULT_LOCAL_WORKSPACE_MIN_FREE_SPACE_BYTES,
);
const localWorkspaceFullCloneMaxRepoKb = readPositiveNumber(
ENV.LOCAL_WORKSPACE_FULL_CLONE_MAX_REPO_KB,
DEFAULT_LOCAL_WORKSPACE_FULL_CLONE_MAX_REPO_KB,
);
const localWorkspaceStaleCleanupAgeSeconds = readPositiveNumber(
ENV.LOCAL_WORKSPACE_STALE_CLEANUP_AGE_SECONDS,
DEFAULT_LOCAL_WORKSPACE_STALE_CLEANUP_AGE_SECONDS,
Expand Down Expand Up @@ -407,6 +412,7 @@ export function loadConfig() {
localWorkspaceSearchMaxTotalBytes,
localWorkspaceMaxDiffBytes,
localWorkspaceMinFreeSpaceBytes,
localWorkspaceFullCloneMaxRepoKb,
localWorkspaceStaleCleanupAgeSeconds,
};
}
Expand Down
3 changes: 3 additions & 0 deletions src/effect/services/webhookHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export const WebhookHandlersCore = Layer.effect(
prNumber: data.pull_request.number,
headSha: data.pull_request.head.sha,
installationId: data.installation.id,
repositorySizeKb: data.repository.size,
},
data.action ?? "",
intakeLog,
Expand Down Expand Up @@ -106,6 +107,7 @@ export const WebhookHandlersCore = Layer.effect(
installationId: data.installation.id,
owner: data.repository.owner.login,
repo: data.repository.name,
repositorySizeKb: data.repository.size,
prNumber: data.issue.number,
commenterId: data.comment.user.id,
commentId: data.comment.id,
Expand Down Expand Up @@ -140,6 +142,7 @@ export const WebhookHandlersCore = Layer.effect(
installationId: data.installation.id,
owner: data.repository.owner.login,
repo: data.repository.name,
repositorySizeKb: data.repository.size,
prNumber: data.pull_request.number,
commenterId: data.comment.user.id,
commentId: data.comment.id,
Expand Down
33 changes: 33 additions & 0 deletions src/prWorkspace/localPrWorkspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const TOKEN_FILE_NAME = "git-token";
const PR_HEAD_REF = "pr-head";

type ChangedFileStatus = "added" | "modified" | "deleted" | "renamed" | "copied" | "other";
export type LocalPrWorkspaceCheckoutMode = "full" | "sparse";

type LocalPrChangedFile = {
readonly path: string;
Expand All @@ -36,6 +37,7 @@ export type LocalPrWorkspace = {
readonly agentCwd: string;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P0 · Return object missing required checkoutMode property

src/prWorkspace/localPrWorkspace.ts · lines 37-43

The LocalPrWorkspace type at line 40 declares readonly checkoutMode: LocalPrWorkspaceCheckoutMode as required, but the return object in prepareLocalPrWorkspace (which returns Promise<LocalPrWorkspace>) never includes it. At runtime this will be undefined. The test at test/localPrWorkspace.test.ts:142 (expect(fullWorkspace.checkoutMode).toBe("full")) and line 180 will fail. The selectLocalPrWorkspaceCheckoutMode function (line 102-116) computes the mode correctly but its result is never assigned into the return value.

Prompt to fix
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate.

Repository: prathamdby/pr-agent
Pull request: #58
Head SHA: d2c29cba3073c478a0e2abd64c043fd3fd849d5e

[P0] @src/prWorkspace/localPrWorkspace.ts lines 37-43
Add `checkoutMode: selectLocalPrWorkspaceCheckoutMode(cfg, params.repositorySizeKb)` to the return object in `prepareLocalPrWorkspace` (inside the try block alongside `rootDir`, `agentCwd`, etc.).

readonly changedFiles: readonly LocalPrChangedFile[];
readonly checkoutPaths: ReadonlySet<string>;
readonly checkoutMode: LocalPrWorkspaceCheckoutMode;
readonly diffIndex: CachedPrDiffIndex;
readonly stats: {
readonly truncated: boolean;
Expand All @@ -57,6 +59,7 @@ export type PrepareLocalPrWorkspaceParams = {
readonly headSha: string;
readonly installationToken: string;
readonly prFiles: ListPullRequestFilesResult;
readonly repositorySizeKb?: number;
readonly remoteUrlOverride?: string;
};

Expand Down Expand Up @@ -99,6 +102,15 @@ function mapGithubStatus(file: PullRequestFileEntry): LocalPrChangedFile {
return { path: file.filename, status: mapped };
}

export function selectLocalPrWorkspaceCheckoutMode(
cfg: Pick<Config, "localWorkspaceFullCloneMaxRepoKb">,
repositorySizeKb?: number,
): LocalPrWorkspaceCheckoutMode {
return repositorySizeKb != null && repositorySizeKb > cfg.localWorkspaceFullCloneMaxRepoKb
? "sparse"
: "full";
}

async function execGit(
args: readonly string[],
opts: {
Expand Down Expand Up @@ -219,6 +231,17 @@ async function indexCheckedOutFiles(agentCwd: string): Promise<Set<string>> {
return paths;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 · Sparse checkout helpers defined but never wired into the clone flow

src/prWorkspace/localPrWorkspace.ts · lines 231-247

sparseCheckoutPattern and sparseCheckoutPatterns are defined and exported, and selectLocalPrWorkspaceCheckoutMode (line 102-116) computes the mode — but prepareLocalPrWorkspace never calls any of them. The repositorySizeKb param is destructured out of params at line 275 but never used. The function unconditionally does a full git checkout -f. Repositories above the size cap still get full clones; the sparse checkout feature is entirely non-functional.

Prompt to fix
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate.

Repository: prathamdby/pr-agent
Pull request: #58
Head SHA: d2c29cba3073c478a0e2abd64c043fd3fd849d5e

[P1] @src/prWorkspace/localPrWorkspace.ts lines 231-247
In `prepareLocalPrWorkspace`, when `checkoutMode === "sparse"`, use `git sparse-checkout init --cone` then `git sparse-checkout set` with patterns from `sparseCheckoutPatterns(changedFiles)`, and populate `checkoutPaths` from the changed file list instead of scanning the full tree.

}

function sparseCheckoutPattern(path: string): string {
return `/${path.replace(/\\/g, "/").replace(/[\\*?[]/g, "\\$&")}`;
}

function sparseCheckoutPatterns(changedFiles: readonly LocalPrChangedFile[]): string {
const paths = changedFiles
.filter((file) => file.status !== "deleted")
.map((file) => sparseCheckoutPattern(file.path));
return paths.length > 0 ? `${paths.join("\n")}\n` : "";
}

const PI_AGENT_DIR_PREFIX = "pr-agent-pi-";

async function cleanupStalePiAgentDirs(cfg: Config): Promise<void> {
Expand Down Expand Up @@ -262,6 +285,7 @@ export async function prepareLocalPrWorkspace(
const askpass = await createAskpass(rootDir);
const tokenFile = await writeTokenFile(rootDir, installationToken);
const changedFiles = prFiles.files.map(mapGithubStatus);
const checkoutMode = selectLocalPrWorkspaceCheckoutMode(cfg, params.repositorySizeKb);
const diffIndex = createCachedPrDiffIndex();
const patchByPath = new Map<string, string>();
const patchOmittedByCapPaths = new Set<string>();
Expand Down Expand Up @@ -338,6 +362,14 @@ export async function prepareLocalPrWorkspace(
["fetch", "--no-tags", "--depth=1", "--no-recurse-submodules", "origin", prRef],
cfg.localWorkspaceFetchTimeoutMs,
);
if (checkoutMode === "sparse") {
await git(["config", "core.sparseCheckout", "true"], cfg.localWorkspaceCloneTimeoutMs);
await git(["config", "core.sparseCheckoutCone", "false"], cfg.localWorkspaceCloneTimeoutMs);
await writeFile(
join(privateGitDir, "info", "sparse-checkout"),
sparseCheckoutPatterns(changedFiles),
);
}
await git(["checkout", "-f", PR_HEAD_REF], cfg.localWorkspaceCloneTimeoutMs);
const { stdout: fetchedHead } = await git(["rev-parse", "HEAD"]);
if (fetchedHead.trim().toLowerCase() !== headSha.toLowerCase()) {
Expand All @@ -358,6 +390,7 @@ export async function prepareLocalPrWorkspace(
agentCwd,
changedFiles,
checkoutPaths,
checkoutMode,
diffIndex,
stats: {
truncated: prFiles.truncated,
Expand Down
21 changes: 17 additions & 4 deletions src/prWorkspace/prRepositoryView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ import {
} from "../review/reviewPreflightFiles.js";
import { installationOctokit } from "../github/appAuth.js";
import { fetchPullRequestFiles } from "../github/listPullRequestFiles.js";
import { prepareLocalPrWorkspace, type LocalPrWorkspace } from "./localPrWorkspace.js";
import {
prepareLocalPrWorkspace,
selectLocalPrWorkspaceCheckoutMode,
type LocalPrWorkspace,
} from "./localPrWorkspace.js";

export type PrRepositoryView = {
readonly workspace: LocalPrWorkspace;
Expand All @@ -20,6 +24,7 @@ export type PreparePrRepositoryViewParams = {
readonly prNumber: number;
readonly headSha: string;
readonly installationToken: string;
readonly repositorySizeKb?: number;
};

type CachedPrRepositoryView = PrRepositoryView & { readonly cleanup: () => Promise<void> };
Expand All @@ -33,9 +38,13 @@ type CacheEntry = {
const cache = new Map<string, CacheEntry>();

function cacheKey(
params: Pick<PreparePrRepositoryViewParams, "owner" | "repo" | "prNumber" | "headSha">,
params: Pick<
PreparePrRepositoryViewParams,
"cfg" | "owner" | "repo" | "prNumber" | "headSha" | "repositorySizeKb"
>,
): string {
return `${params.owner}/${params.repo}#${params.prNumber}:${params.headSha}`;
const checkoutMode = selectLocalPrWorkspaceCheckoutMode(params.cfg, params.repositorySizeKb);
return `${params.owner}/${params.repo}#${params.prNumber}:${params.headSha}:${checkoutMode}`;
}

async function fetchPullRequestHeadSha(
Expand Down Expand Up @@ -72,6 +81,7 @@ async function prepareUncached(
headSha: params.headSha,
installationToken: params.installationToken,
prFiles,
repositorySizeKb: params.repositorySizeKb,
});
return {
workspace,
Expand Down Expand Up @@ -119,7 +129,10 @@ async function acquirePrRepositoryView(
}

async function releasePrRepositoryView(
params: Pick<PreparePrRepositoryViewParams, "owner" | "repo" | "prNumber" | "headSha">,
params: Pick<
PreparePrRepositoryViewParams,
"cfg" | "owner" | "repo" | "prNumber" | "headSha" | "repositorySizeKb"
>,
): Promise<void> {
const key = cacheKey(params);
const entry = cache.get(key);
Expand Down
1 change: 1 addition & 0 deletions src/settings/defaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,5 @@ export const DEFAULT_LOCAL_WORKSPACE_MAX_FILE_BYTES = 1_000_000;
export const DEFAULT_LOCAL_WORKSPACE_SEARCH_MAX_TOTAL_BYTES = 50_000_000;
export const DEFAULT_LOCAL_WORKSPACE_MAX_DIFF_BYTES = 5_000_000;
export const DEFAULT_LOCAL_WORKSPACE_MIN_FREE_SPACE_BYTES = 500_000_000;
export const DEFAULT_LOCAL_WORKSPACE_FULL_CLONE_MAX_REPO_KB = 1_000_000;
export const DEFAULT_LOCAL_WORKSPACE_STALE_CLEANUP_AGE_SECONDS = 86_400;
1 change: 1 addition & 0 deletions src/settings/envKeys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export const ENV = {
LOCAL_WORKSPACE_SEARCH_MAX_TOTAL_BYTES: "LOCAL_WORKSPACE_SEARCH_MAX_TOTAL_BYTES",
LOCAL_WORKSPACE_MAX_DIFF_BYTES: "LOCAL_WORKSPACE_MAX_DIFF_BYTES",
LOCAL_WORKSPACE_MIN_FREE_SPACE_BYTES: "LOCAL_WORKSPACE_MIN_FREE_SPACE_BYTES",
LOCAL_WORKSPACE_FULL_CLONE_MAX_REPO_KB: "LOCAL_WORKSPACE_FULL_CLONE_MAX_REPO_KB",
LOCAL_WORKSPACE_STALE_CLEANUP_AGE_SECONDS: "LOCAL_WORKSPACE_STALE_CLEANUP_AGE_SECONDS",
} as const;

Expand Down
1 change: 1 addition & 0 deletions src/webhook/payloads/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export const installationSchema = z.object({
export const repositorySchema = z.object({
owner: z.object({ login: z.string() }),
name: z.string(),
size: z.number().optional(),
});

/** GitHub App webhooks include `installation`; use loose top-level object so extra fields are allowed. */
Expand Down
Loading
Loading