Skip to content

Phase 18: respect buf.yaml dependency refs + prewarm removal - #38

Merged
onokonem merged 8 commits into
mainfrom
buf-proto-update-3
Jul 7, 2026
Merged

Phase 18: respect buf.yaml dependency refs + prewarm removal#38
onokonem merged 8 commits into
mainfrom
buf-proto-update-3

Conversation

@onokonem

@onokonem onokonem commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Phase 18: Respect buf.yaml dependency refs (not always HEAD); fix related bug; remove prewarm logic
Goal: Three corrections to dependency resolution, shipped as one atomic pass: (1) honor the ref field the buf CLI sends in the Name message so a dep like cyp/cyp-net-listeners:main/v2 returns the SHA at main/v2, not the HEAD; (2) replace the commit != "" && commit != "main" short-circuit in both bitbucket and github providers with an isSHA(commit) gate so non-SHA inputs route through the providers' commit-fetch APIs to resolve; (3) drop the prewarmHeads startup fan-out and replace it with a UUID→SHA-prefix inverse function (commitUUIDInverse) that lets probeCommitID recover the right module on a Download cache-miss without the prewarm.
Status: Verified ✓

A buf.yaml dep like cyp/cyp-net-listeners:main/v2 now returns the SHA at main/v2, not HEAD — moduleRef plumbs the buf BSR Name.ref (proto field 3) end-to-end through parseResourceRefNameServeHTTP/ServeGraph → provider GetMeta. The two providers now gate on isSHA(commit) (40/64 lowercase hex) instead of the commit != "" && commit != "main" short-circuit; non-SHA inputs route through their commit-fetch APIs. The prewarm fan-out is removed and replaced by a commitUUIDInverse helper that derives a 28-char SHA prefix from a 32-char buf-issued UUID; probeCommitID validates that the source's meta.Commit starts with the recovered prefix (fail-closed), which closes the review.md finding #6 wrong-source-match risk.

Changes

Plan 18-01: Refs respected + isSHA gate + prewarm removal

End-to-end ref support for buf.yaml deps + isSHA-gated provider ref-resolution + prewarm removal replaced by commitUUIDInverse-based probe

Key files created:

  • internal/providers/bitbucket/getrepo_test.go — 4 new TestGetMeta_* cases (Empty, RawSHA_40, RawSHA_64, ResolvesRef) for the Bitbucket provider
  • internal/providers/github/getrepo_test.go — 3 new TestGetMeta_* cases for the GitHub provider
  • internal/providers/github/mockrepos_test.go — new mockRepos mock with GetCommit plumbing and call-count tracking

Key files modified:

  • internal/connect/commits_helpers.gomoduleRef gains ref; parseResourceRefName reads field 3; new isSHA, isUUID, commitUUIDInverse helpers
  • internal/connect/commits_helpers_test.goTestIsSHA (8), TestIsUUID (6), TestParseResourceRefName_ReadsRef, TestParseResourceRefName_NoRef, TestCommitUUIDInverse (8)
  • internal/connect/commits.goServeHTTP/ServeGraph pass ref.ref; probeCommitID rewritten with commitUUIDInverse + prefix-match; prewarmHeads/registerResolved/prewarm fields deleted
  • internal/connect/api.goCommitResolution loses PrewarmEnabled/PrewarmTimeout; NewWithConfig drops the goroutine launch
  • internal/connect/api_test.goTestServeDownload_AfterRestart_ProbeResolvesUUID + TestServeDownload_AfterRestart_ProbeMissesOnUnknownUUID; mockSource accepts HasPrefix; TestPrewarmHeads_PopulatesCommitMap removed
  • internal/connect/uuid_format_test.go — stale registerResolved comment updated
  • internal/providers/bitbucket/getrepo.gogetMeta uses isSHA(commit) gate; new getCommit(ctx, ref) helper
  • internal/providers/bitbucket/client.go — new tmplGetCommit template for /commits/{{.id}}
  • internal/providers/github/getrepo.goGetMeta uses isSHA(commit) gate; non-SHA inputs route through repos.GetCommit
  • cmd/easyp/internal/config/config.goPrewarmConfig deleted; Connect retains Probe only
  • cmd/easyp/main.go — handler factory drops cc.Prewarm.* references

Requirements Addressed

  • SC-1 — refs respected: GetCommits/GetGraph return UUID from ref, not HEAD
  • SC-2 — related bug fixed: provider short-circuit replaced with isSHA gate
  • SC-3 — prewarm removed: prewarmHeads, registerResolved, related fields, config block
  • SC-4 — inverse helper: commitUUIDInverse + round-trip tests
  • SC-5 — no regression: Phase 16/17 tests still pass

