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

Take it back: the studio's verb on a shipped change #190

shipped production is serving it opened this has it
opened this

The second half of parked #92, on top of the revert action. A person looking at a change that shipped and made things worse has one word for it, on the page they are already on.

Done looks like

  • [ ] A shipped change's studio page offers Take it back to someone with write access, behind the studio's existing confirm, which names the change and says plainly what will happen (a new change that undoes it, for a person to merge).
  • [ ] Pressing it calls the revert action and lands on the new change's studio page, where its check and its preview run as they do for any change.
  • [ ] A refusal — conflict, already reverted, not a shipped change — is said in the studio's words on the page, and nothing is created.
  • [ ] Same-origin on the mutation and the write check in the action, not only in the page that draws the button; a reader never sees the button and a forged post never works.
  • [ ] The button does not appear on a change that is still open, and dev's pages are untouched.
  • [ ] Tests: the button renders for a writer on a shipped change and not for a reader; the POST from another origin is refused; a successful take-back redirects to the new change. They fail on today's code.

Files I expect to change

src/web/studio/change.ts (the button), src/web/studio/actions.ts (the POST), src/http/server.ts (one route), the confirm table in src/web/pages/search.ts, test/studio.test.ts.

Risks

This is a destructive-looking verb on a page full of safe ones: it must read as reversible (it opens a change; it does not rewrite anything) and it must not sit where Ship it was a moment ago. Keep the page's render free of git work — the action does the computing, the page only offers the verb.

Suggested: Scout.

commented

Parked by the crew: the check failed and the decider saw no point in another try (pull #235 closed). Planner: split this smaller, or rewrite what done looks like; a person can also say what to try.

commented

Parked by the gate, not by the work. Pull #235 was written, complete and tested — the Take it back card, the \/-/confirm\ step that reads the change's real title from the database, the same-origin and write checks, the refusal wording, and six tests in \test/studio.test.ts\ — and the check that failed it is red on main before any change is made. I measured that on an unmodified export of origin/main in a sandbox like the one the crew runs in: test/pulls.test.ts fails with git push origin main: remote: auth required, and studio + apps + web together give 209 pass, 4 fail — one more push-auth failure and three assertions, one of them #225's own test.

So this issue is not re-scoped: what done looks like above stands, and the branch from #235 is the starting point — read it before writing anything.

Two things are added to it:

  • [ ] It waits on #233 (the check passing on main where the crew runs). Taking it before that means meeting the same wall a third time.
  • [ ] Before pushing, run the repository's own check command. If something fails, the pull names the failing tests and proves they fail identically on the base commit — that evidence is what makes a red check reviewable instead of a park.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs