feat(connect): Phase 16 — commit-id resolution improvements (first-16-bytes-of-SHA format)

.planning/PROJECT.md

@@ -85,4 +85,4 @@ The proxy must correctly serve both old (v1.30.1) and modern (v1.69.0+) Buf CLI
 This document evolves at phase transitions and milestone boundaries.
 
 ---
-*Last updated: 2026-06-16 after v1.3 milestone start*
+*Last updated: 2026-07-06 after v1.3 milestone completion (Phases 11–16, including Phase 16 commit-id format cutover)*

.planning/ROADMAP.md

@@ -39,67 +39,115 @@
 - [ ] **Phase 13: Error Path Logging** — Structured error context on all v1beta1/v1 handler failures
 - [ ] **Phase 14: Provider Logging** — Debug-level tracing for GitHub provider and Artifactory cache operations
 - [ ] **Phase 15: Operational Logging** — Panic recovery middleware with full stack trace
+- [x] **Phase 16: Commit ID Resolution Improvements** — Use first 16 bytes of git SHA as commit id (incl. short-sha support), probe all configured repos on cache miss, clearer not-found error response and log (completed 2026-07-06)
+- [x] **Phase 17: Fix PR #37 review findings** — Address pre-merge review findings from PR #37 (Phase 16) so the commit-id format cutover lands without undoing the v1.3 logging-quality work or breaking Bitbucket SHA-256 repositories (completed 2026-07-07)
 
 ## Phase Details
 
 ### Phase 11: Logging Foundation
+
 **Goal**: Operators can configure log level, format, and source info, with centralized sensitive-data redaction applied to all log output
 **Depends on**: Nothing (foundation phase)
 **Requirements**: FOUND-01, FOUND-02, FOUND-03, FOUND-04
 **Success Criteria** (what must be TRUE):
+
   1. Setting `EASYP_LOG_LEVEL=debug` produces debug-level log lines; default (no env var) logs at info level
   2. Sensitive fields (tokens, passwords) are automatically redacted from every log entry via `slog.HandlerOptions.ReplaceAttr` — no sensitive data appears in any output
   3. Setting `EASYP_LOG_FORMAT=json` produces JSON-formatted log output; default is human-readable text
   4. Enabling `AddSource` in config includes source file and line number in log entries
   5. Invalid log level values produce a clear error message at startup and exit gracefully
+
 **Plans**: TBD
 
 ### Phase 12: Logging Infrastructure
+
 **Goal**: Every request is traceable via correlation ID, and v1alpha1 handlers are instrumented via a single Connect RPC unary interceptor
 **Depends on**: Phase 11
 **Requirements**: INFR-01, INFR-02, INFR-03
 **Success Criteria** (what must be TRUE):
+
   1. Every log line in the request lifecycle includes a `request_id` (either from `X-Request-Id` header or auto-generated 8-byte hex)
   2. Logs from concurrent requests are distinguishable by their unique `request_id`
   3. v1alpha1 handler procedures (blobs, modulepins, bynames) produce structured log entries with procedure, peer, duration, request/response size, and error code via a single unary interceptor — zero handler code changes
   4. HTTP middleware logs timing and status at INFO level only — error-level logging is removed from middleware to prevent double-logging with handler-level logs
   5. Error logs from handler code include the `request_id` linking them to the originating request via context propagation
+
 **Plans**: TBD
 
 ### Phase 13: Error Path Logging — v1beta1/v1 Handlers
+
 **Goal**: Every failure in v1beta1/v1 raw handlers produces a structured log entry with full request context and consistent attribute naming
 **Depends on**: Phase 12
 **Requirements**: ERR-01, ERR-02, ERR-03, ERR-04, ERR-05
 **Success Criteria** (what must be TRUE):
+
   1. `ServeHTTP` (CommitService) failure logs include owner, repo, error, and request_id
   2. `ServeGraph` (GraphService) failure logs include owner, module, error, and request_id
   3. `ServeDownload` (DownloadService) failure logs include owner, module, commit, error, and request_id
   4. `ServeGetModules` (ModuleService) failure logs include owner, module, error, and request_id
   5. All handler-level error logs use consistent attribute names (`protocol`, `owner`, `repo`, `commit`, `request_id`, `error`) and include `protocol: "v1beta1"` — no naming inconsistencies across handlers
