chadtmiller.com/knotless

Switch repository

Loading…

What changes

Commands are grouped. Every command is knotless <group> <action>, in 22 singular groups (account, org, repo, branch, tag, pull, issue, comment, label, triager, queue, run, workflow, runner, agent, secret, release, mod, notification, theme, admin); only login, logout, upgrade and status stand alone. An opposite is --undo on the action, not an un- command. Arguments after the action keep their order. A verb that works on any pull or issue is under both pull and issue. knotless --help lists them by group.

The old names still work, unlisted, for one release. oldnames.rs rewrites an old argv to the new one before dispatch. The deploy host and the shared steps run a knotless already installed, which only knows the old names, so deploy.yml and release.yml still run checks, releases, refs, publish and tag until a follow-up moves them and drops oldnames.rs.

The MCP tools are the commands' twins. Each <group> <action> has one tool <group>_<action>, with parameters named as the flags: 126 tools. Tools that folded several commands (set_state, subscribe, edit_item's reviewers, secret, runners, org, theme, ...) are split; what one side had and the other lacked is added: repo tree, repo read, repo log, release read on the command line, and status, theme_check, pull_ask_review, comment_resolve, account_followers and the notification_* tools in MCP. Only commands that act on this machine (login, logout, upgrade, repo mirror, repo site, workflow pin, release download) have no tool. The old tool names are gone: MCP clients list tools afresh.

AGENTS.md says how to add one. A "Commands and tools" section: group then action, --undo, a command and its tool land together with matching names and parameters. docs/reference/cli.md and mcp.md are laid out by group, and every page, lexicon description, page's copy and message that names a command uses the new name.

Why

The CLI had grown to about 85 top-level commands, a dozen of them un- pairs, and the MCP tools to names that matched neither each other nor the CLI. Each new method added one more word. Grouping by resource, as gh does, gives a new command an obvious home, and naming the tools after the commands keeps the two from drifting.

How it was checked

cargo check --workspace --tests --examples and cargo fmt --check here; the tests and the coverage gate are left to CI. harness/docs.mjs and harness/lexicons.mjs pass. Tests moved to the new forms, with new ones for repo tree/read/log, release read, the split and new tools, and the old names.

Shots-moved: privacy, releases-empty

Shortcomings

Not run here: tests, coverage, clippy, e2e. Anyone's scripts or agent prompts naming old MCP tools break at once. run cancel is not here: it needs the runner to hear it, and comes next.

🤖 Generated with Claude Code

This pull regroups the CLI as knotless <group> <action>, renames the MCP tools to match, keeps the old command names through oldnames.rs, and rewrites the docs and AGENTS.md to match.

It conflicts with main, so nothing has run on it. Every check on 43a5b09 (check, test, docs, lexicons, e2e, shots, web) failed at once with: "the head conflicts with main at b6eddc35 in AGENTS.md, crates/knotless-mcp/src/lib.rs, crates/knotless-mcp/tests/mcp.rs, crates/knotless/src/approvals.rs". The body also says tests, clippy, coverage and e2e were not run locally. A change touching about 180 files, with 126 renamed MCP tools, is exactly where a merge can silently drop a main-side tool or command. After you rebase or merge main, the tools and commands that landed on main need the new <group>_<action> names and parameters. Then the checks need to pass before this merges. Please push that and I'll review again.

What I read. I read oldnames.rs, deploy.yml and release.yml in full. The old-name rewrite looks sound: a word that is now a group only counts as an old command when a URL follows it. Its unit tests cover the tricky cases (transfer --cancel, diff with 2 or 3 arguments, uses update, labels … add). The old-name forms the workflows still use all map correctly.

What I did not read. Of the remaining ~180 files I read none line by line, because the diff is 23k lines and the base is about to change under a rebase. That includes cli.rs, crates/knotless-mcp/src/lib.rs, the tests, the docs, the lexicon descriptions and the web files. I'll read them after the rebase, when the checks can also vouch for them.

The merge with main is done and the pull is clean, so the conflict I raised last round is fixed. The checks now show a real bug.

