Fix SVN discovery never detecting package changes (#6)

* Fix SVN discovery never detecting package changes

SVN discovery parsed plugin/theme slugs from the HTML listing but never
extracted timestamps (LastCommitted was always nil). After the initial
sync set last_synced_at on all packages, the update query
`last_committed > last_synced_at` could never match — so no package was
ever re-synced, resulting in perpetual "Changed: 0" builds with stale
metadata.

Fix by tracking SVN revision numbers and using the DAV REPORT endpoint
to fetch the changelog between runs:

1. Parse the current SVN revision from the listing page header
2. Store last-seen revision per type in a new site_meta table
3. On each run, fetch the SVN log (REPORT) for revisions since last run
4. Extract changed slugs from the log and mark their last_committed
5. The update step then picks up only packages that actually changed

Also includes:
- Backfill migration setting last_committed = last_synced_at for
  existing rows with NULL last_committed
- 600s timeout for DAV REPORT requests (catch-up runs can be large)
- Skip changelog marking when --limit is set (test/partial runs)
- Error on malformed stored revision instead of silent fallback

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix gofmt struct field alignment in svn.go

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: retlehs

cmd/wpcomposer/cmd/discover.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"context"
 	"fmt"
+	"strconv"
 	"sync/atomic"
 
 	"github.com/spf13/cobra"
@@ -119,14 +120,23 @@ func discoverFromSVN(ctx context.Context, pkgType string, limit int) error {
 	type svnSource struct {
 		url     string
 		pkgType string
+		metaKey string // site_meta key for storing last-seen SVN revision
 	}
 
 	var sources []svnSource
 	if pkgType == "all" || pkgType == "plugin" {
-		sources = append(sources, svnSource{url: "https://plugins.svn.wordpress.org/", pkgType: "plugin"})
+		sources = append(sources, svnSource{
+			url:     "https://plugins.svn.wordpress.org/",
+			pkgType: "plugin",
+			metaKey: "svn_revision_plugin",
+		})
 	}
 	if pkgType == "all" || pkgType == "theme" {
-		sources = append(sources, svnSource{url: "https://themes.svn.wordpress.org/", pkgType: "theme"})
+		sources = append(sources, svnSource{
+			url:     "https://themes.svn.wordpress.org/",
+			pkgType: "theme",
+			metaKey: "svn_revision_theme",
+		})
 	}
 
 	var totalCount, totalFailed int
@@ -153,7 +163,7 @@ func discoverFromSVN(ctx context.Context, pkgType string, limit int) error {
 			batch = batch[:0]
 		}
 
-		err := client.ParseSVNListing(ctx, src.url, func(entry wporg.SVNEntry) error {
+		result, err := client.ParseSVNListing(ctx, src.url, func(entry wporg.SVNEntry) error {
 			if limit > 0 && totalCount >= limit {
 				return errLimitReached
 			}
@@ -178,6 +188,16 @@ func discoverFromSVN(ctx context.Context, pkgType string, limit int) error {
 			return fmt.Errorf("SVN discovery for %s: %w", src.pkgType, err)
 		}
 
+		// Use SVN revision log to find which packages changed since last run.
+		// Skip when --limit is set (partial/test runs shouldn't mutate global
+		// change state or trigger large update workloads).
+		if limit == 0 && result != nil && result.Revision > 0 {
+			if markErr := markChangedFromSVNLog(ctx, client, src, result.Revision); markErr != nil {
+				application.Logger.Warn("SVN changelog fetch failed, skipping change detection",
+					"type", src.pkgType, "error", markErr)
+			}
+		}
+
 		totalFailed += failed
 		application.Logger.Info("SVN discovery done", "type", src.pkgType, "succeeded", count, "failed", failed)
 
@@ -196,6 +216,58 @@ func discoverFromSVN(ctx context.Context, pkgType string, limit int) error {
 	return nil
 }
 
+// markChangedFromSVNLog fetches the SVN log between the last-seen revision and
+// the current revision, extracts which slugs changed, and marks them in the DB
+// so they'll be picked up by the update step.
+func markChangedFromSVNLog(ctx context.Context, client *wporg.Client, src struct {
+	url     string
+	pkgType string
+	metaKey string
+}, currentRev int64) error {
+	lastRevStr, err := packages.GetMeta(ctx, application.DB, src.metaKey)
+	if err != nil {
+		return fmt.Errorf("reading last revision: %w", err)
+	}
+
+	var lastRev int64
+	if lastRevStr != "" {
+		var parseErr error
+		lastRev, parseErr = strconv.ParseInt(lastRevStr, 10, 64)
+		if parseErr != nil {
+			return fmt.Errorf("malformed stored revision %q for %s: %w", lastRevStr, src.metaKey, parseErr)
+		}
+	}
+
+	if lastRev > 0 && lastRev < currentRev {
+		application.Logger.Info("fetching SVN changelog",
+			"type", src.pkgType, "from_rev", lastRev, "to_rev", currentRev)
+
+		slugs, err := client.FetchSVNChangedSlugs(ctx, src.url, lastRev+1, currentRev)
+		if err != nil {
+			return err
+		}
+
+		if len(slugs) > 0 {
+			affected, err := packages.MarkPackagesChanged(ctx, application.DB, src.pkgType, slugs)
+			if err != nil {
+				return fmt.Errorf("marking changed packages: %w", err)
+			}
+			application.Logger.Info("marked changed packages from SVN log",
+				"type", src.pkgType, "slugs_in_log", len(slugs), "packages_marked", affected)
+		}
+	} else if lastRev == 0 {
+		application.Logger.Info("no previous SVN revision stored, skipping changelog (first run)",
+			"type", src.pkgType, "current_rev", currentRev)
+	}
+
+	// Store current revision for next run.
+	if err := packages.SetMeta(ctx, application.DB, src.metaKey, strconv.FormatInt(currentRev, 10)); err != nil {
+		return fmt.Errorf("storing revision: %w", err)
+	}
+
+	return nil
+}
+
 var errLimitReached = fmt.Errorf("limit reached")
 
 func init() {

internal/packages/package.go

@@ -356,6 +356,48 @@ func RefreshSiteStats(ctx context.Context, db *sql.DB) error {
 	return nil
 }
 
+// MarkPackagesChanged sets last_committed = now for the given slugs of a
+// specific type, so they'll be picked up by GetPackagesNeedingUpdate.
+func MarkPackagesChanged(ctx context.Context, db *sql.DB, pkgType string, slugs []string) (int64, error) {
+	if len(slugs) == 0 {
+		return 0, nil
+	}
+
+	now := time.Now().UTC().Format(time.RFC3339)
+
+	tx, err := db.BeginTx(ctx, nil)
+	if err != nil {
+		return 0, fmt.Errorf("beginning transaction: %w", err)
+	}
+	defer func() { _ = tx.Rollback() }()
+
+	stmt, err := tx.PrepareContext(ctx, `
+		UPDATE packages
+		SET last_committed = ?, provider_group = ?, updated_at = ?
+		WHERE type = ? AND name = ? AND is_active = 1`)
+	if err != nil {
+		return 0, fmt.Errorf("preparing statement: %w", err)
+	}
+	defer func() { _ = stmt.Close() }()
+
+	t, _ := time.Parse(time.RFC3339, now)
+	pg := ComputeProviderGroup(&t)
+	var affected int64
+	for _, slug := range slugs {
+		res, err := stmt.ExecContext(ctx, now, pg, now, pkgType, slug)
+		if err != nil {
+			return affected, fmt.Errorf("marking package %s/%s changed: %w", pkgType, slug, err)
+		}
+		n, _ := res.RowsAffected()
+		affected += n
+	}
+
+	if err := tx.Commit(); err != nil {
+		return 0, fmt.Errorf("committing: %w", err)
+	}
+	return affected, nil
+}
+
 func boolToInt(b bool) int {
 	if b {
 		return 1

internal/packages/site_meta.go

@@ -0,0 +1,32 @@
+package packages
+
+import (
+	"context"
+	"database/sql"
+	"fmt"
+)
+
+// GetMeta retrieves a value from site_meta by key. Returns "" if not found.
+func GetMeta(ctx context.Context, db *sql.DB, key string) (string, error) {
+	var value string
+	err := db.QueryRowContext(ctx, `SELECT value FROM site_meta WHERE key = ?`, key).Scan(&value)
+	if err == sql.ErrNoRows {
+		return "", nil
+	}
+	if err != nil {
+		return "", fmt.Errorf("getting site_meta %q: %w", key, err)
+	}
+	return value, nil
+}
+
+// SetMeta inserts or updates a value in site_meta.
+func SetMeta(ctx context.Context, db *sql.DB, key, value string) error {
+	_, err := db.ExecContext(ctx, `
+		INSERT INTO site_meta (key, value) VALUES (?, ?)
+		ON CONFLICT(key) DO UPDATE SET value = excluded.value`,
+		key, value)
+	if err != nil {
+		return fmt.Errorf("setting site_meta %q: %w", key, err)
+	}
+	return nil
+}