fix(ui): v0.3.1 polish from the first outside-repo run — installer next step, blocked panel, neutral placeholders, label a11y, setup layout, post-publish sync (#282)

lab/internal/cli/cli.go

@@ -36,6 +36,12 @@ import (
 
 var safeID = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`)
 
+// installNextSteps is the installer's closing hint: the whole loop starts
+// from forgecell ui in the repository, with doctor kept as a diagnosis hint.
+func installNextSteps() string {
+	return "Next: cd into your repository and run forgecell ui (Set up your Lab).\nTrouble? Run forgecell doctor to see what is missing.\n"
+}
+
 func Run(ctx context.Context, args []string, in io.Reader, out, stderr io.Writer, version string) int {
 	fail := func(err error) int { fmt.Fprintln(stderr, "forgecell:", err); return 1 }
 	harness.SetRunningVersion(version)
@@ -300,7 +306,7 @@ func Run(ctx context.Context, args []string, in io.Reader, out, stderr io.Writer
 		if err != nil {
 			return fail(err)
 		}
-		fmt.Fprintf(out, "Forgecell %s installed. Lab data preserved.\nCommand: %s\nNext: forgecell doctor → forgecell init → approve Formula → forgecell run <issue>\n", result.Version, result.Command)
+		fmt.Fprintf(out, "Forgecell %s installed. Lab data preserved.\nCommand: %s\n%s", result.Version, result.Command, installNextSteps())
 		fmt.Fprintf(out, "Ensure this directory is on PATH: %s\nGit, authenticated GitHub CLI (gh), and a configured coding harness are required to run tickets.\n", filepath.Dir(result.Command))
 		return 0
 	}

lab/internal/cli/install_next_test.go

@@ -0,0 +1,35 @@
+package cli
+
+import (
+	"os"
+	"strings"
+	"testing"
+)
+
+func TestInstallNextStepsPointsAtUI(t *testing.T) {
+	text := installNextSteps()
+	for _, want := range []string{"cd into your repository", "forgecell ui", "Set up your Lab", "forgecell doctor"} {
+		if !strings.Contains(text, want) {
+			t.Fatalf("installer next step lacks %q:\n%s", want, text)
+		}
+	}
+	for _, stale := range []string{"forgecell init", "approve Formula", "forgecell run", "→"} {
+		if strings.Contains(text, stale) {
+			t.Fatalf("installer next step still holds stale %q:\n%s", stale, text)
+		}
+	}
+	if strings.Index(text, "forgecell ui") > strings.Index(text, "forgecell doctor") {
+		t.Fatalf("forgecell ui must be the next step, doctor only a hint:\n%s", text)
+	}
+	if strings.Contains(text, "/Users/") || strings.Contains(text, "/home/") || strings.Contains(text, "$") {
+		t.Fatalf("installer next step leaks a local path or variable:\n%s", text)
+	}
+	for _, value := range []string{os.Getenv("HOME"), os.Getenv("FORGECELL_HOME"), os.Getenv("FORGECELL_BIN")} {
+		if value != "" && strings.Contains(text, value) {
+			t.Fatalf("installer next step leaks an environment value:\n%s", text)
+		}
+	}
+	if !strings.HasSuffix(text, "\n") {
+		t.Fatalf("installer next step must end its line: %q", text)
+	}
+}

lab/internal/forgesync/cache.go

@@ -7,9 +7,15 @@ import (
 )
 
 // TTL is how long one read of GitHub serves a server: at most one sync per TTL,
-// failed or not, unless a caller asks for a refresh.
+// failed or not, unless a caller asks for a refresh or wants a pull request no
+// attempt has asked for yet.
 const TTL = 60 * time.Second
 
+// maxAskedPRs bounds the pull request numbers the cache remembers asking for.
+// Once full, a new number waits for the TTL like any other read: the bound
+// stops early reads, never the TTL.
+const maxAskedPRs = MaxStateNumbers
+
 // syncTimeout bounds one whole sync, every gh call in it included.
 const syncTimeout = 2 * time.Minute
 
@@ -66,7 +72,11 @@ type Cache struct {
 	reason        string
 	attempted     time.Time
 	attemptedRepo string
-	flight        *flight
+	// asked holds the pull request numbers the attempts for attemptedRepo
+	// wanted, at most maxAskedPRs. A want naming one not in it is read again
+	// within the TTL, once: the first view after publishing sees its PR.
+	asked  map[int64]bool
+	flight *flight
 	// joined, when set by a test, receives once per caller that joins a flight:
 	// a readiness marker, so tests never poll.
 	joined chan struct{}
@@ -87,8 +97,9 @@ func NewCache(fetcher Fetcher, now func() time.Time) *Cache {
 }
 
 // Get returns GitHub state for want, syncing when the latest attempt for its
-// repository is older than TTL or refresh is set. A caller arriving while a
-// sync runs waits for it, or for ctx, instead of starting another.
+// repository is older than TTL, refresh is set, or want names a pull request
+// no attempt for the repository has asked for. A caller arriving while a sync
+// runs waits for it, or for ctx, instead of starting another.
 func (c *Cache) Get(ctx context.Context, want Want, refresh bool) Result {
 	c.mu.Lock()
 	if f := c.flight; f != nil {
@@ -107,7 +118,7 @@ func (c *Cache) Get(ctx context.Context, want Want, refresh bool) Result {
 		return c.result(want.Repository)
 	}
 	fresh := !c.attempted.IsZero() && c.attemptedRepo == want.Repository && c.now().Sub(c.attempted) < TTL
-	if fresh && !refresh {
+	if fresh && !refresh && !c.newPR(want.PRs) {
 		defer c.mu.Unlock()
 		return c.result(want.Repository)
 	}
@@ -128,7 +139,13 @@ func (c *Cache) run(want Want, f *flight) (r Result) {
 			snap, reason = nil, ReasonFailed
 		}
 		c.mu.Lock()
+		if c.attemptedRepo != want.Repository {
+			c.asked = nil
+		}
 		c.attempted, c.attemptedRepo, c.reason = c.now(), want.Repository, reason
+		// Failed or not, the numbers count as asked, so a failure or a read cut
+		// off at MaxStateNumbers never forces another read before the TTL.
+		c.recordAsked(want.PRs)
 		if reason == "" {
 			c.snap = snap
 		} else if c.snap != nil && c.snap.Repository != want.Repository {
@@ -143,6 +160,33 @@ func (c *Cache) run(want Want, f *flight) (r Result) {
 	return r
 }
 
+// newPR reports a wanted pull request no attempt for attemptedRepo asked for,
+// while asked is below its bound; the caller holds mu.
+func (c *Cache) newPR(prs []int64) bool {
+	if len(c.asked) >= maxAskedPRs {
+		return false
+	}
+	for _, n := range prs {
+		if !c.asked[n] {
+			return true
+		}
+	}
+	return false
+}
+
+// recordAsked adds prs to asked, up to maxAskedPRs; the caller holds mu.
+func (c *Cache) recordAsked(prs []int64) {
+	if c.asked == nil {
+		c.asked = map[int64]bool{}
+	}
+	for _, n := range prs {
+		if len(c.asked) >= maxAskedPRs {
+			return
+		}
+		c.asked[n] = true
+	}
+}
+
 // Current reports GitHub state fit to allow a start: the latest sync succeeded
 // and its snapshot is at most twice the TTL old. Anything else fails closed.
 func (r Result) Current() bool {

lab/internal/forgesync/cache_fresh_test.go

@@ -0,0 +1,99 @@
+package forgesync
+
+import (
+	"context"
+	"slices"
+	"testing"
+	"time"
+)
+
+// Right after publishing, the wanted set names a new pull request: the first
+// read with it syncs again within the TTL, so the view sees the PR instead of
+// "the published pull request was not checked" until the TTL expires.
+func TestCacheReadsANewlyWantedPRWithinTheTTL(t *testing.T) {
+	f, clk := &fakeFetcher{}, newClock()
+	c := NewCache(f, clk.now)
+	before := Want{Repository: "o/r", PRs: []int64{3}}
+	if r := c.Get(context.Background(), before, false); r.Snapshot == nil || f.lists.Load() != 1 {
+		t.Fatalf("first: %+v lists %d", r, f.lists.Load())
+	}
+	clk.add(5 * time.Second)
+	published := Want{Repository: "o/r", PRs: []int64{3, 4}}
+	r := c.Get(context.Background(), published, false)
+	if f.lists.Load() != 2 || r.Snapshot == nil || r.Snapshot.PRs[4].State != PRMerged || r.Age != 0 {
+		t.Fatalf("new PR within the TTL: lists %d %+v", f.lists.Load(), r)
+	}
+	if !slices.Equal(f.gotPRs, []int64{3, 4}) {
+		t.Fatalf("states asked for %v", f.gotPRs)
+	}
+	// The same set again, a subset, or a set in another order never syncs again.
+	clk.add(5 * time.Second)
+	for _, want := range []Want{published, before, {Repository: "o/r", PRs: []int64{4, 3}}, {Repository: "o/r"}} {
+		if c.Get(context.Background(), want, false); f.lists.Load() != 2 {
+			t.Fatalf("known PRs %v synced again: lists %d", want.PRs, f.lists.Load())
+		}
+	}
+}
+
+// A number already asked for never forces another read, even when the read
+// failed or did not return it: a failure or a cut-off read waits for the TTL.
+func TestCacheNeverRereadsAnAskedPRBeforeTheTTL(t *testing.T) {
+	f, clk := &fakeFetcher{}, newClock()
+	c := NewCache(f, clk.now)
+	good := c.Get(context.Background(), Want{Repository: "o/r"}, false).Snapshot
+	f.fail(fail(ReasonRateLimited))
+	want := Want{Repository: "o/r", PRs: []int64{9}}
+	r := c.Get(context.Background(), want, false)
+	if f.lists.Load() != 2 || r.Reason != ReasonRateLimited || r.Snapshot != good {
+		t.Fatalf("failed read of a new PR: lists %d %+v", f.lists.Load(), r)
+	}
+	f.fail(nil)
+	for i := 0; i < 3; i++ {
+		clk.add(time.Second)
+		if r = c.Get(context.Background(), want, false); f.lists.Load() != 2 || r.Snapshot != good || r.Reason != ReasonRateLimited {
+			t.Fatalf("an asked PR forced another read: lists %d %+v", f.lists.Load(), r)
+		}
+	}
+	clk.add(TTL)
+	if r = c.Get(context.Background(), want, false); f.lists.Load() != 3 || r.Reason != "" || r.Snapshot.PRs[9].State != PRMerged {
+		t.Fatalf("after the TTL: lists %d %+v", f.lists.Load(), r)
+	}
+}
+
+// The numbers asked for belong to one repository: another repository's are
+// forgotten, and the remembered set is bounded, so a full set falls back to the
+// TTL instead of reading early.
+func TestCacheAskedPRsArePerRepositoryAndBounded(t *testing.T) {
+	f, clk := &fakeFetcher{}, newClock()
+	c := NewCache(f, clk.now)
+	c.Get(context.Background(), Want{Repository: "o/r", PRs: []int64{1}}, false)
+	c.Get(context.Background(), Want{Repository: "o/other", PRs: []int64{2}}, false)
+	if f.lists.Load() != 2 {
+		t.Fatalf("other repository: lists %d", f.lists.Load())
+	}
+	c.mu.Lock()
+	stale := c.asked[1]
+	c.mu.Unlock()
+	if stale {
+		t.Fatal("another repository's asked PRs were kept")
+	}
+
+	many := make([]int64, 0, maxAskedPRs+10)
+	for n := int64(1); n <= maxAskedPRs+10; n++ {
+		many = append(many, n)
+	}
+	c.Get(context.Background(), Want{Repository: "o/r", PRs: many}, false)
+	if f.lists.Load() != 3 {
+		t.Fatalf("repository switch back: lists %d", f.lists.Load())
+	}
+	c.mu.Lock()
+	size := len(c.asked)
+	c.mu.Unlock()
+	if size != maxAskedPRs {
+		t.Fatalf("asked holds %d numbers, want the bound %d", size, maxAskedPRs)
+	}
+	clk.add(time.Second)
+	if c.Get(context.Background(), Want{Repository: "o/r", PRs: []int64{maxAskedPRs + 100}}, false); f.lists.Load() != 3 {
+		t.Fatalf("a full asked set read early: lists %d", f.lists.Load())
+	}
+}

lab/internal/ui/molecule_view_test.go

@@ -362,7 +362,7 @@ func TestMoleculeViewServedJS(t *testing.T) {
 			"'Request changes…'",
 			"c.amendForm = el('form', { class: 'amend-form', id: 'mv-amend-form'",
 			"name: 'acceptance', maxlength: '1000', autocomplete: 'off', placeholder: 'The help text names the new flag…', 'aria-describedby': 'mv-amend-error'",
-			"name: 'title', maxlength: '256', autocomplete: 'off', placeholder: 'Add the Molecule view to the Lab UI…', 'aria-describedby': 'mv-publish-error', dataset: { gate: 'title' }, oninput: (e) => { state.gate.title = e.target.value; } }",
+			"name: 'title', maxlength: '256', autocomplete: 'off', placeholder: 'Describe the change in a few words…', 'aria-describedby': 'mv-publish-error', dataset: { gate: 'title' }, oninput: (e) => { state.gate.title = e.target.value; } }",
 			"el('summary', null, 'Not needed anymore?')",
 			"onclick: () => openConfirm('retire') }, 'Retire this Molecule…')",
 			"onclick: () => openConfirm('stop') }, 'Stop this Molecule…')",

lab/internal/ui/served_test.go

@@ -17,6 +17,7 @@ var servedModules = []string{
 	"/js/molecule.js",
 	"/js/molecule-plan.js",
 	"/js/molecule-decision.js",
+	"/js/molecule-actions.js",
 	"/js/molecule-timeline.js",
 	"/js/work.js",
 	"/js/issues.js",

lab/internal/ui/server.go

@@ -260,6 +260,7 @@ var staticFiles = map[string]string{
 	"/js/molecule.js":          "js/molecule.js",
 	"/js/molecule-plan.js":     "js/molecule-plan.js",
 	"/js/molecule-decision.js": "js/molecule-decision.js",
+	"/js/molecule-actions.js":  "js/molecule-actions.js",
 	"/js/molecule-timeline.js": "js/molecule-timeline.js",
 	"/js/work.js":              "js/work.js",
 	"/js/issues.js":            "js/issues.js",