fix: address post-merge review findings from PR #63

.claude/rules/commits-and-prs.md

@@ -44,3 +44,46 @@ All commits must be signed/verified. CI rejects unverified commits.
 3. Branch name follows `type/description` format
 4. If production code changed, CHANGELOG.md is updated in the same commit
 5. All code compiles and tests pass
+
+## Pre-commit hook failures
+
+When a pre-commit hook modifies files (e.g., goimports reformats struct alignment), the commit did NOT happen. To recover:
+
+1. Re-stage the modified files: `git add <files>`
+2. Create a NEW commit (same message is fine)
+3. Do NOT use `--amend` — the previous commit never happened
+
+This is especially common with `goimports` reformatting Go files.
+
+## Resolving PR review threads
+
+When addressing review feedback, both reply AND resolve:
+
+1. **Reply** via REST: `gh api repos/OWNER/REPO/pulls/N/comments -X POST -F body="..." -F in_reply_to=COMMENT_ID`
+2. **Resolve** via GraphQL: `gh api graphql -f query='mutation { resolveReviewThread(input: {threadId: "THREAD_ID"}) { thread { isResolved } } }'`
+
+To get thread IDs, query:
+
+```sh
+gh api graphql -f query='{
+  repository(owner: "sensiblebit", name: "certkit") {
+    pullRequest(number: N) {
+      reviewThreads(first: 20) {
+        nodes { id isResolved comments(first: 1) { nodes { body } } }
+      }
+    }
+  }
+}'
+```
+
+Just replying does NOT mark the thread as resolved in the GitHub UI.
+
+## Merging PRs
+
+1. Wait for **all** CI checks to complete — including non-blocking checks like `claude-review`. Post-merge review comments require follow-up PRs.
+2. Check for new review comments (`gh api repos/.../pulls/N/comments`, `gh api repos/.../issues/N/comments`) before merging.
+3. Use squash merge: `gh pr merge N --squash --delete-branch`
+
+## Changelog refs (CL-3)
+
+Each changelog entry must reference the commit or PR **where the change was made**, not the PR where the bug was found. A follow-up fix PR gets its own ref, even if it addresses findings from an earlier PR.

CHANGELOG.md

@@ -13,10 +13,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add concurrent AIA resolution — fetches up to `Concurrency` URLs in parallel per depth round (default 20, WASM uses 50) ([`392878a`])
 - Add `serial` field to WASM `getState()` certificate data — hex-encoded serial number ([`392878a`])
 - Add paste support to web UI drop zone — Ctrl+V / Cmd+V pastes PEM or certificate text directly without needing a file ([`392878a`])
