Phase 25: Address PR #39 post-merge review findings

.planning/PROJECT.md

@@ -86,7 +86,9 @@ This document evolves at phase transitions and milestone boundaries.
 
 ## Current State
 
+Phase 23 complete (2026-07-09, local) — two v1.69.0 e2e tests close the ref-shape coverage gap from Phase 19. `TestRefRespected_BranchName_PinsBranchTip` proves a branch-name ref (`gh-pages`) pins buf.lock to the branch tip via the `repos.GetCommit` fall-through; `TestRefRespected_NonDefaultBranchCommitSHA` proves a raw 40-char SHA of a commit off the default branch is honored via the `isSHA` fast path. Both behaviors shipped in Phase 18; this phase is test-only (commit `8cf99fa` + docs commit). Ref-shape e2e coverage now spans tag (P19), branch name (P23-T1), and raw SHA off-default-branch (P23-T2). Tests skip cleanly token-less; live run pending `EASYP_GH_TOKEN` + cached buf binaries.
+
 Phase 22 complete (2026-07-08) — v1.30.1 v1alpha1 read-path now resolves 32-char buf UUIDs to 40-char git SHAs via the Phase 18 probe ladder before provider GetFiles calls. PR-22-1/2/3/5 unit-verified. PR-22-4 live e2e (TestGenerateWithPinnedBufLock/v1.30.1) blocked by provider-transport TLS issue to raw.githubusercontent.com (handler fix independently proven) — tracked in 22-HUMAN-UAT.md. Code review (22-REVIEW.md) flagged CR-01 stale-digest + 6 warnings as future-hardening candidates.
 
 ---
-*Last updated: 2026-07-08 after Phase 22 completion (v1.30.1 v1alpha1 read-path UUID resolution)*
+*Last updated: 2026-07-09 after Phase 23 completion (branch-name + non-default-branch commit ref e2e tests)*

.planning/ROADMAP.md

@@ -44,6 +44,11 @@
 - [x] **Phase 18: Respect buf.yaml dependency refs** — Honor `Name.ref` end-to-end + isSHA-gated provider ref-resolution + prewarm removal with `commitUUIDInverse`-based probe (completed 2026-07-07)
 - [x] **Phase 19: e2e tests for ref-honoring in buf.yaml deps** — Three `TestRefRespected_*` tests that exercise the proxy's ref-honoring behavior end-to-end via a real buf CLI + real GitHub API; tests pass with `EASYP_GH_TOKEN` set, skip cleanly otherwise (completed 2026-07-07)
 - [ ] **Phase 20: Fix ref-honoring regressions discovered in Phase 19** — Restore the no-ref (HEAD) path that Phase 18 broke for buf v1.30.1, and fix the v1beta1 path where the ref is being ignored on `buf dep update`
+- [x] **Phase 21: e2e test for buf generate with pinned buf.lock** — Add `TestGenerateWithPinnedBufLock` matrix test that proves proxy can serve content for a pre-existing pinned commit (completed 2026-07-08)
+- [x] **Phase 22: Fix v1.30.1 v1alpha1 read-path UUID handling** — Wire v1alpha1 DownloadManifestAndBlobs to Phase 18 UUID-resolution machinery (completed 2026-07-09)
+- [x] **Phase 23: e2e tests for branch-name and non-default-branch commit refs** — Add branch-name and raw-SHA ref coverage for buf.yaml deps (completed 2026-07-09)
+- [x] **Phase 24: Resolve buf cid ref in ServeGraph; honor pinned commit** — cid→sha map + ServeGraph UUID branch + infoCache cid-gating + ServeDownload cid→sha preference (completed 2026-07-09)
+- [x] **Phase 25: Address PR #39 post-merge review findings** — Fix 7 post-merge findings: extract duplicated helpers and regex, retain carve-out via shared helper, fix error wrap strings, verify v1alpha1 e2e, harden commitResolver construction (completed 2026-07-10)
 
 ## Phase Details
 
