Skip to content

feat(git): pack unreachable loose objects during idle repo maintenance - #22344

Closed
brennanb2025 wants to merge 4 commits into
mainfrom
brennanb2025/repo-loose-object-maintenance
Closed

brennanb2025 wants to merge 4 commits into
mainfrom
brennanb2025/repo-loose-object-maintenance

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 12 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1840 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1026 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​814
Prod 19 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​987 $\color{#cf222e}{\Huge{\mathbf{−}}}$​416 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​571

ELI5

Orca makes throwaway Git objects in the background every time it checks two branches for conflicts. Git's own cleanup can't get rid of objects this new. Once there are enough of them, Git runs its full cleanup, which can't lower the count, so it starts the same slow cleanup again. Orca already tidies loose refs while you're idle. It now tidies these loose objects at the same idle moments, before Git reaches that point.

What Changed

Before: one large repository (4.6 GB, 802 worktrees) built up about 8,600 unreachable loose objects younger than two weeks. That is more than gc.auto (6,700), so every Git command that runs auto-maintenance started a git gc. On that repository each gc took about 40 minutes, and it did nothing about the count that triggered it, so the next command started another one. Packing those objects by id took 6 seconds and ended the loop.

After: Orca's idle maintenance checks the loose-object count for each repository it drives. Above 1,000 objects, it packs them in batches of up to 10,000 and removes the loose copies. Git's auto-gc never sees a count high enough to start the loop.

Mechanism:

  • One scheduler, several tasks. The idle ref sweep (RepoRefMaintenance) is now RepoMaintenance, with a list of tasks. Each task has an id, a threshold, a bounded probeBacklog, and a pack step. The scheduler still owns the quiet period, single-flight, pause/postpone, the busy gates and the deadline. It knows nothing task-specific. Cooldowns are kept per repository and per task, so a failed object pack does not delay the ref sweep.
  • Refs, then objects, then object-pack housekeeping, in one attempt. pack-refs goes first because worktree creates and ref listings wait on it. Before each task the scheduler checks abort, pause and that task's window again (see "When each task runs"). A task that is held off is skipped and the repository is re-armed for it; the other tasks still run.
  • Loose-object task (pack-loose-objects-maintenance-task.ts):
    • Probe: reads the 256 objects/xx fan-out directories of the common dir, one at a time, and stops at a budget. It runs no Git subprocess. Only two-hex-digit directories and 38- or 62-hex file names count, so tmp_obj_*, pack/, info/ and incoming-* are skipped.
    • Pack step:
      1. prune-packed
      2. scan up to batch + 1 ids
      3. pack-objects --quiet --non-empty <commonDir>/objects/pack/loose, with the ids on stdin
      4. prune-packed again
    • All of these commands exist in Git 2.25.
    • Passing ids on stdin means no history walk and no reachability test. That is what lets it pack the unreachable objects gc can't handle.
  • Batches and re-arm. A task reports batchExhausted when it had more objects than one batch could take. The scheduler then re-arms that repository after 30 seconds, up to 64 extra rounds for each arm(). A pack that finishes with objects still over the threshold, without spending a full batch, counts as a failure and gets the normal failure cooldown. It does not retry at once.
  • Kept packs, so an older Git cannot undo the work. On Git before 2.40, or with gc.cruftPacks=false, gc --auto runs repack -A. That command writes every unreachable object in an ordinary pack back out as a loose file, which would undo the object task on every such gc. So each pack the task writes gets a .keep file, marked as Orca's, before the loose copies are removed. repack -A leaves kept packs alone, and gc --auto does not count them toward gc.autoPackLimit.
  • Object-pack housekeeping (third task, object-packs) is how each keep is eventually dropped:
    • Release: once a pack is as old as gc.pruneExpire, Orca deletes its keep and Git's own gc owns the pack again. By then any unreachable object in it is old enough that even a pre-cruft repack -A drops it instead of writing it loose. The setting is read with git config --type=expiry-date, so every spelling Git accepts works. never means packs are never released.
    • Merge: each batch is its own pack, and kept packs are invisible to gc.autoPackLimit, so nothing else would ever combine them. Kept packs from the same UTC day are merged into one (up to 100,000 objects), and the merged pack's mtime is set to its newest member's. With gc.pruneExpire=never, packs from any day can be merged.
    • Order: the merged pack and its keep are fully written before any member is deleted. Members are removed in Git's own order: the index goes last, then the keep.
    • Only Orca's keeps: a keep counts as Orca's only if its content is Orca's marker. Keeps written by the user or another tool are never touched. Object ids come from reading the version-2 .idx directly, with no Git subprocess, and any pack whose index is unreadable is skipped.
  • When each task runs. The idle gate now reports two separate things: interactive (agents working, a worktree create or removal in flight) and constrained (quitting, on battery, or one-minute load at or above one task per core). Ref packing still waits for a quiet, non-interactive window, because pack-refs holds the packed-refs lock that branch deletions need. The two object tasks take no lock that anything else waits on, so they run beside live agents and only hold off on a constrained machine. Window focus now records the time of user activity (so ref packing waits another quiet period) instead of cancelling the whole attempt. Tasks that are skipped are listed in repo.maintenance_skipped.
  • Opt-out covers every task. maintenance.auto set to false or gc.auto of 0 or below in the user's own config leaves the repository alone. Both are read with git config --type=bool / --type=int, so no, off, 0, FALSE and 1k all mean what Git means by them. A missing or unreadable value counts as consent. Nothing is ever written to Git config. The kill switch is ORCA_DISABLE_REPO_MAINTENANCE. The old ORCA_DISABLE_REPO_REF_MAINTENANCE still works.
  • WSL: the main process reads the UNC path. The pack base passed to Git stays in Git's own path spelling.
  • Renames: repo-ref-maintenance* → repo-maintenance*. The span is now repo.maintenance, with per-task attributes repo.maintenance.<task>.{backlog,threshold,outcome,duration_ms,backlog_after,partial,error}.

Why

  • Why pack by id instead of running git gc / repack: both work by reachability. They can't put unreachable objects into a pack, and they can't prune them until gc.pruneExpire (two weeks). That is why the loop happens in the first place.
  • Why not git maintenance run --task=loose-objects: it needs Git 2.30 or later, and 2.25 is our baseline. Using it would also mean either detecting the capability for every host or writing maintenance config. The plumbing recipe does the same job on every supported Git. An optional fast path through maintenance run is not included (see gaps below).
  • Why extend the existing scheduler rather than add a second one: both jobs need the same quiet window, the same pause during ref-touching work, the same busy and battery gates, and the same shutdown handling. Two schedulers could run at the same time and would each need their own copy of those rules.

Deviations from precedent:

  • The desktop-agent projects we compared against run no repository maintenance at all. At least one turns off Git's auto-maintenance on its fetches (--no-auto-maintenance) and does nothing else in its place.
  • So the only precedent is Orca's own idle ref sweep. That is a reuse precedent (same gates and lifecycle), not proof that the design is right.
  • New in this PR, with no precedent: per-task cooldowns, the batch re-arm, and Orca-owned .keep files released by age.

Linked Issue

No tracking issue: found while diagnosing a runaway git gc loop on a large local repository.

Visual Proof

N/A: background Git maintenance with no UI change.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Unit tests:

  • loose-object-scan: counting, stopping at the budget, only fan-out directories opened, tmp_obj_* skipped, SHA-256 names, a missing store, cancellation.
  • repo-maintenance (scheduler): task order, per-task cooldowns, batch re-arm and its cap, a finished-but-not-drained pack counted as failed, deferral between tasks, gating and single-flight.
  • pack-loose-objects-maintenance-task: argv, ids on stdin, batch limit and the batchExhausted report, no config calls, the unresolvable-repository case.
  • local-repo-maintenance: task order (refs, objects, object-packs); objects pack while agents are working and refs wait; nothing runs when the gate cannot be read; opt-out through typed config; both kill switches.
  • Scheduler task windows: an unconstrained task runs through interactive work and recent user activity, and every task waits on a constrained machine.
  • Idle gate: agents, creates and removals count as interactive; battery, quitting and a saturated CPU count as constrained.
  • planObjectPackHousekeeping: release by age and for orphaned keeps, merge by day or across days, the object cap, and full or unreadable packs left out.
  • The object task writes its keep before the final prune-packed.

Real-git integration (repo-maintenance-real-git.test.ts):

  • About 3,000 unreachable, freshly written blobs. After one attempt:
    • the loose-object count is 0;
    • refs, index, HEAD and packed-refs are byte-identical;
    • every object id passes cat-file --batch-check;
    • fsck is clean.
  • With a batch of 100, the count drops below the threshold across rounds.
  • Loose refs and loose objects are both handled in one attempt.
  • maintenance.auto set to false/no/off/0/FALSE, and gc.auto=-1, each leave the repository untouched. yes and gc.auto=1k do not.

Real-git, kept packs (object-pack-housekeeping-real-git.test.ts):

  • Simulated pre-cruft gc: after Orca packs 300 unreachable objects, git -c gc.cruftPacks=false repack -d -l -A --unpack-unreachable=2.weeks.ago leaves 0 loose objects and none missing.
  • Control: delete the keep and run the same command, and all 300 come back as loose files.
  • Three same-day batch packs merge into one kept pack with every object still readable, the newest member's mtime, and nothing left owed.
  • Packs from different days stay separate by default and merge under gc.pruneExpire=never.
  • gc.pruneExpire=now releases the keep, after which Git's own gc --prune=now removes the unreachable objects.
  • A keep without Orca's marker is never released.

Git binary contract: git-binary-compatibility.test.ts covers, on every Git in the CI matrix including 2.25.5:

  • pack-objects with ids on stdin, printing the pack hash;
  • a kept pack surviving repack -A;
  • prune-packed;
  • config --type=bool, --type=int and --type=expiry-date (never reads as 0).

Ablations:

  • Removing the object task from the local target: 6 tests fail.
  • Removing the final prune-packed: the real-git tests fail, with the loose count stuck at 3,003 and 103.
  • Skipping the keep write: 6 tests fail, including the pre-cruft repack test.
  • Removing the object task's unconstrained window: the "packs while agents are working" test fails. Removing the scheduler's window check: 3 tests fail.
  • Reverting the typed-config opt-out: 5 spelling tests fail.

Commands run on the final head:

  • vitest src/main/git src/shared/repo-maintenance.test.ts src/main/repo-maintenance-idle-gate.test.ts: 1514 pass, 1 fails. The failure is git-admission-storm-measurement, which this PR does not touch; it hits its 5s timeout on its own too, with the machine's load average between 24 and 31.
  • Compat suite on local Git 2.50.1: 20/20 pass.
  • pnpm tc: clean.
  • oxlint src/: clean.
  • pnpm run check:code-quality:changed: passes.

Tested on macOS only. Windows and WSL path handling is covered by unit tests, not tested by hand.

AI Disclosure

Written with Claude (Claude Code).

Review

Remaining gaps:

  • SSH-hosted repositories are still out of scope, as they were for the ref sweep. Maintaining them means running the tasks on the relay, which has no admission control or spans today.
  • Git 2.25: proven only by the CI compat job. The local docker check was not available for this run.
  • No git maintenance run fast path. It is optional, and adding it would need per-host capability detection.
  • Reachable objects in a kept pack wait for it to be released. If an object Orca packed becomes reachable later, gc will not fold it into the main pack until the keep is released (two weeks by default). It is still readable the whole time; this costs a little disk and lookup locality, not correctness.
  • Merging extends an object's life by at most one day, because the merged pack takes the newest member's mtime.
  • On Windows the CPU check never trips, because Node reports a load average of 0 there. Battery and quitting still hold the object tasks off.
  • Adjacent: fix(git): keep merge-tree's throwaway trees out of the real object store #22356 stops merge-tree --write-tree from writing these objects into the object store at all. This PR still handles loose objects from every other source, and any backlog that already exists.

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Security: no new inputs. Object ids come from file names that are checked against a hex pattern.
  • Cross-platform: all paths are built with path, and WSL spelling is handled.
  • SSH: out of scope, as above.
  • Mobile: unaffected.
  • Backwards compatibility: the old kill-switch name is still honoured.
  • Performance:
    • The probe is a bounded read of directories, with no subprocess.
    • Packing uses the background admission tier and never takes an abort signal. That rule comes from the ref sweep, where killing a pack mid-write leaves lock files behind.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Orca's conflict summary runs `merge-tree --write-tree`, which leaves a few
unreachable loose objects behind on every divergent call. Once a repository
holds more than gc.auto (6700) of them, every Git command that runs auto
maintenance starts a full gc that cannot pack or prune them, so the gc starts
again straight away. On a large repository that loop kept a 40-minute gc
running continuously.

The idle ref sweep is now a small scheduler of tasks. Each task has a bounded
probe and a pack step. After the ref task, a loose-object task:
- counts the objects/xx fan-out directly, without running Git;
- above 1000 objects, passes a batch of object ids on stdin to
  `pack-objects`, then runs `prune-packed`;
- re-arms for whatever is left after a batch.
Only Git 2.25 plumbing is used, and nothing is written to the user's Git
config. A repository that sets `maintenance.auto=false` or `gc.auto=0` is left
alone.
@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

📝 Walkthrough

Walkthrough

The change generalizes idle repository maintenance from loose-ref packing to ordered refs and loose-object tasks. It adds shared policy, path-resolution helpers, loose-object scanning, task execution, and per-task cooldowns. Git-specific task implementations and local target wiring support ref and object packing. Application operations now use repository-wide maintenance pause, postpone, and lifecycle APIs. New unit and real-Git tests cover scanning, scheduling, packing, and application integration.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to 16d5a

Idle repository maintenance can start a Git packing job right after it was cancelled, such as when the app is quitting. That job may be interrupted, and the repository is put on a six-hour failure cooldown it did not earn. Repository data is not at risk. Checking for cancellation after each probe is a small fix worth making before merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 52.24% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 67 functions across 27 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the main change: packing unreachable loose objects during idle repository maintenance.
Description check ✅ Passed The description covers the change, rationale, testing, visual proof, AI disclosure, review notes, and checklist. However, the Linked Issue section states that there is no tracking issue, despite the t…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 16269bc6-c253-4fda-a4e8-ea78d3e74dc3

📥 Commits

Reviewing files that changed from the base of the PR and between 7240368 and 16d5ab0.

📒 Files selected for processing (31)
  • src/main/git/git-common-dir-paths.test.ts
  • src/main/git/git-common-dir-paths.ts
  • src/main/git/local-repo-maintenance.test.ts
  • src/main/git/local-repo-maintenance.ts
  • src/main/git/local-repo-ref-maintenance.test.ts
  • src/main/git/pack-loose-objects-maintenance-task.test.ts
  • src/main/git/pack-loose-objects-maintenance-task.ts
  • src/main/git/pack-refs-lock-ownership.ts
  • src/main/git/pack-refs-maintenance-task.ts
  • src/main/git/remote.ts
  • src/main/git/repo-maintenance-real-git.test.ts
  • src/main/git/repo-ref-maintenance-real-git.test.ts
  • src/main/git/worktree-add.ts
  • src/main/git/worktree-branch-removal.ts
  • src/main/git/worktree-create-preparation.ts
  • src/main/git/worktree-removal.ts
  • src/main/repo-maintenance-idle-gate.test.ts
  • src/main/repo-maintenance-idle-gate.ts
  • src/main/runtime/runtime-remote-fetch-controller.ts
  • src/main/runtime/runtime-remote-fetch-maintenance.test.ts
  • src/main/startup/main-process-quit.ts
  • src/shared/git-binary-compatibility.test.ts
  • src/shared/loose-object-scan.test.ts
  • src/shared/loose-object-scan.ts
  • src/shared/repo-maintenance-cooldowns.ts
  • src/shared/repo-maintenance-policy.ts
  • src/shared/repo-maintenance-task-run.ts
  • src/shared/repo-maintenance.test.ts
  • src/shared/repo-maintenance.ts
  • src/shared/repo-ref-maintenance-policy.ts
  • src/shared/repo-ref-maintenance.test.ts
💤 Files with no reviewable changes (4)
  • src/main/git/repo-ref-maintenance-real-git.test.ts
  • src/main/git/local-repo-ref-maintenance.test.ts
  • src/shared/repo-ref-maintenance.test.ts
  • src/shared/repo-ref-maintenance-policy.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.

Comment on lines +39 to +49
// A saturated probe stopped early, so `count` is a floor -- never read it as "clean".
if (!before.saturated && before.count < task.threshold) {
return finish(span, task, { outcome: 'below_threshold', remainder: false })
}
const startedAt = now()
let partial = false
let batchExhausted = false
try {
// No signal: the pack runs to completion. Callers that need the refs wait
// out the rewrite window through `pause()` instead of killing it.
batchExhausted = (await task.pack(lock)).batchExhausted

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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Stop before pack() when the probe ended because of an abort.

countLooseRefs and scanLooseObjects return saturated: true when signal is aborted. Line 40 treats a saturated probe as "must pack". So if dispose() (quit) or the attempt deadline fires during the probe, Line 49 starts a new pack-refs or pack-objects child after cancellation. At quit, that child can then be killed mid-run. This is the stranded-lock case that the no-signal pack() contract exists to prevent. The runDueTasks check signal.aborted happens only between tasks, so it does not cover this window.

The re-probe at Line 64 has the same problem. After an abort it returns a saturated floor, and the verdict becomes failed with a 6-hour cooldown, even though the abort came from Orca.

Check the signal after each probe. Then let runDueTasks route the abort to endAborted.

🐛 Proposed fix
   const before = await task.probeBacklog(budget, signal)
+  // An aborted probe reports a floor; never start a pack Orca already cancelled.
+  signal.throwIfAborted()
   if (!before) {

In src/shared/repo-maintenance.ts runDueTasks, wrap the call:

try {
  remainder = (await this.runTask(key, task, span, signal)) || remainder
} catch (error) {
  if (signal.aborted) {
    this.endAborted(key, tracked, span, signal)
    return
  }
  throw error
}

Before Line 64, when signal.aborted is true, skip the re-probe and return the verdict without the failed cooldown.

@pullfrog pullfrog Bot left a comment

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.

ℹ️ No critical issues — one small abort-handling regression left over from the refactor, plus a design question about pack growth.

Reviewed changes

Initial review of 16d5ab06b7.

  • Multi-task scheduler — RepoRefMaintenance becomes RepoMaintenance, now task-blind: per repo it filters target.tasks against per-task cooldowns, then runs each due task through probe → threshold → pack → re-probe (repo-maintenance.ts, repo-maintenance-task-run.ts).
  • Per-task cooldowns and batch re-arm — new RepoMaintenanceCooldowns; a task that spent a full batch without draining the backlog re-arms the repo after 30s, capped at 64 rounds, while a task that finished and left a backlog is a failure, not a remainder.
  • Loose-object task — pack-loose-objects-maintenance-task.ts runs prune-packed → scan ≤ batch+1 objects → pack-objects --quiet --non-empty …/objects/pack/loose with ids on stdin → prune-packed, packing unreachable objects by name instead of by reachability.
  • Bounded scan — loose-object-scan.ts readdir-only walk of the 256 objects/xx fan-out dirs, budgeted, sequential, with 38/62-hex name gating so tmp_obj_* and non-objects are skipped.
  • Path spelling split — git-common-dir-paths.ts separates the main-process spelling (WSL Linux path → UNC) from Git's own spelling (pack base path kept untranslated).
  • Renames and kill switches — old repo-ref-maintenance* names removed, span renamed repo.maintenance with per-task attributes, ORCA_DISABLE_REPO_MAINTENANCE added while the old ORCA_DISABLE_REPO_REF_MAINTENANCE still works.
  • Tests — new unit suites for the scan, the object task, the scheduler, and local wiring; the real-Git suite now covers a 3,000-unreachable-object drain plus the existing ref cases.

I ran the changed suites locally: the four unit files (51 tests) and repo-maintenance-real-git.test.ts (10 tests, ~3s) all pass.

ℹ️ Packs are written but nothing consolidates them

Each run writes a new uniquely-named objects/pack/loose-<sha>.pack and deletes the loose copies, but Orca has no step that folds those packs together, and it suppresses Git's auto-maintenance on its own fetches (gc.auto=0 in GIT_FETCH_SKIP_AUTO_MAINTENANCE_CONFIG_ARGS disables the gc --auto path that also handles gc.autoPackLimit). On a repo that crosses the 1,000-object threshold often, the pack directory can keep growing until some git command outside Orca runs gc/repack.

Technical details
# Pack accumulation with no consolidation path in Orca

## Affected sites
- `src/main/git/pack-loose-objects-maintenance-task.ts:92-98` — writes a fresh `loose-<sha>.pack` per batch, no cleanup of prior packs.
- `src/shared/repo-maintenance-policy.ts:159` — `LOOSE_OBJECT_PACK_BASE_NAME = 'loose'`.
- `src/shared/git-fetch-auto-maintenance.ts:3-10` — Orca's fetches set `gc.auto=0`, which also suppresses the pack-limit-triggered `gc --auto`.

## Required outcome
- Confirm whether the intended bound on pack count is "the user's own `git gc` / other git commands eventually consolidate", or whether the task should avoid repeated small packs (e.g. a stable base name or a later consolidation step).

## Open questions for the human
- Is pack-count growth on an Orca-only checkout (no user-run git) considered acceptable?
- Does the field repository (802 worktrees) already show a rising `objects/pack` file count after this lands?

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

try {
// No signal: the pack runs to completion. Callers that need the refs wait
// out the rewrite window through `pause()` instead of killing it.
batchExhausted = (await task.pack(lock)).batchExhausted

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.

The pre-refactor packIfNeeded re-checked signal.aborted right after the probe and before packing; runMaintenanceTask dropped that, so an attempt torn down during a probe still starts a pack.

Concretely: dispose() on quit, or the 20-minute attempt deadline, can abort while probeBacklog is walking the refs tree or the object fan-out. The probe returns, the threshold check passes, and task.pack(lock) runs anyway — including pack-refs, which takes packed-refs.lock for its rewrite window. Restoring the guard keeps a shutdown/deadline from beginning new pack-refs work.

Technical details
# Aborted attempt still starts a pack

## Affected sites
- `src/shared/repo-maintenance-task-run.ts:33-49` — probe resolves, no `signal.aborted` check before `task.pack(lock)`.
- `src/shared/repo-maintenance.ts:306-318` — `runDueTasks` checks `signal.aborted` only before each task, not during a task's probe.

## Required outcome
- An attempt aborted by `dispose()`/quit or by the attempt deadline must not begin a pack.

## Suggested approach (optional)
- The task verdict vocabulary has no abort state, so either add one (e.g. an `interrupted` outcome the scheduler maps like `endAborted`) or have `runMaintenanceTask` throw/return a signal the caller turns into `endAborted`. The old code simply yielded: `if (signal.aborted) { this.yieldTo(...); return }`.

## Open questions for the human
- Is the dropped check intentional — i.e. should a probe that crossed the threshold always pack — or an oversight from splitting the ref sweep into task runners?

…lode them

On Git before cruft packs, or with gc.cruftPacks=false, `gc --auto` runs
`repack -A`, which writes every unreachable object in an unkept pack back
out as a loose file. That undid the idle object task on every such gc.

Each pack the task writes now gets an Orca-marked .keep, written before the
loose copies are pruned. A third task, object-packs, is the way that keep
dies: it releases a keep once its pack is as old as gc.pruneExpire (read
through Git's typed config), and merges same-day kept packs so the per-batch
packs do not pile up where gc.autoPackLimit never sees them.

@pullfrog pullfrog Bot left a comment

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.

ℹ️ No critical issues — one doc-accuracy nit inline. The three new commits (kept packs + object-pack housekeeping, per-task run windows, typed opt-out read) are otherwise sound, and every CI lane including the Git 2.25.5 compatibility job is green.

Reviewed changes

Delta against the prior pullfrog review (16d5ab06):

  • Kept packs and object-pack housekeeping — every pack the loose-object task writes now gets a .keep, and a third object-packs task releases a keep once the pack reaches gc.pruneExpire and merges same-day kept packs into one aged to its newest member (loose-object-pack-keep.ts, object-pack-housekeeping-maintenance-task.ts).
  • Direct .idx reads — pack-index-object-ids.ts reads object counts and ids from a version-2 index with no Git child, treating anything else as "leave this pack alone".
  • Per-task run windows — RepoMaintenanceTask.window is new; the two object tasks are unconstrained and run beside live agents, while quit, battery and a saturated CPU still hold everything off (repo-maintenance.ts, repo-maintenance-idle-gate.ts).
  • Typed opt-out read — maintenance.auto and gc.auto are read through git config --type=bool / --type=int, so every spelling Git accepts (no, off, 0, FALSE, 1k, -1) is honoured (local-repo-maintenance.ts).
  • Tests — new real-Git suite for kept packs against a pre-cruft repack -A, merge and release; unit tests for the housekeeping plan; the compat suite gained the loose-object recipe and --type=expiry-date at the 2.25 baseline.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

/**
* Hold the repository open for work that is about to touch refs.
*
* Two things at once: no *new* attempt can start for any repository until the

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.

blockFor now lets an unconstrained task start an attempt while a pause() is held, so this contract overstates the fence: a repository whose only due tasks are the object tasks can begin work during ref-touching operations. Saying idle-window explicitly keeps the doc honest about what pause() stops.

Suggested change
* Two things at once: no *new* attempt can start for any repository until the
* Two things at once: no *new* `idle`-window task can start for any repository until the

@OrcaWin

OrcaWin commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Closing this standing maintenance design under the cost/benefit criterion in #27143: performance work should be free, or deliver a substantial enough remaining gain to justify its ongoing complexity.

Main now includes #22356 (0fea878680bc391c89439f4c6c5cf3ae430fefeb), which addresses the documented recurring producer directly: conflict-summary and squash-cleanup merge-tree --write-tree calls use a disposable object store, preserve partial-clone downloads, and clean up abandoned owned scratch stores. I checked this in main 77130982faf44abe3e4fb02354e79b61cdad5358. Its deliberate capability/environment/path fallbacks mean this is not a claim that every possible loose-object source is gone.

The historical result remains useful evidence: the affected 4.6 GB / 802-worktree repository had about 8,600 young unreachable objects, with reported 40-minute GC runs versus a 6-second by-ID pack. That demonstrates the value of repairing that backlog. It does not establish a continuing post-#22356 workload that warrants this general background mechanism. Preexisting backlogs and objects created by other tools remain possible, so this is a merits decision, not complete supersession.

The remaining benefit does not justify adding custom kept-pack ownership, age-based release, same-day merging, index parsing, per-task cooldowns and batch rearming to routine repository maintenance. Healthy repositories also acquire object-directory probes; object packing can run beside active agents, and the CPU-load gate has no effect on Windows. These are real ongoing costs and obligations, even though the individual pack operations have useful bounds and correctness tests. A focused repair for an observed remaining backlog would be a smaller basis for further work.

The typed gc.auto <= 0 opt-out correction is independently useful—particularly negative values—and can be extracted into the existing maintenance path, preserving main's newer failed-config-probe refusal and admission safeguards. Main's existing ref and pack-index maintenance should also be retained.

Reviewed the complete production change, changed test intents/assertions and discussion at unchanged head e5a4ac173274902949d82205f0f7138631697e6f. The reported Git integration/compatibility runs were not rerun here; no new timing or platform-performance claim is being made. Preserving the branch for any focused extraction.

@OrcaWin OrcaWin closed this Oct 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants