~ 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

Pulls and issues are assigned, and maintainers ask for reviews

Merged
Merged chadtmiller.com asked to merge into main from chadtmiller.com/knotless:forge/assignees

What changes

Assignees. The owner and collaborators assign a pull or an issue to accounts. Each keeps one dev.knotless.assignment record per item in their own account, keyed as their label record is and rewritten as it changes, for a private repository in the space. A reader takes the latest entry for each account from the owner or a collaborator, as it does for labels. knotless assign and unassign, MCP assign_item and the pages' dev.knotless.assignItem write it. knotless show lists the assignees, and assignee:<handle, did or @me> and no:assignee filter lists everywhere the query is read.

Maintainers ask for reviews. The same record holds reviewers: the owner and collaborators ask anyone but the author to review a pull. A pull's reviewers are now those its author asks in the pull, then those the maintainers ask. Each asker stops asking only those it asked, and is told who asked when it did not. knotless ask-review and unask-review (and assign_item with review) work for the author too, writing the pull as edit --reviewer does.

Notifications. Whom an item is assigned to hears an assigned notification. One a maintainer asks to review hears review-requested, unless the author asked already. Muting the item silences neither, as with review requests today. The index keeps whom each assignment record names in a new assigned table, as it keeps pulls' reviewers.

The pages. An item's assignees show in the facts line under its title, beside reviewers and labels. The owner and collaborators assign more people or take one off there, or from Add when it has none. They can now also ask for reviews of someone else's pull from the reviewers fact or Add. A new Assignments switch sits in notification settings.

Why

Maintainers had no way to say whom an item waits on, nor to ask for a review of a pull they did not open: the pull's reviewers live in its author's record. A record of the maintainer's own, read as labels are, gives both without touching what the author writes.

How it was checked

  • assignees_and_reviews_maintainers_ask_for (serve forge tests): every refusal, assigning and taking off as owner and collaborator, show and the filters, notifications of both kinds, the author and a maintainer each failing to stop the other's ask, and an owner's own pull.
  • The gateway opening test: viewer.assign, assignItem as owner and refused as the fan, asking for a review, an unknown role.
  • MCP assign_item and show_item's assignees; a forge unit test for who counts and which word stands; query tests.
  • e2e: the owner assigns from Add and reads it back from their PDS, and in the private flow from the space.
  • Here: cargo check with tests, the knotless unit tests above, svelte-check, layers, contrast and the lexicon checks. The rest is left to CI.

Shortcomings

  • The OAuth scope and the private repository space type now name dev.knotless.assignment. Pages sessions from before are asked to log in again, as with any new record. A PDS that cached the old dev.knotless.space lexicon may need it published (publish-lexicons on main) before it grants the wider space scope.
  • Notifications count collaborators from the repository record. Organization members who are not named collaborators are not counted there, though readers count them.
  • The new-pull page cannot assign yet. That page is another pull's.

🤖 Generated with Claude Code

a-menu-desktop-light.png

a-menu-phone-dark.png

a-add-desktop-light.png

a-add-phone-dark.png

Asked for changes

This adds dev.knotless.assignment records. Owners and collaborators use them to assign pulls and issues and to ask for reviews. The record is read as labels are. The pull also adds the CLI, MCP and page controls, the assigned index table and the new notifications, and has a change file.

I read forge::assign and its two callers, assign_item in the web handler and in the MCP tool. The rules for who may assign, the author's own asks and the "asked by the other party" refusal look consistent. The checks had not finished when I looked: the 1897e5f runs for web, shots, lexicons, e2e, docs and test were still queued or had been cancelled and restarted. I read about two files' worth of the 2,000-line diff, so my coverage is partial.

Must change: the body has no Shots-moved: line. AGENTS.md says the shots check fails a pull that moves a recorded page without naming it. This pull plainly changes the notification settings page (the new Assignments switch) and probably the item pages' facts line. Once the shots run reports what moved, add Shots-moved: <names> on its own line in the body, then rerun the shots check. The body is read when the check runs, so editing it alone changes nothing.

Asked for changes

Nothing has changed since my last review except a "Merge main" commit. The body still has no Shots-moved: line, and the checks for 491231a (check, test) are still queued. I did not find a shots run for this commit.

Must change: this pull moves recorded pages. The new Assignments switch is on the notification settings page, and the facts line changes on the item pages. AGENTS.md says the shots check fails a pull whose body doesn't name the pages it moves.

To fix it:

  1. Get the page names from the shots check once it has run.
  2. Add Shots-moved: <names> on its own line in the body. all also works.
  3. Rerun the check with knotless rerun <repo> --pull <pull uri> --workflow shots. The body is read when the check runs, so editing it alone changes nothing.

I have no code findings yet. I can't clear the pull until the checks have run.

Approved

I withdraw my earlier "must change" about Shots-moved:. The shots run for 1897e5f ended with ==> no page moved and passed with 192 tests, so no line is needed. I should have waited for that run before asking for one.

The 491231a test run failed. The cause was two uncovered lines, index.rs:1315 and web/threads.rs:458 (in assign_item), plus a flaky racing_gateways_rebuild_on_the_moved_branch that passed on retry. The newest commit, "assignItem's author path is tested, and no ? stands alone", targets exactly those lines. That fits the coverage gap, but I have not seen it pass.

The checks for 2d4de94 are still running or queued: check, test, e2e, web, shots and lexicons. docs has passed. I found no further problems and read only part of the diff. This approval assumes test and shots pass on this commit. If test still reports uncovered lines, the pull should not land until they are covered. The pull is already in the merge queue, which runs the checks again before it lands.

Approved

I found nothing new to ask for. Round 3 asked for no changes beyond the coverage fix, and the later commits are merges of main or small follow-ups. All the checks for 743243a (check, test, web, shots, lexicons, e2e, docs) are still queued, so I haven't seen this commit pass. This approval assumes test passes with the two previously uncovered lines now covered. If it reports uncovered lines or a failure, fix that before landing. The merge queue re-runs the checks before the pull lands.

Merged as 151f390