fix: address pre-push reviewer feedback for Plan C

- Add --webhook-secret flag to repos add and repos update (blocker)
- Remove stray fmt.Println from webhook test
- Clarify plan/apply/update help text, mention commands in repos Long
- Sanitize last_sync_error for pull/discover failure paths

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

Author: michaelmcnees

packages/engine/internal/cli/repos.go

@@ -23,9 +23,11 @@ func newReposCommand() *cobra.Command {
 		Short: "Manage GitOps workflow source repositories",
 		Long: `Registers GitOps source repositories whose workflow YAML definitions are
 synced into this Mantle instance. This command manages the registry; use
-` + "`mantle repos sync`" + ` for on-demand syncs, or start ` + "`mantle serve`" + ` to let the
-background poller run. Auth material is stored in a "git" credential type
-(` + "`mantle secrets create --type git`" + `) and referenced here by name.`,
+` + "`mantle repos sync`" + ` for ad-hoc syncs, ` + "`mantle repos plan`" + ` to preview changes
+without writing, ` + "`mantle repos apply`" + ` for manual control over auto_apply:false
+repos, or start ` + "`mantle serve`" + ` to let the background poller run. Auth material
+is stored in a "git" credential type (` + "`mantle secrets create --type git`" + `) and
+referenced here by name.`,
 	}
 	cmd.AddCommand(newReposAddCommand())
 	cmd.AddCommand(newReposUpdateCommand())
@@ -54,7 +56,7 @@ func newRepoStore(cmd *cobra.Command) (*repo.Store, func(), error) {
 }
 
 func newReposAddCommand() *cobra.Command {
-	var url, branch, path, pollInterval, credential string
+	var url, branch, path, pollInterval, credential, webhookSecret string
 	var autoApply, prune bool
 
 	cmd := &cobra.Command{
@@ -73,14 +75,15 @@ credential must already exist and be of type "git".`,
 			defer cleanup()
 
 			r, err := store.Create(cmd.Context(), repo.CreateParams{
-				Name:         args[0],
-				URL:          url,
-				Branch:       branch,
-				Path:         path,
-				PollInterval: pollInterval,
-				Credential:   credential,
-				AutoApply:    autoApply,
-				Prune:        prune,
+				Name:          args[0],
+				URL:           url,
+				Branch:        branch,
+				Path:          path,
+				PollInterval:  pollInterval,
+				Credential:    credential,
+				AutoApply:     autoApply,
+				Prune:         prune,
+				WebhookSecret: webhookSecret,
 			})
 			if err != nil {
 				return err
@@ -97,6 +100,8 @@ credential must already exist and be of type "git".`,
 	cmd.Flags().StringVar(&credential, "credential", "", "Git credential name (required)")
 	cmd.Flags().BoolVar(&autoApply, "auto-apply", true, "Automatically apply changes (false = plan-only)")
 	cmd.Flags().BoolVar(&prune, "prune", true, "When true, workflows deleted from the repo are disabled in Mantle")
+	cmd.Flags().StringVar(&webhookSecret, "webhook-secret", "",
+		"HMAC secret for push webhook verification (empty = no verification)")
 	_ = cmd.MarkFlagRequired("url")
 	_ = cmd.MarkFlagRequired("credential")
 
@@ -222,15 +227,17 @@ func newReposListCommand() *cobra.Command {
 }
 
 func newReposUpdateCommand() *cobra.Command {
-	var branch, path, pollInterval, credential string
+	var branch, path, pollInterval, credential, webhookSecret string
 	var autoApply, prune, enabled bool
 
 	cmd := &cobra.Command{
 		Use:   "update <name>",
 		Short: "Update a registered repo's mutable fields",
 		Long: `Replaces the branch, path, poll-interval, credential, auto-apply, prune,
 and enabled flags on an existing repo. URL and name are immutable — to
-change them, delete and recreate.`,
+change them, delete and recreate.
+All flags must be provided — update replaces the full set of mutable fields.
+Omitting a flag resets that field to its default value.`,
 		Args: cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			store, cleanup, err := newRepoStore(cmd)
@@ -242,6 +249,7 @@ change them, delete and recreate.`,
 			r, err := store.Update(cmd.Context(), args[0], repo.UpdateParams{
 				Branch: branch, Path: path, PollInterval: pollInterval,
 				Credential: credential, AutoApply: autoApply, Prune: prune, Enabled: enabled,
+				WebhookSecret: webhookSecret,
 			})
 			if err != nil {
 				return err
@@ -257,6 +265,8 @@ change them, delete and recreate.`,
 	cmd.Flags().BoolVar(&autoApply, "auto-apply", true, "Automatically apply changes")
 	cmd.Flags().BoolVar(&prune, "prune", true, "Disable workflows deleted from the repo")
 	cmd.Flags().BoolVar(&enabled, "enabled", true, "Whether the repo is active")
+	cmd.Flags().StringVar(&webhookSecret, "webhook-secret", "",
+		"HMAC secret for push webhook verification (empty = no verification)")
 	_ = cmd.MarkFlagRequired("credential")
 	return cmd
 }
@@ -308,7 +318,7 @@ func newReposPlanCommand() *cobra.Command {
 		Short: "Dry-run: show what a sync would change",
 		Long: `Plans a sync without writing anything: pulls the repo, discovers files,
 and classifies each as "would apply" (new or changed) or "unchanged". Use
-before mantle repos apply to verify pending changes on auto_apply:false repos.`,
+before mantle repos apply to preview pending changes before they land.`,
 		Args: cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			store, cleanup, err := newRepoStore(cmd)
@@ -343,10 +353,11 @@ func newReposApplyCommand() *cobra.Command {
 	var fromDir string
 	cmd := &cobra.Command{
 		Use:   "apply <name>",
-		Short: "Manually sync a repo (for auto_apply:false)",
-		Long: `Runs one sync pass for the named repo. Works regardless of auto_apply
-setting — useful when auto_apply is false and you want explicit control
-over when workflow YAML lands in Mantle.`,
+		Short: "Apply pending workflow changes from a repo",
+		Long: `Runs one sync pass for the named repo. Useful when auto_apply is false and you
+want explicit control over when workflow YAML lands in Mantle. Also works for
+auto_apply:true repos when you want to apply immediately without waiting for
+the next poll.`,
 		Args: cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			store, cleanup, err := newRepoStore(cmd)

packages/engine/internal/cli/repos_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/dvflw/mantle/internal/config"
 	"github.com/dvflw/mantle/internal/db"
 	"github.com/dvflw/mantle/internal/dbdefaults"
+	"github.com/dvflw/mantle/internal/repo"
 	"github.com/testcontainers/testcontainers-go"
 	"github.com/testcontainers/testcontainers-go/modules/postgres"
 	"github.com/testcontainers/testcontainers-go/wait"
@@ -76,6 +77,45 @@ func TestReposAdd_PersistsRepo(t *testing.T) {
 	}
 }
 
+func TestReposAdd_WebhookSecretStoredNotPrinted(t *testing.T) {
+	ctx, cfg := reposCtx(t)
+	root := NewRootCommand()
+	var stdout, stderr bytes.Buffer
+	root.SetOut(&stdout)
+	root.SetErr(&stderr)
+	root.SetArgs([]string{"repos", "add", "acme-webhook",
+		"--url", "https://github.com/acme/workflows.git",
+		"--credential", "github-pat",
+		"--webhook-secret", "topsecret",
+		"--database-url", cfg.Database.URL,
+	})
+	if err := root.Execute(); err != nil {
+		t.Fatalf("Execute: %v\nstderr: %s", err, stderr.String())
+	}
+	out := stdout.String()
+	// Secret must NOT appear in CLI output.
+	if strings.Contains(out, "topsecret") {
+		t.Errorf("webhook secret leaked to stdout: %q", out)
+	}
+	if !strings.Contains(out, "Added repo \"acme-webhook\"") {
+		t.Errorf("unexpected stdout: %q", out)
+	}
+	// Verify the secret was actually stored by reading it back via store.
+	conn, err := db.Open(cfg.Database)
+	if err != nil {
+		t.Fatalf("db.Open: %v", err)
+	}
+	defer conn.Close()
+	store := &repo.Store{DB: conn, Actor: "test"}
+	got, err := store.Get(ctx, "acme-webhook")
+	if err != nil {
+		t.Fatalf("Get: %v", err)
+	}
+	if got.WebhookSecret != "topsecret" {
+		t.Errorf("WebhookSecret: got %q, want %q", got.WebhookSecret, "topsecret")
+	}
+}
+
 func TestReposList_ShowsRegisteredRepos(t *testing.T) {
 	_, cfg := reposCtx(t)
 	// Seed one row by calling the add command.

packages/engine/internal/repo/store.go

@@ -38,14 +38,15 @@ var ErrNotFound = errors.New("repo not found")
 // Fields with empty defaults (Branch, Path, PollInterval) are filled
 // in by the caller using the same defaults the `git_repos` table uses.
 type CreateParams struct {
-	Name         string
-	URL          string
-	Branch       string
-	Path         string
-	PollInterval string
-	Credential   string
-	AutoApply    bool
-	Prune        bool
+	Name          string
+	URL           string
+	Branch        string
+	Path          string
+	PollInterval  string
+	Credential    string
+	AutoApply     bool
+	Prune         bool
+	WebhookSecret string // empty → NULL (no HMAC verification)
 }
 
 // Create inserts a new repo row and emits a repo.added audit event.
@@ -68,27 +69,38 @@ func (s *Store) Create(ctx context.Context, p CreateParams) (*Repo, error) {
 
 	teamID := auth.TeamIDFromContext(ctx)
 
+	var webhookSecret any
+	if p.WebhookSecret != "" {
+		webhookSecret = p.WebhookSecret
+	} else {
+		webhookSecret = nil
+	}
+
 	tx, err := s.DB.BeginTx(ctx, nil)
 	if err != nil {
 		return nil, fmt.Errorf("starting transaction: %w", err)
 	}
 	defer tx.Rollback() //nolint:errcheck
 
 	var r Repo
+	var returnedWebhookSecret sql.NullString
 	err = tx.QueryRowContext(ctx,
 		`INSERT INTO git_repos
-		 (team_id, name, url, branch, path, poll_interval, credential, auto_apply, prune)
-		 VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)
+		 (team_id, name, url, branch, path, poll_interval, credential, auto_apply, prune, webhook_secret)
+		 VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)
 		 RETURNING id, name, url, branch, path, poll_interval, credential,
