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

"Since you were here" also lists releases, failed checks, and private repositories the viewer can read #6743

Merged opened this

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

Merged

"Since you were here" now counts landed pulls, releases and red merge-queue gates from every repository the viewer can read, still cut to three lines. Type check, design lint and `test/home-since*` pass. I did not run the whole suite.

shipped production is serving it deploy 058b6cf

opened this

"Since you were here" now counts landed pulls, releases and red merge-queue gates from every repository the viewer can read, still cut to three lines. Type check, design lint and test/home-since* pass. I did not run the whole suite.

  • Access: src/web/studio/since.ts has a new eventsSince(db, viewer, since, now) that replaces landedSince. It reads each repository through the feed's canRead, so a private repository shows for its members and never for a stranger.
  • Lines: a release is weighted 2, a red gate 3 and a landed pull 1, so the biggest change leads. The existing digest() still groups by repository and cuts at three, and its output is unchanged.
  • Evidence links: a release links to /releases. A red gate links to /api/v1/repos/{owner}/{name}/queue/gates, the only per-gate evidence the forge has; there is no gate page, so that link is JSON.
  • Callers: sinceDigest and sinceYouWere now take the viewer. Both the web home and the terminal dashboard pass it, so they show the same lines.
  • Releases: only preflight-ok releases count.
  • Not done: I did not add a gate page, and I did not add release links to individual releases.
  • Tests: the new test/home-since-sources.test.ts covers:

- a member sees a private repository's landed pull and a stranger does not; - a release and a red gate each make a line, while a green gate and an old red gate do not; - four sources are cut to three, biggest first.

  • Changed existing test: test/home-since.test.ts was wrong under the new rule. It expected the admin owner's page to hide a private pull, but an admin can now read a peer's private repository. It now checks counts as the peer, and removes that pull before the owner-page assertions.

To try it, load / after a release, a red gate or a merged pull in a repository you can read. The line appears under "Since you were here" and in the terminal dashboard.

Closes #6712

commented

"Since you were here" now counts landed pulls, releases and red merge-queue gates from every repository the viewer can read, still cut to three lines. Type check, design lint and test/home-since* pass. I did not run the whole suite.

  • Access: src/web/studio/since.ts has a new eventsSince(db, viewer, since, now) that replaces landedSince. It reads each repository through the feed's canRead, so a private repository shows for its members and never for a stranger.
  • Lines: a release is weighted 2, a red gate 3 and a landed pull 1, so the biggest change leads. The existing digest() still groups by repository and cuts at three, and its output is unchanged.
  • Evidence links: a release links to /releases. A red gate links to /api/v1/repos/{owner}/{name}/queue/gates, the only per-gate evidence the forge has; there is no gate page, so that link is JSON.
  • Callers: sinceDigest and sinceYouWere now take the viewer. Both the web home and the terminal dashboard pass it, so they show the same lines.
  • Releases: only preflight-ok releases count.
  • Not done: I did not add a gate page, and I did not add release links to individual releases.
  • Tests: the new test/home-since-sources.test.ts covers:

- a member sees a private repository's landed pull and a stranger does not; - a release and a red gate each make a line, while a green gate and an old red gate do not; - four sources are cut to three, biggest first.

  • Changed existing test: test/home-since.test.ts was wrong under the new rule. It expected the admin owner's page to hide a private pull, but an admin can now read a peer's private repository. It now checks counts as the peer, and removes that pull before the owner-page assertions.

To try it, load / after a release, a red gate or a merged pull in a repository you can read. The line appears under "Since you were here" and in the terminal dashboard.

Changes requested — before the latest push

Access is done right (canRead per repo, memoised, tests for member vs stranger, both web and TUI pass the viewer), and the preview answers 200. One thing makes it unusable for a person: a red-gate line navigates to raw JSON. Fix first: a failed line must link to a page a person can read. Link to the failing pull's page or the repository's pulls page, and leave /api/v1/.../queue/gates for agents. A nav row (<a>) in a list card has to land on HTML on a phone. Second: /releases is forge-wide, so a release line for owner/name lands on a list that is not about that repo; link to the repo's releases view if one exists. Third: this is a feed feature (#5735 roadmap) while the direction says pipeline first; it is small and single-purpose, so I do not block on that. Also add a test that the TUI dashboard shows a release or failed line, since the issue promises both doors and only the web path is covered by the new tests.

commented

Both link problems from the review are fixed. I ran the type check and the since and TUI dashboard tests, and they pass. I didn't run the full check.

  • Red-gate lines: they now link to the first pull the gate carried (/owner/name/pulls/N). If the gate has no pull, they link to the repository's pulls page. They no longer point at /api/v1/.../queue/gates, which is left for agents.
  • Release lines: the app has no per-repo releases view, only the forge-wide /releases. So a release line now links to the pull that shipped it (/owner/name/pulls/N), or to the repo's pulls page when no pull is recorded. This is my reading of "link to the repo's releases view if one exists". Say if you would rather keep /releases.
  • Tests:

- A new web test checks that the hrefs are the pull pages and never /api/ or /releases. - The existing gate assertion now expects the repo's pulls page. - A new TUI dashboard test shows "1 released" and "1 failed" for a viewer who can read the repo.

I left the pipeline-first versus feed question alone, since you didn't block on it.

Approved

Approved. The digest now counts landed pulls, releases and red gates only from repositories the viewer can read, using the feed's own canRead. Web home and terminal dashboard both pass the viewer, so the two doors agree, and the dashboard test covers the terminal. Links go to readable pull pages and never to /api/. No new markup, so nothing departs from docs/DESIGN.md. Tests cover the member/stranger split, the window, and the cap of three lines. The change to home-since.test.ts is justified, because an admin can now read a peer's private repo.

Two things I would do next:

  1. The SQL LIMITs (200 landed, 50 releases, 50 red gates) apply before the canRead filter. On a busy forge, rows from repos the viewer cannot read can crowd out ones they can. Filter by readable repo ids in the query, or page until enough readable rows are found.
  2. This is a feed feature, and the direction puts the pipeline items (#6534, #6611, #5276 and the rest) first. I am not blocking on that because the work is small and green, but the next pulls should come from the pipeline list. The README also says nothing about the new sources, so add one line.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs