~ knotless Search/
↑↓ move↵ open⇧↵ all results

chadtmiller.com/knotless

git that lives in your Atmosphere account

knotless.dev
1 collaborator

knotless 0.13.0 Latest 2 packages, 1 image

1
10

An organization's admins change its repositories' settings on the pages

Merged
Merged chadtmiller.com asked to merge into main from chadtmiller.com/knotless:serve/org-admin-settings

What changes

Admins see the Settings tab. An admin who joined an organization sees the Settings tab on its repositories. getRepo's viewer gains admin: true for the owner and for such an admin (dev.knotless.defs#repoViewerState, which only grows), and the pages show the tab and every section from it instead of comparing the viewer with the owner.

They change settings as the organization. The pages' settings writes (updateRepo, renameRepo, compactRepo, archiveRepo, protectBranch, unprotectBranch, defineLabel, removeLabel, shareRepo, unshareRepo, setTriager, removeTriager, addRunner, removeRunner) take such an admin. Each is written as the organization through the session this server keeps as its gateway, the same way createOrgRepo already makes repositories for admins. An organization that delegated to another gateway is 403. The Actions page's secrets name the owner, so an admin's are left as the organization's.

Transfer and delete stay the organization's own. transferRepo and deleteRepo still take the owner alone. For an admin, the Danger zone keeps Archive and says only the organization itself can transfer or delete the repository.

addRunner from someone who is neither owner nor admin is now 403, from the settings' own check, where it was a 400 from the record write failing.

Why

Part of An org's admins can't change its repositories' settings: an organization's admins needed its own login for any change to a repository once it existed. On the pages that took one gate, as every settings write goes through owned(), and the gateway already holds the organization's session. Transfer and delete are left with the organization because they cannot be undone, and are easy to widen later.

This is built on An organization's admins set its repositories' secrets (an admin's secrets at the appview), which is queued, so its commits show here until it lands.

How it was checked

organizations_invite_and_members_join has the admin's pages login see viewer.admin, change the description and protect main, both read back from the organization's record, while transfer and delete are 403 for the admin and a stranger's change is 403. just look settings --as bob.test --set 'getRepo.viewer.admin=true', attached, shows the general page as an admin, desktop light and phone dark. The rest is left to CI.

Shortcomings

The CLI and MCP still act only as the owner for these settings (protect, runners, rename, archive and the rest). That needs a gateway method an admin calls with a service token, as org add --org does, and is the issue's next part.

viewer.admin is true for a joined admin even when the organization delegated to another gateway. The tab then shows, and each write says why it is refused.

No e2e flow for an admin: the e2e world has no organization yet.

settings-desktop-light.png

settings-phone-dark.png

Approved

This pull lets a joined organization admin use the Settings tab and make the pages' settings writes as the organization. It adds viewer.admin to getRepo, adds an owned() gate that falls back to org_admits and the gateway's organization session, and leaves transferRepo and deleteRepo on owner_only.

I could only read part of the change. The full diff was too large for my tools, so I read the gate (web/settings.rs owned/owner_only), org_admits, the new viewer.admin field, and the secrets handling. I did not read the individual handlers, the web components or the tests.

In what I read, the gate is sound:

  • The owner path is unchanged.
  • An admin must be the organization itself or an admin who has joined.
  • gateway::organization returns 403 when the organization delegated to another gateway.
  • Transfer and delete stay owner-only.

CI had not finished when I looked: check was running and the other six were queued. Treat my approval as conditional on all seven passing, especially test's 100% coverage gate and shots. The body does not appear to have a Shots-moved: line, and the settings pages are listed as moved in the attached pictures. The shots check will fail unless the body names the moved pages on such a line.

Asked for changes

The code is unchanged from round 1 apart from merges of main and the secrets branch. My earlier read of the gate still holds: the owner path is unchanged, an admin must have joined, and transfer and delete stay owner-only.

One thing I asked for last round is still missing. The pull body has no Shots-moved: line. The settings pages move on purpose, as the attached pictures show. AGENTS.md and .knotless/workflows/shots.yml say the shots check fails on any moved page the body doesn't name on such a line.

Edit the body to add a line of its own, for example Shots-moved: settings. Use the page names the shots check reports, or all. Then the shots check runs again.

CI is still running (check running, six queued), so I haven't seen results. The 100% coverage gate in test is the one I'd watch, since the pull's description says only one integration test exercises the new admin path.

Asked for changes

The code is unchanged apart from another merge of main, and my read of the gate still holds. The owner path is unchanged, an admin must have joined, and transfer and delete stay owner-only.

My round 2 request was not done. The pull body still has no Shots-moved: line. The settings pages move on purpose, as the attached pictures show. AGENTS.md and .knotless/workflows/shots.yml say the shots check fails on any moved page the body doesn't name on such a line. Add a line of its own, for example Shots-moved: settings. Use the page names the shots check reports, or all.

CI isn't done yet. shots and e2e are running, and web and test are queued. The earlier test and check runs were cancelled and are being rerun. lexicons, docs and check have passed. The test workflow's 100% coverage gate is the one to watch, because the description says only one integration test exercises the new admin path.

Merged as a466018