fix(gitengine): bound REF_DELTA recursion and harden object-store lifetime

[hammadmajid]

Description

grg reads arbitrary .git directories, and a packfile containing an OBJ_REF_DELTA cycle killed the process outright. Four smaller object-store defects found by the same audit ride along, because they live in the same files.

Closes #12

Type of Change

• Bug fix (non-breaking change fixing an issue)
• New feature (non-breaking change adding functionality)
• Performance improvement
• Refactoring or code cleanup
• Documentation update
• CI/CD or build workflow change

Key Changes

1. CRITICAL — OBJ_REF_DELTA cycle caused fatal error: stack overflow.
maxDeltaDepth = 50 only counted intra-pack OFS_DELTA hops. Every REF_DELTA hop re-entered through ObjectReader.ReadObject → PackReader.ReadObject → readObjectAt(offset, **0**), resetting the counter, so an A→B→A cycle recursed until the stack died. This was a fatal error, not a panic: unrecoverable, skipping defer reader.Close() and the whole exitCodeForError contract, triggered from a pipeline worker goroutine.

Added an unexported deltaBaseResolver interface with readObjectDepth(oid, depth). PackReader.ReadObject delegates to readObjectDepth(oid, 0); the REF_DELTA hop passes depth+1 through both the resolver and the self-lookup branch. RepositoryReader implements it and forwards. The exported ObjectReader surface is unchanged, and the resolver field is typed as the unexported interface so depth can never be silently dropped again.

2. Close() closed packfiles under in-flight readers. The snapshot-then-unlock in ReadObject correctly avoided the recursive-RLock deadlock through the resolver back-edge, but it made mu a field lock, not a lifetime lock — so Close could close an *os.File a reader still held. ErrFileClosed was then swallowed into ErrObjectNotFound: a silent wrong answer during shutdown, invisible to -race because it is a lifetime bug, not a data race. Now refcounted with inflight sync.WaitGroup + closed atomic.Bool; Close marks closed, unlocks, waits, and closes files outside the lock. New exported ErrReaderClosed. readObjectDepth also stops collapsing real source errors into ErrObjectNotFound.

3. SetResolver deleted. An exported unsynchronized write to a field read by every worker, on a type whose doc claimed thread-safety — safe only by accident of call-site placement. NewRepositoryReader now assigns the field before the reader is published; it is immutable afterwards.

4. ApplyDeltaWithBuffer takes *[]byte. Taking the slice by value meant a >64 KiB target reallocated and the caller recycled the original small array, defeating the pool on exactly the objects it was added for.

5. Failed pooled-zlib Reset no longer retries on a mutated stream. Reset eats the RFC-1950 header and installs a read-ahead bufio.Reader before it can fail, so the old zlib.NewReader(f) fallback parsed from a garbage offset. Shared newPooledZlibReader now returns the error directly on the pooled path.

Verification & Testing

• Ran go test -v -count=1 ./...
• Ran go test -race -shuffle=on -count=1 ./...
• Ran go vet ./...
• Added or updated unit/integration tests
• Tested manually against sample Git repository histories

go build ./...                                             clean
go vet ./internal/gitengine/ ./test/benchmark/             clean
go test -race -shuffle=on -count=1 ./internal/gitengine/   ok 1.2s

Reproduction, before the fix — debug.SetMaxStack(16<<20) to fail fast:

runtime: goroutine stack exceeds 16777216-byte limit
fatal error: stack overflow

gitengine.(*PackReader).readObjectAt(0x2408..., 0x32, 0x0)   pack.go:278
gitengine.(*PackReader).ReadObject(...)                       pack.go:120
gitengine.(*RepositoryReader).ReadObject(...)                 object.go:138
gitengine.(*PackReader).readObjectAt(0x2408..., 0xc,  0x0)   pack.go:278

The depth argument is 0x0 in every frame.

New tests. TestRefDeltaCycle_ReturnsCorruptObject builds an A→B→A cycle and drives it through NewRepositoryReader (production resolver wiring), asserting the error wraps ErrCorruptObject and is not ErrObjectNotFound. TestPackReaderRefDelta adds the missing positive coverage for valid REF_DELTA on both the resolver and nil-resolver paths — there was none before. TestConcurrency_RepositoryReader_Deadlock previously used a zero-value reader with no files, so it exercised nothing; it now races 20 readers × 100 reads against 5 concurrent Close calls on a real packfile-backed reader. An instrumented probe over 50 iterations recorded ok=1854 closed=98146 notfound=0 other=0, confirming both asserted outcomes occur and neither forbidden outcome does. TestPerformance_ApplyDeltaWithBuffer_GrowsCallerBuffer fails on the old by-value signature.

buildTestPackAndIdx gained a base-SHA field so the suite can build REF_DELTA packs at all.

Note

Bottom of a 7-PR stack. Nothing below it.

Checklist

• gofmt clean
• Every new test mutation-checked: reverting the fix makes it fail
• This layer builds and passes the full race suite on its own, not just at the top of the stack