+- Extract `RunValidation`, `CheckExpiration`, `CheckKeyStrength`, `CheckSignature`, `CheckTrustChain` from WASM into `internal/certstore` — validation policy logic is now testable without WASM build constraints ([#63])
 
 ### Changed
 
 - Replace Inspect/Verify tab navigation with unified category tabs (Leaf, Intermediate, Root, Keys) — certificates are now organized by type with click-to-expand detail rows showing validation checks and metadata ([`392878a`])
+- `RunValidation` checks `ctx.Err()` between Mozilla root pool load and validation checks to honor WASM timeout constraints ([#63])
+
+### Fixed
+
+- Fix AIA `progressTotal` double-counting certs whose issuer fetch fails — the same cert appeared in both `processed` and `queue` sets, inflating the progress bar total ([#64])
 
 ## [0.8.0] - 2026-02-22
 
@@ -629,6 +635,8 @@ Initial release.
 [`55b5c1e`]: https://github.com/sensiblebit/certkit/commit/55b5c1e
 [`8cf81d9`]: https://github.com/sensiblebit/certkit/commit/8cf81d9
 [`3569926`]: https://github.com/sensiblebit/certkit/commit/3569926
+[#64]: https://github.com/sensiblebit/certkit/pull/64
+[#63]: https://github.com/sensiblebit/certkit/pull/63
 [#58]: https://github.com/sensiblebit/certkit/pull/58
 [#57]: https://github.com/sensiblebit/certkit/pull/57
 [#56]: https://github.com/sensiblebit/certkit/pull/56

internal/certstore/aia.go

@@ -101,6 +101,7 @@ func ResolveAIA(ctx context.Context, input ResolveAIAInput) []string {
 
 	progressTotal := 0
 	processed := make(map[string]bool)
+	totalSeen := make(map[string]bool)
 
 	for range maxDepth {
 		var queue []*CertRecord
@@ -114,9 +115,14 @@ func ResolveAIA(ctx context.Context, input ResolveAIAInput) []string {
 			break
 		}
 
-		// Recount progressTotal each round to include newly-discovered
-		// intermediates, preventing completed from exceeding total.
-		progressTotal = len(processed) + len(queue)
+		// Track unique certs that have ever entered the queue. Using a
+		// separate set avoids double-counting certs that persist across
+		// rounds (e.g. when their AIA fetch fails but they still need
+		// resolution).
+		for _, rec := range queue {
+			totalSeen[certID(rec.Cert)] = true
+		}
+		progressTotal = len(totalSeen)
 
 		// Phase 1: Collect unique work items and pre-validate URLs.
 		// Only the main goroutine touches `seen` — no concurrent access.
@@ -146,9 +152,8 @@ func ResolveAIA(ctx context.Context, input ResolveAIAInput) []string {
 		if len(work) == 0 {
 			// All URLs were already seen or rejected — mark certs processed.
 			for _, rec := range queue {
-				ski := fmt.Sprintf("%x", rec.Cert.SubjectKeyId)
-				if !processed[ski] {
-					processed[ski] = true
+				if id := certID(rec.Cert); !processed[id] {
+					processed[id] = true
 					if input.OnProgress != nil {
 						input.OnProgress(len(processed), progressTotal)
 					}
@@ -223,9 +228,8 @@ func ResolveAIA(ctx context.Context, input ResolveAIAInput) []string {
 
 		// Mark all certs in this round as processed and report progress.
 		for _, rec := range queue {
-			ski := fmt.Sprintf("%x", rec.Cert.SubjectKeyId)
-			if !processed[ski] {
-				processed[ski] = true
+			if id := certID(rec.Cert); !processed[id] {
+				processed[id] = true
 				if input.OnProgress != nil {
 					input.OnProgress(len(processed), progressTotal)
 				}

internal/certstore/aia_test.go

@@ -10,6 +10,7 @@ import (
 	"encoding/pem"
 	"fmt"
 	"strings"
+	"sync"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -897,3 +898,157 @@ func TestResolveAIA_RejectsSSRFURL(t *testing.T) {
 		})
 	}
 }
+
+func TestResolveAIA_ProgressNoDuplicateCounting(t *testing.T) {
+	// WHY: When a cert's AIA fetch fails, it remains unresolved and re-enters
+	// the queue on the next depth round. The progress total must not
+	// double-count it — completed should never exceed total.
+	//
+	// Setup: two leaves under different CAs. Leaf A's AIA URL returns a valid
+	// intermediate (fetched > 0, loop continues to round 2). Leaf B's AIA URL
+	// always fails (B re-enters queue in round 2). If totalSeen doesn't
+	// deduplicate, progressTotal inflates on round 2.
+	t.Parallel()
+
+	store := NewMemStore()
+
+	// CA-A: its intermediate will be fetchable.
+	caAKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	caATmpl := &x509.Certificate{
+		SerialNumber:          randomSerial(t),
+		Subject:               pkix.Name{CommonName: "Progress CA-A"},
+		NotBefore:             time.Now().Add(-time.Hour),
+		NotAfter:              time.Now().Add(24 * time.Hour),
+		IsCA:                  true,
+		BasicConstraintsValid: true,
+		KeyUsage:              x509.KeyUsageCertSign,
+		SubjectKeyId:          []byte{1, 2, 3, 4},
+	}
+	caADER, err := x509.CreateCertificate(rand.Reader, caATmpl, caATmpl, &caAKey.PublicKey, caAKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	caACert, err := x509.ParseCertificate(caADER)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// CA-B: its intermediate will NOT be fetchable.
+	caBKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	caBTmpl := &x509.Certificate{
+		SerialNumber:          randomSerial(t),
+		Subject:               pkix.Name{CommonName: "Progress CA-B"},
+		NotBefore:             time.Now().Add(-time.Hour),
+		NotAfter:              time.Now().Add(24 * time.Hour),
+		IsCA:                  true,
+		BasicConstraintsValid: true,
+		KeyUsage:              x509.KeyUsageCertSign,
+		SubjectKeyId:          []byte{5, 6, 7, 8},
+	}
+	caBDER, err := x509.CreateCertificate(rand.Reader, caBTmpl, caBTmpl, &caBKey.PublicKey, caBKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	caBCert, err := x509.ParseCertificate(caBDER)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Leaf A: AIA URL will succeed.
+	leafAKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	leafATmpl := &x509.Certificate{
+		SerialNumber:          randomSerial(t),
+		Subject:               pkix.Name{CommonName: "leaf-a.example.com"},
+		NotBefore:             time.Now().Add(-time.Hour),
+		NotAfter:              time.Now().Add(24 * time.Hour),
+		IssuingCertificateURL: []string{"http://example.com/ca-a.cer"},
+	}
+	leafADER, err := x509.CreateCertificate(rand.Reader, leafATmpl, caACert, &leafAKey.PublicKey, caAKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	leafACert, err := x509.ParseCertificate(leafADER)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Leaf B: AIA URL will always fail.
+	leafBKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	leafBTmpl := &x509.Certificate{
+		SerialNumber:          randomSerial(t),
+		Subject:               pkix.Name{CommonName: "leaf-b.example.com"},
+		NotBefore:             time.Now().Add(-time.Hour),
+		NotAfter:              time.Now().Add(24 * time.Hour),
+		IssuingCertificateURL: []string{"http://example.com/ca-b.cer"},
+	}
+	leafBDER, err := x509.CreateCertificate(rand.Reader, leafBTmpl, caBCert, &leafBKey.PublicKey, caBKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	leafBCert, err := x509.ParseCertificate(leafBDER)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if err := store.HandleCertificate(leafACert, "leaf-a.pem"); err != nil {
+		t.Fatal(err)
+	}
+	if err := store.HandleCertificate(leafBCert, "leaf-b.pem"); err != nil {
+		t.Fatal(err)
+	}
+
+	// Fetcher: leaf A's URL returns CA-A (success), leaf B's URL always fails.
+	fetcher := func(_ context.Context, url string) ([]byte, error) {
+		if url == "http://example.com/ca-a.cer" {
+			return caADER, nil
+		}
+		return nil, fmt.Errorf("connection refused")
+	}
+
+	var mu sync.Mutex
+	var ticks []struct{ completed, total int }
+	onProgress := func(completed, total int) {
+		mu.Lock()
+		ticks = append(ticks, struct{ completed, total int }{completed, total})
+		mu.Unlock()
+	}
+
+	ResolveAIA(context.Background(), ResolveAIAInput{
+		Store:      store,
+		Fetch:      fetcher,
+		OnProgress: onProgress,
+	})
+
+	mu.Lock()
+	defer mu.Unlock()
+
+	if len(ticks) == 0 {
+		t.Fatal("expected progress ticks, got none")
+	}
+
+	// Total must stay stable: no new certs are discovered (CA-A is a
+	// self-signed root, not an intermediate), so total should be 2
+	// (leaf A + leaf B) in every tick. The old buggy formula inflated
+	// total from 2→3 in round 2 when leaf B re-entered the queue.
+	expectedTotal := 2
+	for i, tick := range ticks {
+		if tick.completed > tick.total {
+			t.Errorf("tick %d: completed (%d) > total (%d)", i, tick.completed, tick.total)
+		}
+		if tick.total != expectedTotal {
+			t.Errorf("tick %d: total = %d, want %d (inflated by double-counting)", i, tick.total, expectedTotal)
+		}
+	}
+}