+
 **Plans**: TBD
 
 ### Phase 14: Provider Logging
+
 **Goal**: Provider API calls and cache operations are traceable at debug level with timing, status, and provider-type context
 **Depends on**: Phase 12
 **Requirements**: PROV-01, PROV-02
 **Success Criteria** (what must be TRUE):
+
   1. GitHub provider HTTP requests log before and after each API call with redacted URL, method, response status, and duration at debug level
   2. Artifactory cache operations log hit/miss with duration at debug level
   3. Cache error logs distinguish context cancellation (client disconnected) from API errors (upstream failure)
   4. Provider log lines include a `provider_type` attribute (e.g., `github`, `artifactory`) for filtering
+
 **Plans**: TBD
 
 ### Phase 15: Operational Logging — Panic Recovery
+
 **Goal**: Unhandled panics are caught, logged with full stack trace, and return HTTP 500 instead of crashing the process
 **Depends on**: Phase 11
 **Requirements**: OPS-01
 **Success Criteria** (what must be TRUE):
+
   1. A panic anywhere in the request handling chain is caught by recovery middleware wrapping the entire ServeMux
   2. The panic is logged with full stack trace including goroutine information and request context
   3. The client receives an HTTP 500 response instead of a connection reset or process termination
   4. Other concurrent requests continue unaffected when one request panics
+
+**Plans**: TBD
+
+### Phase 16: Commit ID Resolution Improvements
+
+**Goal**: Make commit-id resolution more robust (accept short git SHAs, fall back to upstream probe on cache miss) and the not-found failure mode diagnosable (clear error response and structured log line)
+**Depends on**: Phase 15
+**Requirements**: TBD
+**Success Criteria** (what must be TRUE):
+
+  1. The minted commit id is the first 16 bytes of the git SHA (no SHA-256 derivation), and a unit test verifies that both full 40-char and short (7-char) git SHAs round-trip through `commitUUID` deterministically
+  2. When a `DownloadService/Download` request carries a commit id that is not in `commitMap` and the proxy serves multiple modules, the handler probes every configured source for the sha and uses the first match — single-source deployments keep the existing `resolveForeignCommitID` fast path
+  3. The 400 response returned for an unresolvable commit id names the id itself in both the wire body and the structured log line, so an operator can correlate a client-side "unknown commit id" with a prior `GetCommits` log entry without re-reading the request
+
 **Plans**: TBD
 
+### Phase 17: Fix PR #37 review findings
+
+**Goal:** Address pre-merge review findings from PR #37 (Phase 16) so the commit-id format cutover lands without undoing the v1.3 logging-quality work or breaking Bitbucket SHA-256 repositories
+**Depends on:** Phase 16
+**Requirements**: TBD
+**Success Criteria** (what must be TRUE):
+
+  1. A `computeB4Digest` failure from `GetFiles`, `computeB4DigestFromFiles`, or `commitUUID` is logged via `logHandlerError`/`upstreamError` (not `internalError`) with full context (owner, module, repo, commit, request_id, server, protocol, status) and returns 502 for upstream failures — restores the `ERR-05` contract that the new `internalError` helper bypassed
+  2. The new `internalError` helper is removed; all handler-level 500s flow through the existing `logHandlerError` (commits.go:777) so the structured 5xx log line is joinable on `request_id` and `error_class=internal` is set automatically
+  3. `commitUUID` accepts git SHAs of 40 chars (SHA-1) and 64 chars (SHA-256) — Bitbucket Server on a SHA-256-enabled repo returns 64-char commits and currently 500s on the strict `len != 40` check (commits_helpers.go:39); a unit test covers both lengths
+  4. A new test in `commits_helpers_test.go` (not `_test.go` production file) exercises the SHA-256 path; `preResolveForTest` (commits_helpers.go:60-70) is moved out of the production source so it cannot be reached by future code
+  5. The 400 not-found response message and structured log line for an unresolvable commit id remain generic enough to apply to both stale-lockfile misses and genuine foreign-id misses from other registries
+
+**Plans:** 1 plan
+Plans:
+
+- [x] 17-01-PLAN.md — Atomic 4-task fix: routing `computeB4Digest` errors through the right helpers, removing `internalError`, accepting 64-char SHA-256, softening the 400 message, moving `preResolveForTest` to the test file (covers SC-1 through SC-5)
+
 ## Progress
 
 | Phase | Milestone | Plans Complete | Status | Completed |
@@ -119,6 +167,9 @@
 | 13. Error Path Logging        | v1.3 | 0/0 | Not started | - |
 | 14. Provider Logging          | v1.3 | 0/0 | Not started | - |
 | 15. Operational Logging       | v1.3 | 0/0 | Not started | - |
+| 16. Commit ID Resolution Improvements | v1.3 | 3/3 | Complete    | 2026-07-06 |
+| 17. Fix PR #37 review findings | v1.3 | 1/1 | Complete | 2026-07-07 |
 
 ---
-*Roadmap last updated: 2026-06-16*
+
+*Roadmap last updated: 2026-07-07*

.planning/STATE.md

@@ -1,15 +1,15 @@
 ---
 gsd_state_version: 1.0
 milestone: v1.3
-milestone_name: Diagnostic Logging
-status: planning
-last_updated: "2026-06-18T17:32:59.092Z"
-last_activity: "2026-06-16 — Roadmap created for v1.3 Diagnostic Logging (5 phases: 11-15)"
+milestone_name: Diagnostic Logging — In Progress
+status: shipped
+last_updated: "2026-07-07T10:32:00.000Z"
+last_activity: 2026-07-07 -- Phase 17 shipped (review fixes posted as PR #37 comment)
 progress:
-  total_phases: 5
-  completed_phases: 5
-  total_plans: 5
-  completed_plans: 5
+  total_phases: 7
+  completed_phases: 7
+  total_plans: 9
+  completed_plans: 9
   percent: 100
 ---
 
@@ -21,30 +21,31 @@ 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:** v1.3 Diagnostic Logging — Phase 11 (Logging Foundation)
+**Current focus:** Milestone complete
 
 ## Current Position
 
-Phase: 11 of 15 (Logging Foundation)
-Plan: None yet
-Status: Ready to plan
-Last activity: 2026-06-16 — Roadmap created for v1.3 Diagnostic Logging (5 phases: 11-15)
+Phase: 17
+Plan: 17-01 shipped
+Status: Phase 17 shipped — review fixes posted as comment on PR #37
+Last activity: 2026-07-07 -- Phase 17 shipped (PR #37 comment <https://github.com/easyp-tech/server/pull/37#issuecomment-4901262336>)
 
-Progress: [                    ] 0%
+Progress: [####################] 100%
 
 ## Performance Metrics
 
 **Velocity:**
 
-- Total plans completed: 0 (this milestone)
-- Average duration: N/A
-- Total execution time: N/A
+- Total plans completed: 4 (this milestone)
+- Average duration: ~8 min
+- Total execution time: ~32 min
 
 **By Phase:**
 
 | Phase | Plans | Total | Avg/Plan |
 |-------|-------|-------|----------|
-| (none yet) | 0 | - | - |
+| 16    | 3     | -     | -        |
+| 17    | 1     | -     | 8 min    |
 
 **Recent Trend:**
 
@@ -61,6 +62,7 @@ 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
 
 ### Pending Todos
 
@@ -70,6 +72,11 @@ 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
+
 ## Deferred Items
 
 Items acknowledged and carried forward from previous milestone close:
@@ -82,6 +89,6 @@ Items acknowledged and carried forward from previous milestone close:
 
 ## Session Continuity
 
-Last session: 2026-06-18T17:32:59.075Z
-Stopped at: Phase 11 context gathered
-Resume file: .planning/phases/11-logging-foundation/11-CONTEXT.md
+Last session: 2026-07-07T10:32:00.000Z
+Stopped at: Phase 17 shipped (1/1 plan, 4 tasks, 5 SCs satisfied, 13/13 verified, comment posted on PR #37)
+Resume file: <https://github.com/easyp-tech/server/pull/37#issuecomment-4901262336>