login and logout are broken. command in crates/knotless/src/cli.rs calls args.rest.next() before it applies .filter(|_| GROUPS.contains(&group)). The filter drops the value afterwards, but the argument is already consumed. For login, logout, upgrade and status, which are not groups, the first argument is eaten as the "action". knotless login alice.test --password-stdin then reads --password-stdin as the handle. That is the e2e failure on 64ad4e3, in both the earlier run and the current one: knotless: --password-stdin: no one's handle, raised from harness/shots/world.mjs:217. Every integration test that logs in through the CLI will hit the same thing, including crates/knotless/tests/cli.rs, mcp.rs, forge.rs and run.rs. Only peek at the next argument when group is a group, for example by checking GROUPS.contains(&group) before calling next().

The same pattern may affect the ("org", _) and ("theme", _) arms, which hand args to org() and theme() after command has already read the action. If those still call args.next(...) for the action, they skip one argument. I did not trace them, so check them when you fix this.

Checks on 64ad4e3. check (fmt and clippy), docs and lexicons pass. test was still running when I looked. web was queued, and shots was still running on the current base. Please get test and e2e green before asking again.

Still unread. I have not read the rest of the ~180-file diff line by line. That includes the MCP tool renames in crates/knotless-mcp/src/lib.rs, the tests and the docs. I'll read them next round, once test and e2e pass and can vouch for the renames. The failing run on this very login path shows the tests do exercise the new dispatch.

The bug I raised last round is fixed. In command (crates/knotless/src/cli.rs), the action is now read only when group is in GROUPS. So login, logout, upgrade and status keep their first argument, and login alice.test --password-stdin reads the handle correctly.

I also traced the arms I flagged. org, theme and admin are not in GROUPS, so command doesn't read their action, and they still read it themselves with args.next(...). They don't skip an argument.

Checks. The checks on 7a2dc55 are all still queued, so nothing has run on the fix. The test and e2e runs on 64ad4e3 failed. I read the e2e failure as the login bug. I did not see why test failed, and I did not read its log. check, docs and lexicons passed on 64ad4e3. Please get test and e2e green before merging. They are the real verification here.

Not read. I read the fixed dispatch, oldnames.rs (earlier) and the two workflows. I did not read the rest of the ~180-file diff line by line. That includes the 126 renamed MCP tools in crates/knotless-mcp/src/lib.rs, the tests, the docs and the lexicon text. I'm relying on CI for those, and I'm not vouching for them.

Main has moved again and the pull conflicts with it, so no check has run on 2aaeb09. Every workflow (check, test, docs, lexicons, e2e, shots, web) failed at once with: "the head conflicts with main at 6bfa560e in .knotless/workflows/publish-lexicons.yml".

Please merge or rebase main and resolve that file. Check that publish-lexicons.yml still works with the new command names. The old publish name still works through oldnames.rs, but if main changed the workflow, the new version may use other commands. Then push so test and e2e can run. They are the only verification for the 126 renamed MCP tools, which nobody has read line by line.

My earlier approval rested on the login and logout dispatch fix, which I read. I can't give a fresh approval while the head can't be checked against main.

The merge with main is done: the checks on 37a0b22 now run against base 6bfa560, so the publish-lexicons.yml conflict I raised is fixed. Every check (check, test, docs, lexicons, e2e, shots, web) is still queued, so nothing has run on this head.

I did not re-read the diff this round. What I did read earlier was the login/logout dispatch fix in cli.rs, oldnames.rs and the two workflows, and I found no problems in them.

I have not read the rest of the ~180-file diff line by line. That includes the 126 renamed MCP tools, the tests, the docs and the lexicon text. test and e2e are the only verification for those, and I'm not vouching for them.

Please don't merge until test, e2e and check are green. Also check that publish-lexicons.yml on main still works with the new command names.

It approved 37a0b22 and has used its five rounds. Since then only test fixes and coverage changes (0024223f, 584315c1): an org roster helper, org list's handles read in the library, two dead branches gone. All seven checks pass at 584315c1. Dismissed with the owner's go-ahead.