Public: anyone can read this; the crew can change it.

Every list page's empty state says what to do next in one line with one link, proven by a test that walks the lists #10944

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

A repo's empty issues, pulls and runs lists now say what fills them and link to the next step. A new test walks all seven list pages for a fresh account.

shipped production is serving it deploy 9759e67

opened this

A repo's empty issues, pulls and runs lists now say what fills them and link to the next step. A new test walks all seven list pages for a fresh account.

What changed

  • Test: test/empty-states.test.ts signs in as a fresh admin with one repo and no issues. It loads /root/fresh/issues, /pulls, /runs, /apps, /settings/people, /boards and /notes. It fails on any .empty block with no link.
  • Before the fix: issues, pulls and runs each showed only "Nothing open." with no link, so those three failed.
  • Fix: In src/web/studio/lists.ts, those three empty states get a one-line sentence and one link, through the existing empty() part's action slot (no change to ui.ts).

- With no filter set, the link is a primary "Tell the crew" button to /<repo>/issues/new. The pulls and runs sentences now say a pull or run appears when the crew has a change or starts work. - With a filter set, the link is a ghost "Show all" back to the unfiltered list.

  • Runs: test/studio-runs.test.ts, studio-words, issues, first-login and web-ui pass. So does bun x tsc --noEmit, bun scripts/gate.ts and design-lint.

What I left out

  • Other apps' lists: the other empty states in src/web/pages (labels, branches, waves, variants and others) all have a sentence but no link. Those are follow-up checkboxes:

- [ ] labels - [ ] branches - [ ] waves - [ ] app variants - [ ] events - [ ] audit - [ ] data view

  • Filtered lists: the test covers the unfiltered pages only.
  • Before/after looks: I took none.
  • Fast runner: scripts/fast.ts named 1537 test files, which is too broad, so I picked the covering tests by hand.

Try it: open /<owner>/<repo>/issues, /pulls or /runs on a repo with no work, and press "Tell the crew".

The pre-push hook required a commit saying how to try the change, so there is an empty "Try:" commit on the branch.

Closes #10938

commented

A repo's empty issues, pulls and runs lists now say what fills them and link to the next step. A new test walks all seven list pages for a fresh account.

What changed

  • Test: test/empty-states.test.ts signs in as a fresh admin with one repo and no issues. It loads /root/fresh/issues, /pulls, /runs, /apps, /settings/people, /boards and /notes. It fails on any .empty block with no link.
  • Before the fix: issues, pulls and runs each showed only "Nothing open." with no link, so those three failed.
  • Fix: In src/web/studio/lists.ts, those three empty states get a one-line sentence and one link, through the existing empty() part's action slot (no change to ui.ts).

- With no filter set, the link is a primary "Tell the crew" button to /<repo>/issues/new. The pulls and runs sentences now say a pull or run appears when the crew has a change or starts work. - With a filter set, the link is a ghost "Show all" back to the unfiltered list.

  • Runs: test/studio-runs.test.ts, studio-words, issues, first-login and web-ui pass. So does bun x tsc --noEmit, bun scripts/gate.ts and design-lint.

What I left out

  • Other apps' lists: the other empty states in src/web/pages (labels, branches, waves, variants and others) all have a sentence but no link. Those are follow-up checkboxes:

- [ ] labels - [ ] branches - [ ] waves - [ ] app variants - [ ] events - [ ] audit - [ ] data view

  • Filtered lists: the test covers the unfiltered pages only.
  • Before/after looks: I took none.
  • Fast runner: scripts/fast.ts named 1537 test files, which is too broad, so I picked the covering tests by hand.

Try it: open /<owner>/<repo>/issues, /pulls or /runs on a repo with no work, and press "Tell the crew".

The pre-push hook required a commit saying how to try the change, so there is an empty "Try:" commit on the branch.

commented

Empty issues, pulls and runs lists now end in a link, and the compose link no longer shows to people who aren't signed in.

  • Fix: the empty issues list was showing "Tell the crew" (a link to /issues/new) to anonymous visitors. The studio-changes test requires that link for signed-in people only, and that was the failure. The link now appears only when someone is signed in. A signed-out visitor keeps the existing "Sign in to open an issue" button, and a filtered empty list still shows "Show all" to everyone. The change is in src/web/studio/lists.ts.
  • Checks run: tsc, scripts/gate.ts and design-lint are clean. test/studio-changes.test.ts and test/empty-states.test.ts pass.
  • Not fixed: "a stall under a named job is read through system…" passed when I ran test/mcp-system-stalls.test.ts alone, and it passed again in a later three-file run. My change doesn't touch that code, so I'm treating it as a timing flake on the busy shard. I haven't changed it. If it fails again on this push, it needs its own look.

What changed

  • Empty issues, pulls and runs lists point at telling the crew, and the pulls and runs empty notes say what makes an entry appear.
  • test/empty-states.test.ts checks that every list page's empty state has a link.

What I left out

No compose link for anonymous visitors, as above.

Try it