@@ -160,11 +165,11 @@ Plans:
 | 3. Test Infrastructure        | v1.1 | 2/2 | Complete | 2026-05-07 |
 | 4. Old Protocol Validation    | v1.1 | 1/1 | Complete | 2026-05-07 |
 | 5. New Protocol Validation    | v1.1 | 2/2 | Complete | 2026-05-07 |
-| 6. Dependency Upgrades        | v1.2 | 2/2 | Complete | 2026-05-08 |
-| 7. Proto Regeneration         | v1.2 | 2/2 | Complete | 2026-05-08 |
-| 8. Go Code Modernization      | v1.2 | 1/1 | Complete | 2026-05-08 |
-| 9. Submodule Cleanup          | v1.2 | 1/1 | Complete | 2026-05-09 |
-| 10. Code Quality Fixes        | v1.2 | 4/4 | Complete | 2026-05-09 |
+| 6. Dependency Upgrades        | v1.1 | 2/2 | Complete | 2026-05-08 |
+| 7. Proto Regeneration         | v1.1 | 2/2 | Complete | 2026-05-08 |
+| 8. Go Code Modernization      | v1.1 | 1/1 | Complete | 2026-05-08 |
+| 9. Submodule Cleanup          | v1.1 | 1/1 | Complete | 2026-05-09 |
+| 10. Code Quality Fixes        | v1.1 | 4/4 | Complete | 2026-05-09 |
 | 11. Logging Foundation        | v1.3 | 0/0 | Not started | - |
 | 12. Logging Infrastructure    | v1.3 | 0/0 | Not started | - |
 | 13. Error Path Logging        | v1.3 | 0/0 | Not started | - |
@@ -238,6 +243,28 @@ Plans:
 
 - [ ] [22-01](./phases/22-fix-v1-30-1-v1alpha1-read-path-uuid-handling-verify-v1-proto/22-01-PLAN.md) — Wire v1alpha1 DownloadManifestAndBlobs to Phase 18 UUID-resolution (CommitResolver interface + resolveCommitForRead wrapper + isUUID branch in blobs.go) with TDD unit cover for PR-22-1/2/3 and the Phase 21 e2e gate for PR-22-4
 
+### Phase 24: Resolve buf cid ref in ServeGraph; honor pinned commit
+
+**Goal:** When a client sends a proxy-minted 32-hex buf commit_id as `Name.ref` (the normal buf.lock state), ServeGraph must resolve it to the real git SHA and return the pinned commit — never forward the cid to the upstream (422/502) and never serve a differently-cached commit (HEAD). Ports the Phase 18 commitUUIDInverse+prefix technique into ServeGraph, adds a cid→full-sha map populated at every mint site, gates infoCache hits on cid match, and makes ServeDownload's foreign-cid path prefer cid→sha over the owner/module infoCache.
+**Requirements**: PR-24-1..6 (see 24-01-PLAN.md)
+**Depends on:** Phase 22 (shares the UUID-resolution helper lineage)
+**Plans:** 1/1 plans complete
+Plans:
+
+- [ ] [24-01](./phases/24-resolve-buf-cid-ref-in-servegraph-honor-pinned-commit/24-01-PLAN.md) — cid→sha map + ServeGraph UUID branch + infoCache cid-gating + ServeDownload cid→sha preference; turn the two RED confirming tests GREEN and extend the Phase 21 e2e gate to assert pinned-commit (not HEAD) content
+
+### Phase 25: Address PR #39 post-merge review findings: fix isConventionalDefaultName regression, fix misleading error wrap in blobs.go (GetRepository→GetFiles), verify v1alpha1 read-path against real GitHub (PR-22-4 live e2e), extract duplicated isSHA to shared provider package, fix dangling proto path in commits_helpers_test.go, eliminate commitLineRE duplication between e2e and testutil, make commitResolver construction-time safer
+
+**Goal:** All 7 post-merge review findings from PR #39 are fixed: no behavioral regression for repos with non-default branches named "main"/"master"/"develop"/"trunk"; duplicated helpers and regex are consolidated; error strings, comments, and construction-time safety are hardened; v1alpha1 e2e gate passes against real GitHub
+**Requirements**: FIX-01, FIX-02, FIX-03, FIX-04, FIX-05, FIX-06, FIX-07
+**Depends on:** Phase 24
+**Plans:** 3 plans
+Plans:
+
+- [ ] [25-01](./phases/25-address-pr-39-post-merge-review-findings-pin-multi-default-b/25-01-PLAN.md) — Remove isConventionalDefaultName carve-out from both providers (FIX-01) + extract isSHA to internal/providers/content (FIX-04)
+- [ ] [25-02](./phases/25-address-pr-39-post-merge-review-findings-pin-multi-default-b/25-02-PLAN.md) — Fix error wrap string in blobs.go (FIX-02), fix dangling proto path in test comment (FIX-05), add initCommitResolver panic guard to api.go (FIX-07)
+- [ ] [25-03](./phases/25-address-pr-39-post-merge-review-findings-pin-multi-default-b/25-03-PLAN.md) — Extract commitLineRE/ExtractCommitFromLock to testutil (FIX-06); human-verify v1alpha1 e2e gate against real GitHub (FIX-03)
+
 ---
 