-		           auto_apply, prune, enabled, created_at, updated_at`,
+		           auto_apply, prune, enabled, webhook_secret, created_at, updated_at`,
 		teamID, p.Name, p.URL, p.Branch, p.Path, p.PollInterval, p.Credential,
-		p.AutoApply, p.Prune,
+		p.AutoApply, p.Prune, webhookSecret,
 	).Scan(&r.ID, &r.Name, &r.URL, &r.Branch, &r.Path, &r.PollInterval,
-		&r.Credential, &r.AutoApply, &r.Prune, &r.Enabled,
+		&r.Credential, &r.AutoApply, &r.Prune, &r.Enabled, &returnedWebhookSecret,
 		&r.CreatedAt, &r.UpdatedAt)
 	if err != nil {
 		return nil, fmt.Errorf("creating repo %q: %w", p.Name, err)
 	}
+	if returnedWebhookSecret.Valid {
+		r.WebhookSecret = returnedWebhookSecret.String
+	}
 
 	if err := audit.EmitTx(ctx, tx, audit.Event{
 		Timestamp: time.Now(),
@@ -155,13 +167,14 @@ func (s *Store) List(ctx context.Context) ([]Repo, error) {
 // intentionally omitted — changing either requires delete + recreate so
 // that audit history clearly reflects the identity change.
 type UpdateParams struct {
-	Branch       string
-	Path         string
-	PollInterval string
-	Credential   string
-	AutoApply    bool
-	Prune        bool
-	Enabled      bool
+	Branch        string
+	Path          string
+	PollInterval  string
+	Credential    string
+	AutoApply     bool
+	Prune         bool
+	Enabled       bool
+	WebhookSecret string // empty → NULL (no HMAC verification)
 }
 
 // Update replaces the mutable fields of a repo by name and emits a
@@ -176,31 +189,43 @@ func (s *Store) Update(ctx context.Context, name string, p UpdateParams) (*Repo,
 
 	teamID := auth.TeamIDFromContext(ctx)
 
+	var updateWebhookSecret any
+	if p.WebhookSecret != "" {
+		updateWebhookSecret = p.WebhookSecret
+	} else {
+		updateWebhookSecret = nil
+	}
+
 	tx, err := s.DB.BeginTx(ctx, nil)
 	if err != nil {
 		return nil, fmt.Errorf("starting transaction: %w", err)
 	}
 	defer tx.Rollback() //nolint:errcheck
 
 	var r Repo
+	var updatedWebhookSecret sql.NullString
 	err = tx.QueryRowContext(ctx,
 		`UPDATE git_repos
 		 SET branch = $3, path = $4, poll_interval = $5, credential = $6,
-		     auto_apply = $7, prune = $8, enabled = $9, updated_at = NOW()
+		     auto_apply = $7, prune = $8, enabled = $9, webhook_secret = $10,
+		     updated_at = NOW()
 		 WHERE name = $1 AND team_id = $2
 		 RETURNING id, name, url, branch, path, poll_interval, credential,
-		           auto_apply, prune, enabled, created_at, updated_at`,
+		           auto_apply, prune, enabled, webhook_secret, created_at, updated_at`,
 		name, teamID, p.Branch, p.Path, p.PollInterval, p.Credential,
-		p.AutoApply, p.Prune, p.Enabled,
+		p.AutoApply, p.Prune, p.Enabled, updateWebhookSecret,
 	).Scan(&r.ID, &r.Name, &r.URL, &r.Branch, &r.Path, &r.PollInterval,
-		&r.Credential, &r.AutoApply, &r.Prune, &r.Enabled,
+		&r.Credential, &r.AutoApply, &r.Prune, &r.Enabled, &updatedWebhookSecret,
 		&r.CreatedAt, &r.UpdatedAt)
 	if errors.Is(err, sql.ErrNoRows) {
 		return nil, fmt.Errorf("%w: %q", ErrNotFound, name)
 	}
 	if err != nil {
 		return nil, fmt.Errorf("updating repo %q: %w", name, err)
 	}
+	if updatedWebhookSecret.Valid {
+		r.WebhookSecret = updatedWebhookSecret.String
+	}
 
 	if err := audit.EmitTx(ctx, tx, audit.Event{
 		Timestamp: time.Now(),

packages/engine/internal/repo/store_test.go

@@ -390,6 +390,45 @@ func TestStore_Delete_IgnoresMissingCloneDir(t *testing.T) {
 	}
 }
 
+func TestStore_Create_StoresWebhookSecret(t *testing.T) {
+	store := newTestStore(t)
+	ctx := defaultCtx()
+	r, err := store.Create(ctx, CreateParams{
+		Name: "with-secret", URL: "https://example.com/a.git", Branch: "main",
+		Path: "/", PollInterval: "60s", Credential: "pat",
+		WebhookSecret: "s3cr3t",
+	})
+	if err != nil {
+		t.Fatalf("Create: %v", err)
+	}
+	got, err := store.GetByID(ctx, r.ID)
+	if err != nil {
+		t.Fatalf("GetByID: %v", err)
+	}
+	if got.WebhookSecret != "s3cr3t" {
+		t.Errorf("WebhookSecret: got %q, want %q", got.WebhookSecret, "s3cr3t")
+	}
+}
+
+func TestStore_Create_NullWebhookSecretByDefault(t *testing.T) {
+	store := newTestStore(t)
+	ctx := defaultCtx()
+	r, err := store.Create(ctx, CreateParams{
+		Name: "no-secret", URL: "https://example.com/b.git", Branch: "main",
+		Path: "/", PollInterval: "60s", Credential: "pat",
+	})
+	if err != nil {
+		t.Fatalf("Create: %v", err)
+	}
+	got, err := store.GetByID(ctx, r.ID)
+	if err != nil {
+		t.Fatalf("GetByID: %v", err)
+	}
+	if got.WebhookSecret != "" {
+		t.Errorf("WebhookSecret should be empty, got %q", got.WebhookSecret)
+	}
+}
+
 func TestStore_UpdateSyncState_ClearsErrorWhenEmpty(t *testing.T) {
 	store := newTestStore(t)
 	ctx := defaultCtx()

packages/engine/internal/repo/sync/engine.go

@@ -50,7 +50,7 @@ func SyncRepo(ctx context.Context, database *sql.DB, store *repo.Store, r *repo.
 
 	pull, err := driver.Pull(ctx, r)
 	if err != nil {
-		_ = store.UpdateSyncState(ctx, r.ID, "", fmt.Sprintf("pull failed: %v", err))
+		_ = store.UpdateSyncState(ctx, r.ID, "", sanitizeURL(fmt.Sprintf("pull failed: %v", err)))
 		emit(ctx, database, audit.Event{
 			Timestamp: time.Now(),
 			Actor:     "sync",
@@ -64,7 +64,7 @@ func SyncRepo(ctx context.Context, database *sql.DB, store *repo.Store, r *repo.
 
 	files, err := Discover(pull.LocalPath, r.Path)
 	if err != nil {
-		_ = store.UpdateSyncState(ctx, r.ID, pull.SHA, fmt.Sprintf("discover failed: %v", err))
+		_ = store.UpdateSyncState(ctx, r.ID, pull.SHA, sanitizeURL(fmt.Sprintf("discover failed: %v", err)))
 		emit(ctx, database, audit.Event{
 			Timestamp: time.Now(),
 			Actor:     "sync",
@@ -149,7 +149,7 @@ func SyncRepo(ctx context.Context, database *sql.DB, store *repo.Store, r *repo.
 
 	errSummary := ""
 	if len(report.Failures) > 0 {
-		errSummary = summarizeFailures(report.Failures)
+		errSummary = sanitizeURL(summarizeFailures(report.Failures))
 	}
 	_ = store.UpdateSyncState(ctx, r.ID, pull.SHA, errSummary)
 

packages/engine/internal/server/git_webhook_test.go

@@ -7,7 +7,6 @@ import (
 	"crypto/sha256"
 	"database/sql"
 	"encoding/hex"
-	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -132,5 +131,4 @@ func TestGitWebhook_UnknownRepo_404(t *testing.T) {
 	if rec.Code != http.StatusNotFound {
 		t.Errorf("status: got %d, want 404", rec.Code)
 	}
-	fmt.Println("ok")
 }