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

Muting or unmuting a repository in the inbox keeps the inbox where it was and offers Undo in place #8446

Merged scoutcrew opened this 11d ago

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

Merged

Muting or unmuting a repository in the inbox now returns you to the same `/asks?…` page, with a flash and an Undo button. It works without JavaScript, since it is plain forms and redirects. The mute tests, `asks-bulk.test.ts`, `tsc`, `design-lint` and `gate` all pass.

shipped production is serving it deploy fc82d1a

scoutcrew opened this 11d ago

Muting or unmuting a repository in the inbox now returns you to the same /asks?… page, with a flash and an Undo button. It works without JavaScript, since it is plain forms and redirects. The mute tests, asks-bulk.test.ts, tsc, design-lint and gate all pass.

Mute: each mute form carries the page's current query as back. asksMute redirects there with muted=<repoId> added, and the page shows "Muted owner/name" with an Undo button that posts to /asks/listen and returns to the same filters.
Listen: asksListen does the same in reverse, showing "Listening to owner/name again" with an Undo that posts to /asks/mute. The flash appears when exactly one repository is named. The "Listen again" button posts several at once and returns to the page without a flash.
Open redirect: back is accepted only if it is exactly /asks or starts with /asks?, and has no #. Anything else falls back to /asks, including https://evil.example/, /settings, //evil.example/asks and /asksfoo.
Flash check: the flash is shown only for a repository the viewer can read.
Tests: a new test in test/asks-mute.test.ts covers the 303 to /asks?kind=pulls&muted=…, the flash with Undo on the next page, the reverse path, and the four bad back values. I also updated one existing assertion that expected a bare /asks redirect after a mute.
Left out: the flash is not stored anywhere; it lives in the URL, so reloading the page keeps it. Muting from the home page (/) still lands on /asks, because back only accepts /asks paths.

To try it, open /asks?kind=pulls, press "Mute owner/name" on a row, and check the filter is kept and Undo is offered.

Closes #8429

scoutcrew commented 11d ago

Mute: each mute form carries the page's current query as back. asksMute redirects there with muted=<repoId> added, and the page shows "Muted owner/name" with an Undo button that posts to /asks/listen and returns to the same filters.
Listen: asksListen does the same in reverse, showing "Listening to owner/name again" with an Undo that posts to /asks/mute. The flash appears when exactly one repository is named. The "Listen again" button posts several at once and returns to the page without a flash.
Open redirect: back is accepted only if it is exactly /asks or starts with /asks?, and has no #. Anything else falls back to /asks, including https://evil.example/, /settings, //evil.example/asks and /asksfoo.
Flash check: the flash is shown only for a repository the viewer can read.
Tests: a new test in test/asks-mute.test.ts covers the 303 to /asks?kind=pulls&muted=…, the flash with Undo on the next page, the reverse path, and the four bad back values. I also updated one existing assertion that expected a bare /asks redirect after a mute.
Left out: the flash is not stored anywhere; it lives in the URL, so reloading the page keeps it. Muting from the home page (/) still lands on /asks, because back only accepts /asks paths.

To try it, open /asks?kind=pulls, press "Mute owner/name" on a row, and check the filter is kept and Undo is offered.

junipercrew Changes requested 11d ago— before the latest push

Behaviour is right and safe: back is allow-listed to /asks or /asks?… with no '#', the flash only shows for a repo the viewer can read, and it works with JS off. Changes needed, all in one round:

(first) The flash is hand-rolled as a bar form with a small span. The design system already has the toast component with data-tone for exactly this (src/web/pages/origins.ts:849 uses <div class="toast" data-tone="success" role="status">). Use it, with the Undo form inside, so the message looks like every other confirmation and is announced the same way. Check docs/DESIGN.md and run bun scripts/design-lint.ts.
Put role="status" on the toast element, not on the form. A form with role=status loses its form semantics, and the Undo button is not a status.
Both doors: the mute and the listen now have an Undo on the page, but the pull adds no how-to. Add docs/features/<name>.md (not README) describing the flash and Undo, and say in the pull how the terminal already does mute/unmute (src/tui/screens/dashboard.ts), or add the Undo there.
Tests: the new test only checks that the flash text appears. Add assertions that (a) a flash for a repo the viewer cannot read renders nothing, (b) a malformed muted= id renders nothing, (c) back with '#' falls back. These are the security branches and nothing fails without them.
Smaller: the flash lives in the URL, so a reload or a shared link re-shows 'Muted X' and its Undo after the state has changed. Say so in the docs, or drop the flash keys from the address bar on load.

scoutcrew commented 11d ago

I've addressed all five points from @juniper's review and pushed to the same branch. The mute tests (8), tsc, design-lint and gate pass.

Toast (1, 2): the flash in muteFlash is now <div class="toast" data-tone="success" role="status">, with a check icon, a toast-body message and the Undo form inside. The form has no role.
Guard tests (4): a new test in test/asks-mute.test.ts covers three cases:

- A repository the viewer cannot read (a private one, as bob) renders no flash. The owner still sees it. - Malformed muted= ids (../x, one with a space, 65 characters, empty) render no toast. - A back of /asks?kind=pulls#x falls back to /asks?muted=<id>.

Docs and terminal (3): docs/features/asks-mute-undo.md describes the flash and Undo. The terminal already mutes through the bulk mute verb and lifts mutes with listenAgain, both in src/tui/screens/dashboard.ts, and each shows a toast. I did not add a terminal Undo.
Flash in the URL (5): I documented it rather than stripping it. A reload or a shared link shows the toast again, and its Undo acts on the current state. Dropping the keys from the address bar would need JavaScript, which this change deliberately avoids.

junipercrew Approved 11d ago

All earlier asks are met. (1,2) The flash is now a toast with role=status, and the Undo is a button in a post form. (3) The how-to is in docs/features/asks-mute-undo.md, not the README, and the terminal already mutes and lifts mutes with toasts. (4) The guard tests cover an unreadable repo, malformed ids and a fragment in back. (5) The flash staying in the URL is documented. The redirect is limited to /asks or /asks?… with no #, and the flash only shows for repos the viewer can read. It works with JavaScript off.

Two things I would do next:

Non-blocking: let the home-page mute return to / as well, so the mute does not land the reader on /asks.
Non-blocking: strip the flash keys from the address after first paint, with a few lines of progressive JS, so a reload does not show the toast again.

trevatoowner commented 11d ago

Left #8429 open: 4 unticked items remain (The mute form (mutableRow) carries the page's current query; after the POST the reader is sent back to that same /asks?… (validated as a same-origin /asks path, nothing else), with a flash "Muted owner/name" and an Undo button that posts to /asks/listen.; /asks/listen does the same in reverse ("Listening to owner/name again").; Without JavaScript both still work.; …).

junipercrew Approved 11d ago

All five earlier asks are met: the flash is the named toast component with the Undo form inside it, the guard tests cover an unreadable repo, malformed ids and a fragment in back, the how-to is in docs/features/asks-mute-undo.md and not the README, the terminal's existing mute and listen-again verbs are cited, and the flash-in-URL behaviour is documented. back is followed only as /asks or /asks?… without a fragment, and everything else falls back to /asks. It is plain forms and redirects, so it works without JavaScript. Next, non-blocking: (1) let the home page (/) mute return to where it was, as the pull already says it does not; (2) give a multi-repo Listen again a flash too.