Verification

  • go build ./... exits 0
  • go vet ./... exits 0
  • go test ./internal/connect/ -count=1 passes (all tests, no skips)
  • go test ./internal/providers/bitbucket/ -count=1 passes
  • go test ./internal/providers/github/ -count=1 passes
  • Targeted tests pass: TestCommitUUIDInverse (8 subtests), TestIsSHA (8), TestIsUUID (6), TestParseResourceRefName_ReadsRef, TestParseResourceRefName_NoRef, TestServeDownload_AfterRestart_ProbeResolvesUUID, TestServeDownload_AfterRestart_ProbeMissesOnUnknownUUID
  • Phase 16/17 regression guards pass: TestServeHTTP_GetCommits_ReturnsDashlessUUID, TestServeDownload_RoundTripWithMintedUUID, TestCommitUUID_SHA256_KnownSHA, TestCommitUUID_KnownSHA
  • Structural greps (production code, *.go files):
    • prewarmHeads / registerResolved$ (old) / PrewarmConfig / PrewarmEnabled / prewarmEnabled / prewarmOnce / prewarmTimeout0 hits
    • commitUUIDInverse — 1 hit
    • registerResolvedAlias — 1 hit
    • isSHA(commit) in bitbucket — 2 hits
    • isSHA(commit) in github — 2 hits
    • num == 3 && typ == protowire.BytesType (ref field 3 parse) — 1 hit
    • GetMeta(r.Context(), ref.owner, ref.module, ref.ref) (ServeHTTP + ServeGraph) — 2 hits
    • Prewarm in cmd/easyp/main.go and cmd/easyp/internal/config/config.go — 0 hits
    • strings.HasPrefix(meta.Commit, probeArg) (prefix-match validation) — 1 hit

Key Decisions

  • isSHA/isUUID are unexported in the connect package; each provider has its own 8-line local isSHA. Avoids an unexported-cross-package import path; helpers are pure functions, ~8 lines each, and the cost of one duplication is far less than the cost of exporting them and widening the public API.
  • registerResolvedAlias is a new helper, not a rename of registerResolved. Takes the original client-sent id (typically a buf-issued UUID) as the primary key and the resolved SHA as the alias — matches the buf v1.69.0 commit_id contract.
  • probeCommitID fails closed on a prefix mismatch. A source whose meta.Commit does not start with the recovered 28-char prefix is treated as a miss, not a hit. Closes review.md finding A tutorial is missing #6 (probe/register cache contract divergence): a wrong-source match would silently alias a real commit id to a wrong module and serve wrong content.
  • Inputs that are neither SHA-shaped (40/64 hex) nor UUID-shaped (32 hex) are negative-cached and return early in probeCommitID. No upstream fan-out for unresolvable inputs. The 'bogus000' fixture in Phase 16's probe tests was updated to a 40-char SHA to match the new gate.
  • The prewarm removal is observable to the client. A buf build immediately after a proxy restart will incur one extra upstream round-trip per module (the probe fan-out). The probeSem cap (maxConcurrentProbes=4) and per-source probeTimeout (8s default) bound the worst case at 4 × 8s = 32s per request in the absolute worst case; in practice the fan-out is parallel and completes in < 8s.
  • TDD with RED-then-GREEN at the task level. Failing tests committed first, then the production change in a separate commit. Phase 16/17 regression tests served as the safety net.

Deviations (4 auto-fixed, all in test data/infrastructure)

  1. Fixed test data length (66 → 64 chars) for the 64-char SHA case in TestCommitUUIDInverse — plan's fixture was off-by-two, commitUUID rejected it. Truncated to 64 chars.
  2. Fixed test expectation length (32 → 28 chars) in 3 of 4 success cases of TestCommitUUIDInverse — the inverse returns 14 bytes = 28 hex chars (the recoverable portion), not the full SHA. Plan's hand-derivation was off-by-four; function output is correct.
  3. Updated existing TestProbeCommitID_* tests for the new isSHA gate — old fixtures ("bogus000", "real") now bypass the probe via the early-return branch. Updated to 40-char lowercase hex.
  4. Updated mockSource.GetMeta to accept a SHA prefix — needed for the new post-restart probe path where the probe arg is a 28-char SHA prefix from commitUUIDInverse. Added !strings.HasPrefix(s.commit, commit) clause.

No production code was changed beyond the plan's intent.

onokonem and others added 8 commits July 7, 2026 16:52
…e ref

- TestIsSHA: 8 cases for the SHA-shape check (40/64 lowercase hex, reject refs and UUIDs)
- TestIsUUID: 6 cases for the 32-lowercase-hex check
- TestParseResourceRefName_ReadsRef: build a Name proto with ref=3, assert parsed
- TestParseResourceRefName_NoRef: build a Name proto without ref, assert ref=""

Tests fail at compile time: isSHA, isUUID, and moduleRef.ref do not exist yet.

Co-Authored-By: Claude <noreply@anthropic.com>
Refs end-to-end:
- moduleRef gains a ref field (buf BSR Name.ref, proto field 3)
- parseResourceRefName reads field 3 (ref), keeps existing owner/module arms
- ServeHTTP and ServeGraph pass ref.ref to GetMeta (was ""), so a buf
  client pinning cyp/cyp-net-listeners:main/v2 gets the SHA at main/v2
- ServeDownload unchanged (operates on UUIDs/SHAs, not refs)

Provider ref-resolution:
- Add isSHA helper in commits_helpers.go for the SHA-vs-ref gate
- Add isUUID helper for probeCommitID to detect UUID inputs
- Bitbucket: replace 'commit != "" && commit != "main"' with
  isSHA(commit); non-SHA inputs call new getCommit(ctx, ref) which
  hits /commits/{ref} and returns the resolved id
- GitHub: same gate; non-SHA inputs call repos.GetCommit(ctx, owner,
  repo, ref, nil) and stamp rc.GetSHA() into meta.Commit

Provider tests:
- bitbucket/getrepo_test.go: 4 subtests (Empty, RawSHA_40, RawSHA_64,
  ResolvesRef via httptest server returning {id,displayId})
- github/getrepo_test.go: 3 subtests (Empty, RawSHA_40, ResolvesRef
  via new mockRepos that supports GetCommit)
- github/mockrepos_test.go: new mockRepos mock with GetCommit plumbing
  and call-count tracking

Co-Authored-By: Claude <noreply@anthropic.com>
8 cases:
- 4 success: round-trip a known 40-char SHA, all-zero, all-ones,
  and a 64-char SHA-256 (recovery is shape-independent)
- 4 error: empty, 31 chars, 33 chars, 32 chars non-hex

Tests fail at compile time: commitUUIDInverse is not defined yet.

Co-Authored-By: Claude <noreply@anthropic.com>
The inverse of commitUUID: given a 32-char dashless UUID, recover the
first 28 hex characters of the SHA that produced it. Used by
probeCommitID (task 3) to derive a probe arg from a buf-issued UUID
input on the post-restart Download cache-miss path.

Inverse is lossy by design: commitUUID drops the last 6 bytes of the
SHA (2^112 collision surface) and overwrites the version/variant
bytes at positions 6 and 8. The recovered 14 bytes are sufficient to
identify a specific source among the configured providers and to
scope a probeCommitID fan-out to the right repository.

TestCommitUUIDInverse (8 cases):
- 4 success: round-trip a known 40-char SHA, all-zero, all-ones,
  and a 64-char SHA-256 (recovery is shape-independent)
- 4 error: empty, 31 chars, 33 chars, 32 chars non-hex

Co-Authored-By: Claude <noreply@anthropic.com>
…Inverse

Prewarm removal:
- Delete prewarmHeads and registerResolved from commits.go
- Delete prewarmOnce, prewarmEnabled, prewarmTimeout fields from
  commitServiceHandler
- Delete PrewarmEnabled, PrewarmTimeout fields from CommitResolution
- Delete the goroutine launch in NewWithConfig
- Delete PrewarmConfig struct from config.go
- Drop cc.Prewarm.* references in main.go
- Delete the now-obsolete TestPrewarmHeads_PopulatesCommitMap

Probe rewrite (review.md finding #6 closure):
- probeCommitID now uses commitUUIDInverse for 32-char UUID inputs
  and validates the source's meta.Commit starts with the recovered
  28-char prefix (prefix-match validation; fail-closed on mismatch)
- New registerResolvedAlias helper takes the buf-issued id (typically
  a UUID) as the primary key and the resolved SHA as the alias
- Inputs that are neither SHA-shaped (40/64 hex) nor UUID-shaped
  (32 hex) are negative-cached and return early (no upstream fan-out)

New tests:
- TestServeDownload_AfterRestart_ProbeResolvesUUID: post-restart
  Download with a UUID succeeds via the inverse + prefix-match path
- TestServeDownload_AfterRestart_ProbeMissesOnUnknownUUID: a UUID
  whose first 14 bytes do not match any source's HEAD returns 400
- Updated mockSource to accept a prefix match (HasPrefix check) for
  the post-restart probe fan-out

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@onokonem
onokonem merged commit bcdcbba into main Jul 7, 2026
1 check passed
onokonem added a commit that referenced this pull request Jul 8, 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.

1 participant