Wire sync's -project flag to actually project onto the board

openspec/changes/wire-sync-board-projection/proposal.md

@@ -0,0 +1,86 @@
+# Wire sync's -project flag to actually project onto the board
+
+## Why
+
+`sync -project owner/N` (or `openspec/specsync.yml`) resolves a
+`BoardTarget`, threads it through `Options.Project`, and `Sync` reports
+`ItemResult.BoardConfigured` from it — but never calls `ProjectOntoBoard`.
+Only `pull` does (`pull.go:134`). The result: `sync -project` runs clean,
+reports success, `BoardConfigured: true`, and silently does nothing to the
+board. `printBoardPlan` (`cmd/specsync/main.go:887`) prints nothing useful
+either, since it's handed a permanently-zero-value `BoardPlan` — so there's
+no error, no warning, just quiet non-function exactly matching the
+documented, expected behavior in shape (`BoardConfigured` says true) while
+doing none of it.
+
+**This is a regression, not a gap.** `git log -S ProjectOntoBoard -- sync.go`
+shows `Sync` used to call it, inline in a per-provider `syncOne` helper, with
+a hand-rolled three-way-merge dance (query current board state, compare
+against a saved binding, decide push-local vs. respect-human-move vs.
+report-conflict) before deciding whether to call `ProjectOntoBoard` at all.
+Commit `4177dc7` ("multi-provider sync (fan-out)") restructured `Sync` into
+its current per-provider loop and dropped that whole block — `Board:` and
+the `ProjectOntoBoard` call vanished from the diff; `BoardConfigured:
+opts.Project.Configured()` and the `Board BoardPlan` field on `ItemResult`
+were left behind, reporting stale/empty data ever since. No test caught it:
+`board_test.go` only calls `ProjectOntoBoard` directly, never through
+`Sync`; `TestBoardTargetCarriesStatusMapping` (added for an earlier,
+narrower version of this same class of bug — `BoardTarget.StatusMapping`
+reaching the CLI) only checks target construction, not that projection runs.
+
+**The fix is smaller than the regression, though.** `ProjectOntoBoard`
+itself (`board.go:98`) has been rewritten since the fan-out refactor into a
+fully self-contained operation: it resolves the board schema, detects human
+moves in both directions (`HumanMovedToDone`/`HumanMovedToActive`),
+protects against clobbering a human-set Status via
+`LastWrittenOptionID`-based reasoning, and saves its own board binding
+(`changeDir` param) — internally, on every call. None of the external
+three-way-merge machinery the old `syncOne` had to hand-roll is needed
+anymore; `pull.go` already proves this by calling `ProjectOntoBoard` with a
+single line. Restoring the call to `Sync` is that same single line, not a
+rebuild of the old logic.
+
+## Proposed Changes
+
+- In `Sync`'s per-provider loop (`sync.go`), after `Push` succeeds, call
+  `bp.ProjectOntoBoard(ctx, opts.Project, ref, item, opts.DryRun, c.Dir)`
+  when `opts.Project.Configured()` and the provider implements
+  `BoardProjector` — mirroring `pull.go`'s existing call exactly. It already
+  handles dry-run internally (a zero-API preview), so no extra dry-run
+  branching is needed here.
+- Capture the first successful plan into `ItemResult.Board` (first-provider-
+  wins, matching the existing `firstRef`/`firstCreated` pattern for
+  URL/created reporting) so `printBoardPlan` finally has real data.
+- A `ProjectOntoBoard` error is non-fatal to the change's sync (the issue
+  itself already pushed successfully) but printed as a warning to stderr —
+  silence is exactly the failure mode this proposal exists to remove, so a
+  new failure path here must not repeat it.
+
+## Non-Goals
+
+- **Not** touching `DepSync`'s existing error handling
+  (`sync.go`, dependency-edge reconcile) even though it has the same
+  silent-swallow shape (`if _, err := DepSync(...); err != nil { /* Log and
+  continue */ }` — no logging actually happens). Same bug family, separate
+  fix, separate PR.
+- **Not** reintroducing the old inline three-way-merge dance from before
+  the fan-out refactor — superseded by `ProjectOntoBoard`'s own internal
+  handling, per the Why section.
+- **Not** per-provider board reporting in `ProviderResult` for multi-board
+  fan-out (multiple providers simultaneously implementing `BoardProjector`)
+  — first-provider-wins is enough today since GitHub is the only
+  `BoardProjector` implementation that exists.
+
+## Open Questions
+
+1. Should a `ProjectOntoBoard` failure also skip `DepSync` and any later
+   post-push step for that provider in the same run, or are they
+   independent enough to both attempt regardless? Leaning independent —
+   they touch unrelated state (board vs. issue cross-links).
+
+## Release Notes
+
+Fixed: `sync -project` (and `openspec/specsync.yml`'s board target) now
+actually projects issues onto the configured GitHub Projects board — it
+previously reported `BoardConfigured: true` and silently did nothing.
+`pull` was unaffected; this restores parity for `sync`.

openspec/changes/wire-sync-board-projection/tasks.md

@@ -0,0 +1,55 @@
+# Tasks
+
+- [x] 1. In `Sync`'s per-provider loop (`sync.go`), after `ref, perr :=
+      prov.Push(...)` succeeds, add: when `opts.Project.Configured()` and
+      `prov` implements `BoardProjector`, call
+      `bp.ProjectOntoBoard(ctx, opts.Project, ref, item, opts.DryRun, c.Dir)`.
+      Track the first successful plan into a `boardPlan BoardPlan` local
+      (mirroring the existing `firstRef`/`firstCreated` pattern), and set
+      `ItemResult.Board: boardPlan` in the `res.Items = append(...)` call
+      that already sets `BoardConfigured`.
+      Validation: a fixture `Sync` call with a configured `BoardTarget`
+      against a `BoardProjector`-implementing fake provider results in a
+      `ProjectOntoBoard` call being made (mirrors `pull_test.go`'s existing
+      coverage of the same call via `Pull`) and `ItemResult.Board` non-zero.
+
+- [x] 2. A `ProjectOntoBoard` error is non-fatal: print
+      `fmt.Fprintf(os.Stderr, "specsync: warning: board projection for %s
+      failed: %v\n", c.Slug, err)` and continue the rest of that provider's
+      post-push steps (dependency sync, etc.) rather than `continue`-ing
+      past them. Add the `os` import to `sync.go`.
+      Validation: a fixture where the fake `BoardProjector` returns an error
+      still completes the sync (issue created/updated, no early exit) and
+      the warning is observable (captured stderr, or the function signature
+      exposes it for the test — match whatever pattern existing warnings in
+      this codebase use, e.g. `pull.go`'s intake-transition warning).
+
+- [x] 3. Regression test proving the specific regression this proposal
+      describes: `Sync` with `Options.Project` configured against a
+      `BoardProjector` fake provider must result in exactly one
+      `ProjectOntoBoard` call per change per provider — not zero (the bug),
+      not more than one.
+
+- [x] 4. Dry-run coverage: `Sync` with `DryRun: true` and a configured board
+      target still calls `ProjectOntoBoard` (it handles dry-run internally,
+      zero API calls) and the resulting `ItemResult.Board` is non-zero, so
+      `-dry-run` output actually previews the board plan instead of the CLI
+      printing nothing under "board:".
+
+- [x] 5. Manual/live verification: ran `specsync -dry-run -change
+      wire-sync-board-projection -project androidand/2` against a real
+      GitHub Projects board with both the unpatched and patched binary.
+      Unpatched: only the static "would ensure the issue is on the board"
+      line printed (StatusName/AssigneeLogin always empty). Patched: all
+      three preview lines printed (add/status/assignee), confirming the
+      exact silent-no-op this proposal describes and its fix. Did not
+      perform a real (non-dry-run) write against that board — it is an
+      unrelated project (BrickNow Prioritization), not a low-stakes test
+      board for this repo, so a live mutation was skipped pending a real
+      target.
+
+- [x] 6. No documentation change needed for accuracy — `README.md:592-599`
+      ("sync + project onto board 6") and `site/features.json`'s "Projects
+      board sync" entry already correctly describe the intended behavior;
+      this proposal makes reality match what's already documented, not the
+      other way round.

pull_test.go

@@ -644,12 +644,24 @@ func TestPull_RequiresIssueForNonGithub(t *testing.T) {
 
 type stubNonGithubProvider struct{}
 
-func (s *stubNonGithubProvider) Name() string                                         { return "beads" }
-func (s *stubNonGithubProvider) Find(context.Context, string) (*Ref, error)           { return nil, nil }
-func (s *stubNonGithubProvider) Push(context.Context, WorkItem, *Ref) (Ref, error)    { return Ref{}, nil }
-func (s *stubNonGithubProvider) Get(context.Context, string) (FetchedItem, error)     { return FetchedItem{}, nil }
+func (s *stubNonGithubProvider) Name() string                                      { return "beads" }
+func (s *stubNonGithubProvider) Find(context.Context, string) (*Ref, error)        { return nil, nil }
+func (s *stubNonGithubProvider) Push(context.Context, WorkItem, *Ref) (Ref, error) { return Ref{}, nil }
+func (s *stubNonGithubProvider) Get(context.Context, string) (FetchedItem, error) {
+	return FetchedItem{}, nil
+}
 
 func TestPull_CannotResolveFromBranch(t *testing.T) {
+	// Force a branch name that never matches the feat/<n>-<slug> pattern,
+	// regardless of what branch this test suite actually happens to be
+	// checked out on (e.g. a real feat/<n>-<slug> branch, which would
+	// otherwise make this test try to resolve issue <n> for real and fail
+	// with an unrelated JSON-parse error instead of the branch-mismatch
+	// error it's meant to assert on).
+	prev := currentBranchFn
+	currentBranchFn = func() (string, error) { return "main", nil }
+	defer func() { currentBranchFn = prev }()
+
 	dir := t.TempDir()
 	// On a branch that doesn't match the pattern, Pull should error.
 	_, err := Pull(context.Background(), PullOptions{

sync.go

@@ -3,6 +3,7 @@ package specsync
 import (
 	"context"
 	"fmt"
+	"os"
 	"strings"
 )
 
@@ -175,6 +176,7 @@ func Sync(ctx context.Context, opts Options) (Result, error) {
 		var firstRef Ref
 		var firstCreated bool
 		var firstClosedDeferred string
+		var boardPlan BoardPlan
 		providerResults := make([]ProviderResult, 0, len(providers))
 
 		for _, prov := range providers {
@@ -260,6 +262,17 @@ func Sync(ctx context.Context, opts Options) (Result, error) {
 				firstClosedDeferred = ref.ClosedDeferred
 			}
 
+			if opts.Project.Configured() {
+				if bp, ok := prov.(BoardProjector); ok {
+					plan, berr := bp.ProjectOntoBoard(ctx, opts.Project, ref, item, opts.DryRun, c.Dir)
+					if berr != nil {
+						fmt.Fprintf(os.Stderr, "specsync: warning: board projection for %s failed: %v\n", c.Slug, berr)
+					} else if boardPlan == (BoardPlan{}) {
+						boardPlan = plan
+					}
+				}
+			}
+
 			if opts.DryRun {
 				// A dry run must never mutate local state.
 			} else {
@@ -344,6 +357,7 @@ func Sync(ctx context.Context, opts Options) (Result, error) {
 			Flips:           allFlips,
 			TitleSuggestion: suggestion,
 			BoardConfigured: opts.Project.Configured(),
+			Board:           boardPlan,
 			ClosedDeferred:  firstClosedDeferred,
 			Providers:       providerResults,
 		})

sync_test.go

@@ -0,0 +1,169 @@
+package specsync
+
+import (
+	"bytes"
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// boardCapturingProvider is a fake WorkProvider that also implements
+// BoardProjector, so Sync's board-projection call can be observed without a
+// real GitHub round-trip.
+type boardCapturingProvider struct {
+	ref      Ref
+	plan     BoardPlan
+	err      error
+	calls    int
+	lastItem WorkItem
+}
+
+func (p *boardCapturingProvider) Name() string { return "github" }
+func (p *boardCapturingProvider) Push(context.Context, WorkItem, *Ref) (Ref, error) {
+	return p.ref, nil
+}
+func (p *boardCapturingProvider) Find(context.Context, string) (*Ref, error) { return nil, nil }
+
+func (p *boardCapturingProvider) ProjectOntoBoard(_ context.Context, _ BoardTarget, _ Ref, item WorkItem, _ bool, _ string) (BoardPlan, error) {
+	p.calls++
+	p.lastItem = item
+	return p.plan, p.err
+}
+
+func configuredTarget() BoardTarget {
+	return BoardTarget{Owner: "acme", Number: 1}
+}
+
+func TestSyncProjectsOntoBoardWhenConfigured(t *testing.T) {
+	root := t.TempDir()
+	mustWrite(t, filepath.Join(root, "changes", "c1", "proposal.md"), "# C1\n")
+	prov := &boardCapturingProvider{
+		ref:  Ref{Provider: "github", ID: "1", URL: "https://example.test/1"},
+		plan: BoardPlan{ProjectID: "PVT_1", AddedToBoard: true},
+	}
+
+	res, err := Sync(context.Background(), Options{OpenSpecDir: root, Provider: prov, Project: configuredTarget()})
+	if err != nil {
+		t.Fatalf("Sync: %v", err)
+	}
+	if prov.calls != 1 {
+		t.Fatalf("expected exactly 1 ProjectOntoBoard call, got %d", prov.calls)
+	}
+	if len(res.Items) != 1 {
+		t.Fatalf("expected 1 item, got %d", len(res.Items))
+	}
+	item := res.Items[0]
+	if !item.BoardConfigured {
+		t.Error("expected BoardConfigured true")
+	}
+	if item.Board == (BoardPlan{}) {
+		t.Error("expected ItemResult.Board to carry the projected plan, got zero value")
+	}
+	if item.Board.ProjectID != "PVT_1" {
+		t.Errorf("Board.ProjectID = %q, want PVT_1", item.Board.ProjectID)
+	}
+}
+
+func TestSyncSkipsBoardProjectionWhenUnconfigured(t *testing.T) {
+	root := t.TempDir()
+	mustWrite(t, filepath.Join(root, "changes", "c1", "proposal.md"), "# C1\n")
+	prov := &boardCapturingProvider{ref: Ref{Provider: "github", ID: "1"}}
+
+	res, err := Sync(context.Background(), Options{OpenSpecDir: root, Provider: prov})
+	if err != nil {
+		t.Fatalf("Sync: %v", err)
+	}
+	if prov.calls != 0 {
+		t.Fatalf("expected no ProjectOntoBoard calls when Project is unconfigured, got %d", prov.calls)
+	}
+	if res.Items[0].BoardConfigured {
+		t.Error("expected BoardConfigured false")
+	}
+}
+
+func TestSyncBoardProjectionErrorIsNonFatal(t *testing.T) {
+	root := t.TempDir()
+	mustWrite(t, filepath.Join(root, "changes", "c1", "proposal.md"), "# C1\n")
+	prov := &boardCapturingProvider{
+		ref: Ref{Provider: "github", ID: "1", URL: "https://example.test/1"},
+		err: os.ErrInvalid,
+	}
+
+	var stderr bytes.Buffer
+	restore := redirectStderr(t, &stderr)
+	defer restore()
+
+	res, err := Sync(context.Background(), Options{OpenSpecDir: root, Provider: prov, Project: configuredTarget()})
+	if err != nil {
+		t.Fatalf("Sync must not fail when board projection errors: %v", err)
+	}
+	if res.Updated != 1 && res.Created != 1 {
+		t.Fatalf("expected the issue push itself to still count as created/updated, got %+v", res)
+	}
+	if len(res.Items) != 1 || res.Items[0].URL == "" {
+		t.Fatalf("expected the change to still be pushed despite the board error: %+v", res.Items)
+	}
+	restore()
+	if !bytes.Contains(stderr.Bytes(), []byte("board projection for c1 failed")) {
+		t.Errorf("expected a warning naming the change on stderr, got: %s", stderr.String())
+	}
+}
+
+func TestSyncDryRunStillProjectsOntoBoard(t *testing.T) {
+	root := t.TempDir()
+	mustWrite(t, filepath.Join(root, "changes", "c1", "proposal.md"), "# C1\n")
+	prov := &boardCapturingProvider{
+		ref:  Ref{Provider: "github", ID: "0"},
+		plan: BoardPlan{ProjectID: "PVT_1", AddedToBoard: true},
+	}
+
+	res, err := Sync(context.Background(), Options{OpenSpecDir: root, Provider: prov, Project: configuredTarget(), DryRun: true})
+	if err != nil {
+		t.Fatalf("dry-run Sync: %v", err)
+	}
+	if prov.calls != 1 {
+		t.Fatalf("expected ProjectOntoBoard to run under -dry-run (it handles dry-run internally), got %d calls", prov.calls)
+	}
+	if res.Items[0].Board == (BoardPlan{}) {
+		t.Error("expected -dry-run to preview a non-zero board plan")
+	}
+}
+
+// redirectStderr temporarily replaces os.Stderr so a test can capture warnings
+// printed via fmt.Fprintf(os.Stderr, ...). Returns a restore func; safe to
+// call more than once.
+func redirectStderr(t *testing.T, into *bytes.Buffer) func() {
+	t.Helper()
+	orig := os.Stderr
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("os.Pipe: %v", err)
+	}
+	os.Stderr = w
+	done := make(chan struct{})
+	go func() {
+		buf := make([]byte, 4096)
+		for {
+			n, rerr := r.Read(buf)
+			if n > 0 {
+				into.Write(buf[:n])
+			}
+			if rerr != nil {
+				close(done)
+				return
+			}
+		}
+	}()
+	restored := false
+	return func() {
+		if restored {
+			return
+		}
+		restored = true
+		os.Stderr = orig
+		w.Close()
+		<-done
+		r.Close()
+	}
+}