-*Roadmap last updated: 2026-07-08*
+*Roadmap last updated: 2026-07-10*

.planning/STATE.md

@@ -2,15 +2,15 @@
 gsd_state_version: 1.0
 milestone: v1.3
 milestone_name: Diagnostic Logging — In Progress
-status: "Phase 22 shipped — PR #39"
-last_updated: "2026-07-08T13:36:01.102Z"
-last_activity: 2026-07-08
+status: executing
+last_updated: "2026-07-10T08:20:00Z"
+last_activity: 2026-07-10 -- Phase 25 shipped (PR #40)
 progress:
-  total_phases: 12
-  completed_phases: 11
-  total_plans: 15
-  completed_plans: 15
-  percent: 92
+  total_phases: 15
+  completed_phases: 15
+  total_plans: 23
+  completed_plans: 23
+  percent: 100
 ---
 
 # Project State
@@ -21,37 +21,28 @@ See: .planning/PROJECT.md (updated 2026-05-10)
 
 **Core value:** The proxy must correctly serve both old (v1.30.1) and modern (v1.69.0+) Buf CLI clients simultaneously
 
-**Current focus:** Milestone complete
+**Current focus:** Phase 25 complete — all 7 PR #39 post-merge review findings addressed
 
 ## Current Position
 
-Phase: 22
-Plan: Not started
-Status: Phase 22 shipped — PR #39
-Last activity: 2026-07-08
+Phase: 25 (address-pr-39-post-merge-review-findings-pin-multi-default-b) — COMPLETE
+Plan: 3 of 3
+Status: Complete
+Last activity: 2026-07-10 -- Phase 25 execution complete
 
 Progress: [██████████] 100%
 
 ## Performance Metrics
 
 **Velocity:**
 
-- Total plans completed: 6 (this milestone)
+- Total plans completed: 23 (this milestone)
 - Average duration: ~10 min
-- Total execution time: ~50 min
-
-**By Phase:**
-
-| Phase | Plans | Total | Avg/Plan |
-|-------|-------|-------|----------|
-| 16    | 3     | -     | -        |
-| 17    | 1     | -     | 8 min    |
-| 18    | 1     | -     | 18 min   |
-| 22 | 1 | - | - |
+- Total execution time: ~3h 50min
 
 **Recent Trend:**
 
-- (milestone just started)
+- Phase 25: 3 plans, 22 min total, ~7 min/plan
 
 ## Accumulated Context
 
@@ -61,10 +52,7 @@ Decision log maintained in PROJECT.md Key Decisions table.
 
 Recent decisions affecting current work:
 
-- [Roadmap]: 5 phases for v1.3, numbered 11-15 (continuing from v1.2)
-- [Roadmap]: Phase ordering follows dependency chain — Foundation before Infrastructure before handler logging
-- [Roadmap]: OPS-01 (panic recovery) placed in its own phase since it's a distinct infrastructure concern with no handler-level dependency
-- [Roadmap]: Phase 16 (Commit ID Resolution Improvements) added 2026-07-06 — three items bundled into one phase: drop SHA-256 derivation in favor of first-16-bytes of git SHA (incl. short-sha support), probe all configured repos on cache miss, clearer not-found error response and log
+- [Phase 25]: The isConventionalDefaultName carve-out was RETAINED (not removed) because the v1alpha1 ResolveService handler (modulepins.go GetModulePins) is NOT gated by parseResourceRefName and still passes label_name="main" (proto field 3) as commit="main" to GetMeta. The carve-out now uses the shared content.IsConventionalDefaultName helper extracted to internal/providers/content/helpers.go.
 
 ### Pending Todos
 
@@ -74,20 +62,6 @@ None yet.
 
 None yet.
 
-### Roadmap Evolution
-
-- Phase 16 added: предлагаю изменения — use first 16 bytes of git commit id, probe all repos on miss, fix unclear not-found error message
-- Phase 17 added: Fix PR #37 review findings — address pre-merge issues from Phase 16 PR: re-route digest errors through `logHandlerError`/`upstreamError` (not `internalError`), remove `internalError` helper that bypassed `ERR-05`, accept SHA-256 Bitbucket commits (regression at commits_helpers.go:39), move `preResolveForTest` to a `_test.go` file
-- Phase 18 added: respect buf.yaml dependency refs (not always HEAD); fix related bug; remove prewarm logic now that buf id → git id is derivable
-- Phase 18 planned: 1 plan (18-01-PLAN.md) with 3 tasks — (1) honor `Name.ref` end-to-end + add `isSHA`/`isUUID`/`commitUUIDInverse` helpers + provider ref-resolution; (2) add `commitUUIDInverse` + table-driven tests; (3) delete `prewarmHeads`/`registerResolved`/`PrewarmConfig` and rewrite `probeCommitID` to use the inverse for 32-char UUID inputs. See `.planning/phases/18-respect-buf-yaml-dependency-refs-not-always-head-fix-related/18-{RESEARCH,01-PLAN,VALIDATION}.md`
-- Phase 19 added: e2e tests for the `ref` specified for dependency — `looks like it does not work`. Phase 18 introduced end-to-end honoring of `Name.ref` in `buf.yaml` dependencies (plus `commitUUIDInverse` for 32-char UUID inputs); need a real-server e2e test that exercises the path where the `ref` resolves through buf cache/git to prove it works (or surfaces the bug). See `e2e/ref_test.go` and `.planning/phases/19-we-need-e2e-tests-for-the-ref-specified-for-dependency-looks/`.
-- Phase 19 executed: e2e tests committed in `e15927b feat(19-01): add e2e tests for ref-honoring in buf.yaml deps`. Tests pass token-less (skip cleanly). With `EASYP_GH_TOKEN` set, tests revealed 2 real proxy regressions: (a) v1.30.1 no-ref path tries to resolve `"main"` as a ref → GitHub 422 (also breaks the pre-existing `TestSmokeBufModUpdate`); (b) v1.69.0 ref-pinned run returns HEAD's SHA instead of the ref's SHA on both `buf mod update` and `buf dep update`. The e2e tests successfully caught the regression; the proxy fixes are deferred to Phase 20.
-- Phase 20 added: Fix ref-honoring regressions discovered in Phase 19 — restore the no-ref (HEAD) path for buf v1.30.1 and fix the v1beta1 path where the ref is being ignored on `buf dep update`. See `.planning/phases/20-fix-the-problem-with-refs-discovered-on-phase-19/`.
-- Phase 21 added: e2e test for `buf generate` with `buf.lock` pointing to a valid-but-not-latest commit. This is a valid situation (the lock is intentionally pinned to an older commit) and should work with both v1 and v2 buf CLIs. See `.planning/phases/21-we-need-another-e2e-test-we-are-doing-buf-generate-with-buf-`.
-- Phase 21 executed: `TestGenerateWithPinnedBufLock` committed in `8df1f54 test(21-01): add TestGenerateWithPinnedBufLock matrix test`. With `EASYP_GH_TOKEN` set, the test caught two issues: (a) a deprecated `remote:` field in the plan's buf.gen.yaml — fixed in `e0c79b1 fix(21-01): use 'plugin:' not 'remote:' in generated buf.gen.yaml` (the alpha-remote-generation API was removed in v1.69.0+ and is deprecated in v1.30.1); (b) a real proxy regression in the v1.30.1 v1alpha1 `DownloadManifestAndBlobs` read-path: the proxy passes 32-char buf UUIDs directly to GitHub's `/git/trees/<id>` API which 404s, because the v1alpha1 `Download` chain does not apply `commitUUIDInverse` (only the v1beta1 path from Phase 18 does). The v1.69.0 subtest hits a persistent TLS handshake timeout fetching HEAD's tree from `raw.githubusercontent.com` (and is not actually testing the pinned-UUID path — the v1.69.0 client ignores the `buf.lock` for `buf generate` and just asks the proxy for HEAD). The v1.30.1 fix is deferred to a follow-up phase (proposed `22-fix-v1alpha1-download-uuid-handling`); the v1.69.0 subtest redesign is deferred to another follow-up.
-- Phase 22 proposed: Fix the v1.30.1 v1alpha1 read-path to apply `commitUUIDInverse` on 32-char buf-issued UUIDs in the `DownloadManifestAndBlobs` handler chain, then re-run `TestGenerateWithPinnedBufLock` to confirm both subtests pass. This is the same class of bug Phase 18 fixed for the v1beta1 path; Phase 19/20 e2e tests only exercised the v1beta1 path.
-- Phase 22 added: Fix v1.30.1 v1alpha1 read-path UUID handling; verify v1 protocol works. Depends on Phase 21. Scope: (1) apply `commitUUIDInverse` in the v1alpha1 `DownloadManifestAndBlobs` handler chain so 32-char buf UUIDs resolve to git SHAs before hitting GitHub's tree API; (2) re-run `TestGenerateWithPinnedBufLock` to confirm both subtests pass; (3) broader verification that the v1 (v1alpha1) protocol path works end-to-end (not just `buf generate` — also `buf mod update` and the existing smoke test that Phase 19 revealed was broken for v1.30.1). See `.planning/phases/22-fix-v1-30-1-v1alpha1-read-path-uuid-handling-verify-v1-proto/`.
-
 ## Deferred Items
 
 Items acknowledged and carried forward from previous milestone close:
@@ -97,9 +71,10 @@ Items acknowledged and carried forward from previous milestone close:
 | UAT | Phase 03 human UAT (1 pending smoke test) | From v1.1 | 2026-05-07 |
 | Verification | Phase 05 human verification (E2E with GitHub token) | From v1.1 | 2026-05-07 |
 | v2 features | Performance, new endpoints | Future milestone | 2026-05-10 |
+| E2E | TestGenerateWithPinnedBufLock/v1.30.1 blocked by 403 from buf.build remote plugin registry (not a proxy regression) | Phase 25 | 2026-07-10 |
 
 ## Session Continuity
 
-Last session: 2026-07-08T10:47:11Z
-Stopped at: Phase 21 executed (5 commits: tasks 1-2 + initial summary + buf.gen.yaml fix + live-test findings update). Status: verifying. Live test with EASYP_GH_TOKEN caught a real proxy regression in v1.30.1 v1alpha1 read-path (UUID passed to GitHub tree API without commitUUIDInverse). Proposed follow-up Phase 22 to fix the v1alpha1 Download chain. Phase 22 not yet added to ROADMAP.
-Resume file: `.planning/phases/21-we-need-another-e2e-test-we-are-doing-buf-generate-with-buf-/21-01-SUMMARY.md`
+Last session: 2026-07-10
+Stopped at: Phase 25 complete — all 7 FIX items addressed.
+Resume file: None

.planning/agent-history.json

@@ -0,0 +1 @@
+{"version":"1.0","max_entries":50,"entries":[]}

.planning/config.json

@@ -8,7 +8,8 @@
     "plan_check": true,
     "verifier": true,
     "auto_advance": false,
-    "nyquist_validation": true
+    "nyquist_validation": true,
+    "_auto_chain_active": false
   },
   "resolve_model_ids": "omit",
   "git": {