Separate accounts from repositories, and give an account a role - #120
Merged
Merged
Conversation
The Settings screen labelled "Repositories" is the SCM account registry, and the one labelled "Webhooks" is the screen that lists repositories. The 409 from POST /api/runs sends an operator to "Settings -> Providers", which is in no nav, to set a role the form cannot set. The design renames the three screens (Accounts with Machine accounts and People tabs; Repositories), adds a role field that is fixed after registration, shows on every repository row which account reviews it and which pushes, and adds one read-only endpoint that answers that question with the same resolvers the pipeline uses. No new tables. Out-of-scope items are listed with the reason each stays out.
Every settings screen consumes ?edit=<id> through useEditDeepLink, so the Manage-on-Context link can carry it and the redirects must keep it. Also lists the attention bell's label map and the one existing test that asserts an editable role, which the design reverses.
Thirteen tasks: role fixed after registration, the serving-accounts endpoint, screen-name strings, UI types, routes and redirects, the Accounts tabs, a split of the accounts screen, the list, the form, the Repositories chips, the Context column, docs, and mutation checks.
A PUT that names a different role is refused with 409. Changing a role re-purposes one token under the other authority set: the review path would post as the push identity, or the factory would push as the bot. A role-less PUT keeps the stored role, as before; an equal one is a no-op. Registering a new account and deleting the old one is the way to change a role, and the refusal says so.
The status alone would stay green if the message moved off the response body, which is the one thing the refusal exists to carry.
GET /api/providers/serving?type=&workspace= reports the reviewer and the factory registration for one forge and workspace in one of five states: ok, no-identity, no-login, disabled, missing. The factory answer comes from MachineAccounts.resolve, the filter the dispatch path applies, so the screen that shows it and the 409 that refuses a run cannot disagree. Admin-only, like every registry read; built from the view, so it can carry no secret.
A reviewer whose token named no login but did resolve an account id is a recognisable bot. The existing rows set both fields or neither, so a check on the wrong field passed every test.
Server messages and attention links named Settings -> Providers and Settings -> Webhooks. The nav calls the first Repositories and the dashboard is about to call it Accounts and the second Repositories. Every string now points at the screen the operator will find.
ProviderView and ProviderInput gain role; ServingAccounts mirrors the new endpoint. Two pure modules read them: roleLabel treats an unlisted role as unknown rather than as a reviewer, and servingChip maps the five states to the review pills' tones with an unlisted state grey, never green.
One sentence said FACTORY and the next said Factory. The operator is being sent to a form whose field reads Factory, so both now do.
The bot-token registry was labelled Repositories and the per-repository webhook registry was labelled Webhooks. The first is now Accounts, with Operators as its People tab; the second is Repositories. The three old routes redirect and keep their query, because ?edit=<id> is what opens the record an attention row names. The bell labels both old and new paths so a row from a not-yet-upgraded service still reads correctly.
Machine accounts hold a token and a role; people are operators the identity provider knows. They share a page, never a table. The strip is plain anchors so a screen rendered without a router keeps working.
A mechanical extraction ahead of the changes the next two tasks make to the table and the form. No behaviour changes; the existing tests pass unchanged.
Forge accounts show Kind, Role, the login the forge knows them by and the workspace they serve; reviewer-only columns are dashes on a Factory row. Tracker and knowledge accounts appear read-only after them with a link to Context, so one screen answers who acts as what while two registries still back it.
The form gains a Role field: Reviewer by default, Factory for the machine account the M2 factory needs and could not be registered from the UI. On edit the stored role is shown read-only and sent back unchanged. A Factory account has no allowlist and no conversation level, so those fields are not offered for it. The allowlist asks for the stable user id: /fix accepts ids only, and the field said username.
The Repositories screen asks the orchestrator which accounts serve each row's forge and workspace, once per distinct pair, and renders both roles as chips: green only when usable, amber for a registration that exists but will not work, grey for disabled, missing or unknown. Each chip verifies with its own account. The picker offers reviewer accounts only and says that nothing about accounts is stored on the row.
Hiding a field and clearing it from the payload are different things, and only the first was tested. The test now fills both reviewer fields before choosing Factory and asserts they are gone.
The effect ran on every array identity, so each save or delete re-fetched and flashed every chip to loading, and a caller passing an inline array would loop. It now runs when the set of forge and owner pairs changes. A chip whose account changed drops the verify result it no longer describes.
Every source is read by one consumer today, the reviewer. The column makes that visible beside the account that reads it.
The runbook, the README and the roadmap named screens the nav no longer has. The smoke test's factory step registers the machine account from the UI now that the form has a Role field. The register dialog said Settings -> Providers too.
The row described the webhook registry by its old screen name.
Two screens still sent operators to Settings -> Operators, a nav entry that no longer exists. A Verify whose account changed while the request was in flight could write "reachable" beside a different account; the request is now numbered and a stale answer is dropped. The Accounts page showed its empty state for one render when only tracker accounts existed, because loading was cleared before the second fetch. fetchServingAccounts gains the URL-construction test every api function here has, with a nested-group owner so the encoding is pinned.
The serving endpoint answered "can this factory account push" by resolving the account, which decrypts the push credential on every Repositories page load and turns an undecryptable ciphertext into a 500. The one rule behind that resolution is that a login exists; it is now a static predicate on MachineAccounts, applied to the view the endpoint already holds, so dispatch and display still share one rule. The endpoint also refuses an unknown forge type with 400 instead of a confident "missing", and the two renamed attention actions are asserted.
.env.example, four runbook lines and two roadmap lines still named the Providers and Webhooks screens. Two techdebt entries record the size overage of the two Settings screens this branch grew and the constant Used-by cell; the orchestrator size entry gains ProviderRegistry.
The review state file records what round 1 of the four-lens review found and how each item was closed or dismissed. The status snapshot names the Accounts and Repositories screens and carries the counts measured after every module was re-run.
artyomsv
marked this pull request as ready for review
September 7, 2026 22:36
When only the gateway's and the worker's sessions had lapsed, the dashboard re-logged them in one prefix per page load, each a full navigation back to the root: one blink per prefix, ending on Reviews. The chained login the cold sign-in already uses serves this case too, so the browser paints once. The route the operator was on is kept in the browser across the jump and restored on return; the server still never redirects to a client-supplied address.
Rows on Accounts were three lines tall because the connection cell stacked the login, the date and the time, and the kind wrapped. The Repositories table stacked a Verify button under each chip and let the payload URL take the width. Cells no longer wrap, the last-check badge shows the date and keeps the time in its tooltip, the URL truncates with the full path on copy, and the chips stand alone: what they show is which account serves the row, and verifying happens where the account or the webhook is edited.
The Manage-on-Context link wrapped onto three lines in a squeezed last column, which is what made the tracker rows tall. Long identities and check failures now truncate with the full text on hover. Accounts drops to eight columns: enabled is a dot by the name, and the two reviewer-only settings share one Policy cell. Repositories drops the Secret column, which always read "secret set", and shows both serving accounts in one labelled cell.
The note said where the secret comes from and forgot to say what to do with it. A webhook wired with the URL alone fails signature checks.
The table outgrew the page. Three cells held values with no bound on their length — the base URL, the bot identity and the workspace — and a fourth repeated the same category word on every row. Long values are now truncated to the column and carry the whole value in their tooltip and on a copy button, which is what an operator does with them: paste them into a forge's own portal. The kind is an icon with the word as its accessible name and its tooltip, so Forge, Tracker and Knowledge stop spending a column on three repeated strings. A rejection message from a provider is held to one line at a tighter width.
The Accounts table still ran off the page. Two cells were the reason. The connection cell printed the login the token authenticated as, the date of the last check, and on a refusal the provider's own message — three variable-length strings, the longest of them a paragraph. It is now one badge in four states: OK, Failed, Not checked, Checking. All three strings moved to its tooltip, which is where an operator reads them once rather than down a column of eleven rows. A tracker's badge reports the stored standing and is not a control: nothing is checked from this screen, that happens on Context. The tracker row's "Manage on Context" was a button of words two icons wide, and squeezed it wrapped, which is what made those rows taller than the forge rows above them. It is the shared icon button now, lining up with Edit and Delete, with its name on the hover and as its label.
Three cells still spent width on words. The connection badge carried its state as a label beside a dot, the Policy cell was a sentence — "0 ids · Inherit (global)" — and the two copyable values were bounded a quarter wider than they need to be. The connection is one icon in four shapes and colours: a tick, a cross, a dashed circle, a spinner. The word is the icon's accessible name and the head of its tooltip, so nothing is lost to a screen reader or to a hover, and a column of eleven rows is read by shape. Policy is a head-count icon with a number and the conversation level without the parenthesis the form needs, each with its own sentence on hover. Identity and Scope truncate at 150px instead of 200.
The walkthrough fix wave moved information out of visible text and into icons, tooltips and copy buttons three times over. Review found where that lost something. A disabled account reported "Not checked" while the registry held a rejected token, because the badge hard-coded idle for anything not enabled; both badges now derive the stored standing the same way, and a live result no longer outlives the account that earned it. The enabled dot and the two policy marks named a bare span, which the generic ARIA role cannot carry, so they had no accessible name at all. The connection button was named after its state and never its action. An unknown conversation level read as "Inherit" because the short form compared a rendered label instead of the level. The payload URL's width bound sat on a table cell, where every browser ignores it. The style contract now reads a class out of a template literal and excuses nothing by prefix, which caught prov-error: asked for twice by the General screen and defined nowhere since it was written. Five behaviours no test reached have tests, the accounts table's cells moved into their own file under the size cap, and the return-route guard refuses a protocol-relative hash itself rather than trusting the router.
This was referenced Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Separate who acts from where it acts in Settings, and add the one field the M2 factory needs and the UI cannot set: an account's role.
The screen labelled Repositories is the SCM account registry (one bot token per forge and workspace). The screen labelled Webhooks is the one that lists repositories.
POST /api/runstells an operator to register the machine account "under Settings -> Providers", a screen that does not exist in the nav. This PR renames the three screens, adds the role field, and shows on each repository row which account reviews it and which pushes.Design:
docs/superpowers/specs/2026-09-07-accounts-and-roles-design.md. Plan:docs/superpowers/plans/2026-09-07-accounts-and-roles.md./settings/providers)/settings/accounts), tabs Machine accounts and People/settings/operators)/settings/webhooks)/settings/repositories)No new tables. No migration. One new read-only endpoint. Old routes redirect and keep
?edit=.Checklist
Design spec
Implementation plan (13 tasks)
T1 Backend: an account's role is fixed after registration; a changing
PUTgets 409 (§6.2)T2 Backend:
GET /api/providers/serving— which accounts serve a (forge, workspace), five states (§6.1)T3 Backend + gateway: every message and attention link names a screen that exists (§6.3)
T4 UI:
ProviderView.role/botUsername,ProviderInput.role,ServingAccountstypes; pure helpers (§6.4)T5 UI: routes, redirects preserving the query string, nav rail, titles, bell labels (§7)
T6 UI: Accounts tab strip; Operators becomes the People tab (§5.1)
T7 UI: split the accounts screen into page, table and form (no behaviour change)
T8 UI: Machine accounts list — Kind, Role, Identity, Scope; tracker rows read-only (§5.1)
T9 UI: account form — Role field, fixed on edit; Factory hides reviewer-only fields; allowlist asks for the stable id (§5.1)
T10 UI: Repositories — Reviewed by / Pushed by chips with per-account Verify; picker offers reviewer accounts only (§5.2)
T11 UI: Context — Used by column (§5.3)
T12 Docs: README, ROADMAP, SMOKE-TEST, HISTORY (§10)
T13 Mutation checks: 5/5 killed by exactly the predicted tests (§9)
/code-reviewround 1 (four lenses, ref f5b6bab); 16 findings fixed in a8abcd8, 84d4f5f, bd0d998; re-review clean; dispositions in.claude/reviews/global/accounts-and-roles.mdFinal verification (2026-09-08, every module re-run): Java 324 suites / 2889 tests / 0 failures / 1 pre-existing skip (
testFast+testServices);spire-ui70 files / 529 tests;tsc --noEmitsilentPost-test fixes from the operator's walkthrough: one line per cell, chips without Verify, URL truncation (34daaa8); one chained re-login with the route restored (cc89567); eight-column Accounts, one Accounts column and no Secret column on Repositories (b9d831a, d2dee80); reviewed clean
Accounts fits the page: long values truncate with a copy button and the full value on the tooltip, the kind is an icon (5e8b608); the connection is one four-state badge and a tracker is managed from an icon button (2916b62); the connection is an icon, Policy is two marks, values truncate tighter (d7d8f2e)
/code-reviewround 2 (four lenses, ref d7d8f2e, scopee9cff10..HEAD): no HIGH from any lens; Semgrep 16 files / 0 findings. 20 findings fixed in 2d0244d — a disabled account reporting "Not checked" over a stored rejection, three cells with no accessible name, an unknown conversation level reading as "Inherit", a width bound every browser ignores on a table cell, and a style contract that could not see a class written in a template literal (widening it caught a real orphan,prov-error). Dispositions in.claude/reviews/global/accounts-and-roles.mdFinal verification (2026-09-08):
spire-ui71 files / 560 tests / 0 failures;tsc --noEmitsilent; six mutation probes, each killed by exactly the predicted testOut of scope (recorded in the spec, §11)
Tracker accounts with write rights; many accounts per workspace; push-rights verification; a "review this account's pull requests" toggle; moving the allowlist to a workspace policy.