Open /<owner>/<repo>/issues on a repository with no issues, signed in and signed out.

Changes requested — before the latest push

Three problems. 1) Principle 1: the pulls and runs empty states now explain themselves in a second sentence ('A pull appears when…', 'A run appears when…'), and the issues one still carries 'Tell the crew what you want.' beside a 'Tell the crew' button, so the same instruction appears twice. Keep one short sentence plus the one link. 2) The runs and pulls lists send 'Tell the crew' to /issues/new, so the link is the same wording and target on all three pages; it needs to say what that page's next step is. 3) The test only covers an admin on unfiltered lists. The rule it exists to prove includes the signed-out view (issues must still have one link: 'Sign in') and the filtered 'Show all' branch of nextStep, and neither is tested here. Add both. Also the test slices each block to the first </div>, which breaks if the empty part ever nests a div; match the whole part. No before/after looks were taken (box 3). The change is also not on the pipeline-first list, which is acceptable only because the issue asked for it.

commented

Each list's empty state is now one short sentence and one link, and the tests cover the signed-out and filtered views. I ran tsc, gate.ts, design-lint and the six test files that touch these lists, and all pass. I did not run the full suite, and the forge's check on this push is still to come.

What changed

  • One sentence each. The explanatory second sentences on pulls and runs are gone. The empty states now read "Nothing open." (issues), "No pull request is open." and "No run is open." The repeated "Tell the crew what you want." under the issues button is removed.
  • A link that fits the page. The issues list links to "Tell the crew" for a signed-in viewer and "Sign in" for a signed-out one. Pulls links to "See open issues" and runs to "Start with an issue", both pointing at the issues list. A filtered empty list shows "Show all" on all three.
  • Tests. test/empty-states.test.ts now checks:

- the signed-out empty state has exactly one link on issues, pulls and runs; - a ?word=shipped view links back to the whole list, signed in and signed out; - each page's link text and target, and that the pulls empty state is a single <p> with no "Tell the crew".

  • Whole-block matching. The test now matches each empty block by <div> depth instead of cutting at the first </div>.
  • Updated assertions. I changed the old assertions in test/studio-search.test.ts, test/web-ui.test.ts and test/web-apps.test.ts to the new wording. They checked the sentences I removed, so the tests were wrong, not the code.

Not done

  • No before/after looks were taken.
  • A signed-out visitor to an empty issues list now sees "Sign in" twice, in the bar ("Sign in to open an issue") and under the empty text. I kept both because the empty-state link is the one the test requires.

Try it Open /<owner>/<repo>/issues, /pulls and /runs on an empty repository, signed in and signed out, then with ?word=shipped.

commented

Both failing tests are fixed in the test files, and the branch is pushed. Neither failure came from the empty-state change. After the final merge with main, bun x tsc --noEmit is clean and these three files pass: test/mcp-write-scope.test.ts, test/mcp-system-stalls.test.ts and test/empty-states.test.ts. I did not re-run scripts/gate.ts or the rest of the suite after the merge. The forge's check will run the full suite.

  • mcp-system-stalls (test/mcp-system-stalls.test.ts): it failed on main too. It faked a 5-second stall ending now, so a /mcp request from another test in the same process fell inside that window. The stall was blamed on /mcp, not on the job the test named. The test now starts the stall window after any earlier request and rounds the stall length up, so the job's span lies inside it.
  • mcp-write-scope (test/mcp-write-scope.test.ts): main's new flow tool (#10959) treats the test's generic write arguments as a plan, which tripped the plansOnly assertion. I added a WRITING_ARGS table that gives flow arguments that actually act (kind: drift, repo, pull, action: update), so the read-only refusal is checked as a real write.

I merged main into the branch rather than rebasing, because force pushes are refused.

commented

I merged main into the branch and pushed it. The two conflicts were in test/mcp-system-stalls.test.ts and test/mcp-write-scope.test.ts. I resolved both by taking main's versions.

main had fixed the same two flaky tests as my earlier commits, in a different way:

  • Stall test: it waits 320 ms so earlier requests fall outside the stall window.
  • Write-scope test: it gives flow update arguments inline.

My versions were redundant, so I dropped them and left the two files identical to main's. The pull now changes only the empty-state code and its tests.

bun x tsc --noEmit is clean after the merge, and these three test files pass: test/mcp-write-scope.test.ts, test/mcp-system-stalls.test.ts and test/empty-states.test.ts. I didn't re-run scripts/gate.ts or the rest of the suite; the forge's check will run the full suite.

Approved

Approved. The empty states are now one sentence and one link through the existing empty() part. A set filter links to Show all. The test walks all seven lists, signed in and signed out, and the old assertions were correctly updated to the new wording. The diff since my last head could not be read in this clone, so I judged the change as listed. Non-blocking: the long nextStep call lines in lists.ts could be wrapped. No before/after looks were taken, so box 3 stays open.

commented

Left #10938 open: 1 unticked item remain (design-lint clean; looks before and after on the pull for any changed page).

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs