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

An admin renames or recolours a label from the terminal's Labels screen with `e` #8017

Merged opened this

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

Merged

On the terminal's Labels screen, `e` on the selected label now opens the same form `n` uses, with the name and colour prefilled as `name hex`. I ran `bun x tsc --noEmit`, `scripts/gate.ts`, `test/tui-labels.test.ts` and `test/mcp-size.test.ts`, and all passed. I did not run the full suite; the forge's check does that on the push.

shipped production is serving it deploy ef63cd6

opened this

On the terminal's Labels screen, e on the selected label now opens the same form n uses, with the name and colour prefilled as name hex. I ran bun x tsc --noEmit, scripts/gate.ts, test/tui-labels.test.ts and test/mcp-size.test.ts, and all passed. I did not run the full suite; the forge's check does that on the push.

  • Saving: the form calls updateLabel and reloads the list in name order. A leading # on the colour is accepted, and the colour is lowercased.
  • Refusals: a taken name or a bad colour shows updateLabel's words on the form and keeps what was typed. If the label was deleted underneath, the form says LABEL_WORDS.gone. If the user was demoted mid-edit, it says "Only a repository admin can make a label." (the existing NOT_ALLOWED_LINE, which now covers edits too).
  • Who gets e: it is listed and bound only for a repository admin, as n and d are. A reader gets no e and the key does nothing.
  • Form helper: I reused promptScreen and the name-and-colour parsing that n already has, and added no second form. The parsing is now a shared parse, and the refusal wording a shared refusal.
  • Tests: test/tui-labels.test.ts gains three. They cover an admin renaming and recolouring a label that an issue carries, a taken name refused in place, and a reader with no e. The first checks the form is prefilled, that the label keeps its id, and that the issue still shows it under the new name. I did not run them against the old code to watch them fail.
  • MCP: nothing touched.

To try it, open a repository you administer, go to Labels, move to a label and press e.

Closes #7868

commented

On the terminal's Labels screen, e on the selected label now opens the same form n uses, with the name and colour prefilled as name hex. I ran bun x tsc --noEmit, scripts/gate.ts, test/tui-labels.test.ts and test/mcp-size.test.ts, and all passed. I did not run the full suite; the forge's check does that on the push.

  • Saving: the form calls updateLabel and reloads the list in name order. A leading # on the colour is accepted, and the colour is lowercased.
  • Refusals: a taken name or a bad colour shows updateLabel's words on the form and keeps what was typed. If the label was deleted underneath, the form says LABEL_WORDS.gone. If the user was demoted mid-edit, it says "Only a repository admin can make a label." (the existing NOT_ALLOWED_LINE, which now covers edits too).
  • Who gets e: it is listed and bound only for a repository admin, as n and d are. A reader gets no e and the key does nothing.
  • Form helper: I reused promptScreen and the name-and-colour parsing that n already has, and added no second form. The parsing is now a shared parse, and the refusal wording a shared refusal.
  • Tests: test/tui-labels.test.ts gains three. They cover an admin renaming and recolouring a label that an issue carries, a taken name refused in place, and a reader with no e. The first checks the form is prefilled, that the label keeps its id, and that the issue still shows it under the new name. I did not run them against the old code to watch them fail.
  • MCP: nothing touched.

To try it, open a repository you administer, go to Labels, move to a label and press e.

Changes requested — before the latest push

Authorization is right: updateLabel re-checks requireRepoAdmin on every write, e is bound and listed only for admins, and the web viewers get the label.updated event. Two things to fix in one round. (1) Two behaviours you added have no test: the gone branch (label deleted underneath the open form, or the admin demoted mid-edit, which should show NOT_ALLOWED_LINE) and the state.clamp after saving. Add a test that deletes the label while the form is open and submits, and one that demotes the actor and submits; both must fail without the branches. (2) NOT_ALLOWED_LINE reads "Only a repository admin can make a label." and is now shown for edits; use wording that fits both, or pick the line per action. Also say in the README how to edit a label from the terminal Labels screen (e), next to where n and d are described, if they are.

commented

I pushed the fixes for both points, except that I dropped the state.clamp after saving instead of testing it. tsc, scripts/gate.ts and test/tui-labels.test.ts pass.

  • New tests: I added two tests for the gone and demoted paths.

- Deleted underneath: the label is deleted while the form is open, then the form is submitted. It shows "That label no longer exists." and the list stays empty. - Demoted: the actor is demoted mid-edit, then the form is submitted. It shows NOT_ALLOWED_LINE, and the label is unchanged. - Mutation check: I checked the first test fails when the not_found && admin() branch is removed. I did not run that check for the demoted test.

  • Wording: NOT_ALLOWED_LINE now reads "Only a repository admin can change a label." That fits both creating and editing, so there is no per-action line.
  • Docs: there is no terminal section in the README that describes n and d. They are described in docs/APPS.md, so that is where I added e. It now says e opens the same form prefilled, a refusal keeps what was typed, and a reader is offered none of n, e or d.
  • The clamp: a rename never changes how many labels there are, so the state.clamp after saving could never do anything, and no test could make it fail. I removed it rather than write a test that could not fail without it.
Approved

Approved. e is bound and listed only for repo admins, goes through updateLabel (which re-checks admin and emits label.updated to web viewers), reuses the n form and keeps typed text on refusal. The gone and demoted paths now have tests, and the wording and APPS.md are updated. Next: (1) rename rename to edit, since it also recolours; (2) skip the write when name and colour are unchanged, so no spurious label.updated event goes out.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs