chadtmiller.com/knotless
git that lives in your Atmosphere account
1
10
A branch's gates are named and taken out on its settings page
.changeset/review-gates-settings.mdnew+7−0
| @@ -0,0 +1,7 @@ | ||
--- | ||
default: minor | ||
--- | ||
| ||
# A branch's gates are named on its settings page | ||
| ||
A protected branch's settings on the pages now have "Wait for a review from", which names an account, such as a review agent, as a gate of the branch by its handle. The page lists each gate, with Remove to take it out, as `knotless protect --gate` does. | ||
crates/knotless-serve/src/web/settings.rs+13−0
| @@ -69,6 +69,10 @@ pub struct Settings { | ||
/// do not push to it. | ||
#[serde(skip_serializing_if = "Option::is_none")] | ||
readers: Option<Vec<ActorView>>, | ||
/// The accounts its rules name as gates, each once, in the order | ||
/// they are first named: for the pages to show by handle. | ||
#[serde(skip_serializing_if = "Vec::is_empty")] | ||
gates: Vec<ActorView>, | ||
} | ||
| ||
/// The repository `repo` names, as a URL by its owner's DID. | ||
| @@ -106,6 +110,14 @@ pub async fn get_repo_settings( | ||
let record = repo.record(&app).unwrap_or(repo.synced.clone()); | ||
let collaborators = record.collaborators.iter().map(|c| actor(&app, &c.did)); | ||
let collaborators = futures::future::join_all(collaborators).await; | ||
let mut gated: Vec<&str> = Vec::new(); | ||
let named = (record.rules.iter()).filter_map(|r| r.reviews.as_ref()); | ||
for gate in named.flat_map(|r| &r.gates) { | ||
if !gated.contains(&gate.did.as_str()) { | ||
gated.push(&gate.did); | ||
} | ||
} | ||
let gates = futures::future::join_all(gated.into_iter().map(|did| actor(&app, did))).await; | ||
let mut triagers = Vec::new(); | ||
for t in record.triagers { | ||
triagers.push(TriagerView::new(&app, t).await); | ||
| @@ -140,6 +152,7 @@ pub async fn get_repo_settings( | ||
storage, | ||
delegated, | ||
readers, | ||
gates, | ||
})) | ||
} | ||
| ||
crates/knotless-serve/tests/pages.rs+10−0
| @@ -595,6 +595,16 @@ fn owners_manage_branches_labels_and_access() { | ||
assert_eq!(rules, json!({ "rules": held })); | ||
let set = server.settings(&owner.account.did, "managed"); | ||
assert_eq!(set["rules"], held); | ||
assert_eq!(set.get("gates"), None); | ||
// Its gates, named twice, are shown once, as accounts. | ||
let gate = json!({ "did": other.account.did }); | ||
let reviews = json!({ "gates": [gate, gate] }); | ||
let gated = with(json!({"branch": "main", "reviews": reviews})); | ||
assert_eq!(server.call("protectBranch", me, gated).0, 200); | ||
let set = server.settings(&owner.account.did, "managed"); | ||
let gates = set["gates"].as_array().unwrap(); | ||
assert_eq!(gates.len(), 1, "{set}"); | ||
assert_eq!(gates[0]["did"], other.account.did.as_str()); | ||
let main = with(json!({"branch": "main"})); | ||
assert_eq!(server.call("protectBranch", not_me, main.clone()).0, 403); | ||
let (status, rules) = server.call("unprotectBranch", me, main.clone()); | ||
docs/design/serve.md+4−1
| @@ -936,7 +936,10 @@ and default branch (`knotless describe`), renames it, shows what it takes up and | ||
compacts it, and has a Danger zone to offer it to another account or | ||
withdraw the offer, and to delete it once its full name is typed. | ||
Branches makes, renames and deletes branches and protects them, with or | ||
without merge commits (`knotless branch`, `protect`); Labels defines, | ||
without merge commits, the merge queue and reviews, and names and takes | ||
out the accounts a branch waits for a review from, its | ||
[gates](collaboration.md#gates), by handle (`knotless branch`, | ||
`protect`); Labels defines, | ||
changes and takes away labels (`knotless labels`); Access adds and removes | ||
collaborators (`knotless share --write`) and triagers, by scope | ||
(`knotless triagers`), and for a private repository its readers | ||
docs/reference/api.md+1−1
| @@ -575,7 +575,7 @@ name it has now. | ||
| `dev.knotless.getAccountData` | what knotless data the account logged in to the pages holds, read from its PDS: `{did, collections, repos, spaces}`, each `dev.knotless.*` collection it writes in as `{collection, count}`, its repositories as `{name, private, collaborators, members?}` (`members`, for a private one, the accounts that read it but its runners), and the spaces of its private repositories; the pages' cookie only, 401 otherwise | | ||
| `dev.knotless.getEmails` | `{count}`, how many addresses the asker's `dev.knotless.emails` claims: only hashes are kept, so they cannot be listed. 401 with no asker | | ||
| `dev.knotless.listBlocks` | the asker's blocks, the latest first, from its own repo: each `uri`, `subject` (an actor), `repo` when it is from one repository only, and `createdAt`. Those naming a private repository live in its space and are not listed. 401 with no asker | | ||
| `dev.knotless.getRepoSettings?repo=` | what the owner sets: description, links, default branch, protected branches, labels, collaborators, triagers, gateway, an offer to move it (`transfer`), `storage`, as `knotless storage` counts it, the gateway the owner delegated to from these pages (`delegated`), and for a private one, to its owner alone, the members who read but do not push (`readers`) | | ||
| `dev.knotless.getRepoSettings?repo=` | what the owner sets: description, links, default branch, protected branches, labels, collaborators, triagers, gateway, an offer to move it (`transfer`), `storage`, as `knotless storage` counts it, the gateway the owner delegated to from these pages (`delegated`), for a private one, to its owner alone, the members who read but do not push (`readers`), and the accounts its rules name as [gates](../design/collaboration.md#gates), each once (`gates`) | | ||
| `dev.knotless.getOrg?actor=` | an organization's members, as the index has its `dev.knotless.org` and `dev.knotless.orgMember` records and its members' `dev.knotless.orgAcceptance` ones: `{org, organization, inside, gateway?, members}`, `gateway` the one its public `dev.knotless.org` record names, each member `{actor, role, standing, invitedAt, expiresAt?, joinedAt?}`, the first invited first, `standing` one of `invited`, `lapsed`, `public` and `hidden`. To anyone, its members in public; to the organization and its members, logged in or with a service token (`inside`), its invitations and hidden members too, from its space, which this server keeps an index of; to an account it invited, its own entry. `organization: false`, with no members, for an account that is none; `ProfileNotFound` for an unknown account | | ||
| `dev.knotless.getActorStars?actor=` | the public repositories an account starred, newest first, each with its `language`, `starCount` and `forkCount`: 404 with no index, `ProfileNotFound` for an unknown account | | ||
| `dev.knotless.getMergeStatus?repo=&pull=` | whether an open pull can merge now, as `knotless merge --check` says ([below](#landing-and-checks)): 401 with no asker, 400 for a pull not open, 404 for none | | ||
harness/e2e/flows.spec.mjs+29−0
| @@ -540,6 +540,35 @@ test('making, protecting, renaming and deleting a branch', async () => { | ||
await anew(page, (expect) => expect(row('topical')).toHaveCount(0)) | ||
}) | ||
| ||
test('naming and removing a gate of a branch', async () => { | ||
await go(page, '/alice.test/tally/settings/branches') | ||
await page.getByLabel('New branch').fill('gated') | ||
await page.getByRole('button', { name: 'Create', exact: true }).click() | ||
const row = (page) => page.locator('li', { has: page.getByRole('link', { name: 'gated', exact: true }) }) | ||
await row(page).getByRole('button', { name: 'Protect' }).click() | ||
await expect(row(page)).toContainText('protected') | ||
await row(page).getByLabel('Wait for a review from').fill('bob.test') | ||
await row(page).locator('form', { has: page.getByLabel('Wait for a review from') }).getByRole('button', { name: 'Add' }).click() | ||
const gate = (page) => row(page).locator('.gates li', { hasText: 'bob.test' }) | ||
await expect(gate(page)).toBeVisible() | ||
await anew(page, (expect) => expect(gate(page)).toBeVisible()) | ||
// The rule in alice's record names bob by DID. | ||
const rule = async () => { | ||
const repo = (await records('alice', 'dev.knotless.repo')).find((r) => r.uri.endsWith('/tally')) | ||
return repo.value.rules.find((r) => r.branch === 'gated') | ||
} | ||
expect((await rule()).reviews.gates).toEqual([{ did: who.bob.did }]) | ||
await gate(page).getByRole('button', { name: 'Remove' }).click() | ||
await expect(gate(page)).toHaveCount(0) | ||
await anew(page, (expect) => expect(gate(page)).toHaveCount(0)) | ||
expect((await rule()).reviews).toBeUndefined() | ||
await row(page).getByRole('button', { name: 'Unprotect' }).click() | ||
await expect(row(page).getByRole('button', { name: 'Protect' })).toBeVisible() | ||
page.once('dialog', (d) => d.accept()) | ||
await row(page).getByRole('button', { name: 'Delete' }).click() | ||
await expect(row(page)).toHaveCount(0) | ||
}) | ||
| ||
test('adding and removing a collaborator', async () => { | ||
await go(page, '/alice.test/tally/settings/access') | ||
const add = () => page.locator('form', { has: page.getByLabel('Add a collaborator') }).getByRole('button', { name: 'Add' }).click() | ||
lexicons/dev/knotless/getRepoSettings.json+1−0
| @@ -33,6 +33,7 @@ | ||
"storage": { "type": "ref", "ref": "#storageView", "description": "What it takes up on its owner's PDS; absent if that could not be read." }, | ||
"indexed": { "type": "boolean", "description": "Deprecated, and no longer set: knotless.dev indexes every private repository its members read there. Was, for a private repository, whether its owner let this appview keep an index of it." }, | ||
"delegated": { "type": "string", "format": "did", "description": "The gateway its owner delegated writes to from this appview's pages (dev.knotless.delegateWrites), which sharing it for writing names as its gateway; absent until they have." }, | ||
"gates": { "type": "array", "description": "The accounts its protected branches' rules name as gates (dev.knotless.repo#reviews), each once: for the pages to show by handle. Absent when there are none.", "items": { "type": "ref", "ref": "dev.knotless.defs#actorView" } }, | ||
"readers": { "type": "array", "description": "For a private repository, asked by its owner, the members who read it but do not push to it, its runners among them; absent for a public one, which anyone reads, and empty to anyone else.", "items": { "type": "ref", "ref": "dev.knotless.defs#actorView" } } | ||
} | ||
} | ||
web/src/lib/organisms/SettingsBranches.svelte+47−3
| @@ -1,5 +1,6 @@ | ||
<script lang="ts"> | ||
import type { RepoViewDetailed } from '#lib/lexicons/dev/knotless/defs.ts'; | ||
import type { ActorView, RepoViewDetailed } from '#lib/lexicons/dev/knotless/defs.ts'; | ||
import type * as getProfile from '#lib/lexicons/dev/knotless/getProfile.ts'; | ||
import type { Reviews, Rule } from '#lib/lexicons/dev/knotless/repo.ts'; | ||
import type * as createBranch from '#lib/lexicons/dev/knotless/createBranch.ts'; | ||
import type * as deleteBranch from '#lib/lexicons/dev/knotless/deleteBranch.ts'; | ||
| @@ -7,17 +8,26 @@ | ||
import type * as renameBranch from '#lib/lexicons/dev/knotless/renameBranch.ts'; | ||
import type * as unprotectBranch from '#lib/lexicons/dev/knotless/unprotectBranch.ts'; | ||
import { invalidateAll } from '$app/navigation'; | ||
import { call } from '#lib/data.ts'; | ||
import { ask, call } from '#lib/data.ts'; | ||
import { repoHref, urlPath } from '#lib/format.ts'; | ||
import type { AtIdentifierString, DidString } from '@atproto/lex'; | ||
import Avatar from '#lib/atoms/Avatar.svelte'; | ||
import Icon from '#lib/atoms/Icon.svelte'; | ||
import Empty from '#lib/molecules/Empty.svelte'; | ||
import HandleInput from '#lib/molecules/HandleInput.svelte'; | ||
import Person from '#lib/molecules/Person.svelte'; | ||
/** | ||
* The branches of `repo`, owned as `authority` names it, the default | ||
* first: made, renamed and deleted as `knotless branch` does, and held | ||
* by `rules`, the protected ones, as `knotless protect` holds them. | ||
* `gates` are the accounts the rules name as gates, to show by handle. | ||
*/ | ||
let { repo, authority, branches, defaultBranch, rules }: { repo: RepoViewDetailed; authority: string; branches: string[]; defaultBranch?: string; rules: Rule[] } = $props(); | ||
let { repo, authority, branches, defaultBranch, rules, gates = [] }: { repo: RepoViewDetailed; authority: string; branches: string[]; defaultBranch?: string; rules: Rule[]; gates?: ActorView[] } = $props(); | ||
const rule = (name: string) => rules.find((r) => r.branch === name); | ||
/** The account a rule names as a gate by `did`, as the settings show it. */ | ||
const gate = (did: string): ActorView => gates.find((g) => g.did === did) ?? ({ did, handle: did } as ActorView); | ||
// What is typed to add as a gate, by branch. | ||
let adding = $state<Record<string, string>>({}); | ||
let name = $state(''); | ||
let from = $state(''); | ||
let renaming = $state(''); | ||
| @@ -65,6 +75,27 @@ | ||
const rule = { repo: repo.uri, branch: held.branch, merges: held.history === 'merges', queue: change.queue ?? !!held.queue, reviews: asks ? reviews : undefined }; | ||
doing(`rule ${held.branch}`, () => call<typeof protectBranch.main>('dev.knotless.protectBranch', rule)); | ||
} | ||
/** Names whoever is typed for `held`'s branch as a gate of its rule, found by handle or DID. */ | ||
async function addGate(e: SubmitEvent, held: Rule) { | ||
e.preventDefault(); | ||
const who = (adding[held.branch] ?? '').trim(); | ||
const named = held.reviews?.gates ?? []; | ||
const added = async () => { | ||
const found = await ask<typeof getProfile.main>('dev.knotless.getProfile', { actor: who as AtIdentifierString }); | ||
if (named.some((g) => g.did === found.did)) return; | ||
const reviews = { ...held.reviews, gates: [...named, { did: found.did }] }; | ||
await call<typeof protectBranch.main>('dev.knotless.protectBranch', { repo: repo.uri, branch: held.branch, merges: held.history === 'merges', queue: !!held.queue, reviews }); | ||
}; | ||
if (await doing(`gate ${held.branch}`, added)) adding[held.branch] = ''; | ||
} | ||
/** Takes the gate `did` out of `held`'s rule. */ | ||
function removeGate(held: Rule, did: DidString) { | ||
const left = (held.reviews?.gates ?? []).filter((g) => g.did !== did); | ||
const reviews = { ...held.reviews, gates: left }; | ||
const asks = !!(reviews.approvals || reviews.codeOwners || reviews.dismissStale || left.length); | ||
const rule = { repo: repo.uri, branch: held.branch, merges: held.history === 'merges', queue: !!held.queue, reviews: asks ? reviews : undefined }; | ||
doing(`ungate ${held.branch} ${did}`, () => call<typeof protectBranch.main>('dev.knotless.protectBranch', rule)); | ||
} | ||
/** How many approvals a branch may ask for, as the select offers them. */ | ||
const counts = [0, 1, 2, 3, 4, 5, 6]; | ||
function unprotect(branch: string) { | ||
| @@ -113,6 +144,18 @@ | ||
<label class="check"><input type="checkbox" checked={!!held.reviews?.dismissStale} disabled={!!busy} onchange={(e) => again(held, { reviews: { dismissStale: e.currentTarget.checked } })} />Only approvals of the latest push count</label> | ||
{#if busy === `rule ${b}`}<span class="mute" role="status">Saving…</span>{/if} | ||
</div> | ||
<form class="form row-form" onsubmit={(e) => addGate(e, held)}> | ||
<label for="gate-{b}">Wait for a review from</label> | ||
<div class="owned"><HandleInput id="gate-{b}" name="gate" autocomplete="off" bind:value={() => adding[b] ?? '', (v) => (adding[b] = v)} /><button class="btn" type="submit" disabled={!!busy} aria-busy={busy === `gate ${b}`}>{busy === `gate ${b}` ? 'Adding…' : 'Add'}</button></div> | ||
{#if held.reviews?.gates?.length} | ||
<ul class="gates"> | ||
{#each held.reviews.gates as g (g.did)} | ||
{@const a = gate(g.did)} | ||
<li><span class="row-main"><Avatar name={a.handle} did={a.did} src={a.avatar} /><Person handle={a.handle} /></span><button class="btn danger" type="button" disabled={!!busy} aria-busy={busy === `ungate ${b} ${g.did}`} onclick={() => removeGate(held, g.did)}>{busy === `ungate ${b} ${g.did}` ? 'Removing…' : 'Remove'}</button></li> | ||
{/each} | ||
</ul> | ||
{/if} | ||
</form> | ||
{/if} | ||
{#if renaming === b} | ||
<form class="form row-form" onsubmit={(e) => rename(e, b)}> | ||
| @@ -131,4 +174,5 @@ | ||
<p class="hint">A protected branch only moves forward and is never deleted, as collaborators push it through the gateway; without merge commits, pulls merge into it squashed. The owner, who writes directly, is held only by their own client.</p> | ||
<p class="hint">With the merge queue required, pulls land on a branch only through the queue, and through the gateway only the runner that lands the queue moves it.</p> | ||
<p class="hint">With approvals required, a pull merges only once that many of the owner and collaborators, other than its author, approve it, and none of them asks for changes. With code owners required, each path it changes that the branch's CODEOWNERS file gives owners needs one of them to approve it.</p> | ||
<p class="hint">A pull into a branch that waits for a review from an account, such as a review agent, merges only once that account has reviewed its latest push without asking for changes. The account needs no write access, and its approval counts toward no approvals. An approval of the latest push from the owner or a collaborator, given after its review, stands in for it.</p> | ||
</section> | ||
web/src/routes/[authority]/[repo]/settings/branches/+page.svelte+1−1
| @@ -7,5 +7,5 @@ | ||
</script> | ||
| ||
<RepoSettings repo={r} section="branches" title="Branches"> | ||
<SettingsBranches repo={r} authority={data.authority} branches={data.branches} defaultBranch={s.defaultBranch} rules={s.rules} /> | ||
<SettingsBranches repo={r} authority={data.authority} branches={data.branches} defaultBranch={s.defaultBranch} rules={s.rules} gates={s.gates} /> | ||
</RepoSettings> | ||
web/src/app.css+5−0
| @@ -1391,6 +1391,11 @@ textarea.field{resize:vertical;font-family:var(--font-mono);font-size:var(--text | ||
.row-end .btn{height:var(--control-sm);padding:0 var(--space-2);font-size:var(--text-xs)} | ||
.row-form{flex-basis:100%;max-width:var(--measure)} | ||
.row-form.reviews{display:flex;flex-wrap:wrap;align-items:center;gap:var(--space-2) var(--space-4)} | ||
.row-form .gates{list-style:none;margin:var(--space-2) 0 0;padding:0;display:flex;flex-direction:column;gap:var(--space-2)} | ||
.list li .gates>li{padding:0;border-top:0;justify-content:space-between;gap:var(--space-3)} | ||
.list li .gates>li:hover{background:none} | ||
.gates .row-main{flex-wrap:nowrap} | ||
.gates .row-main>a{min-width:0;overflow-wrap:anywhere} | ||
.row-form.reviews select{width:auto;height:var(--control-sm);min-height:0;font-size:var(--text-field-sm)} | ||
.check{display:flex;align-items:center;gap:var(--space-2);font-size:var(--text-sm)} | ||
.form .check{font-weight:var(--weight-medium)} | ||