audit: sweep CLI flags, subcommands, and MCP arg/action names for name-vs-behavior mismatches #441
Labels
No labels
breaking
bug
documentation
enhancement
epic
good first issue
help wanted
refactoring
resolution/duplicate
resolution/invalid
resolution/wontfix
security
severity/critical
severity/high
severity/low
severity/medium
status/abandoned
status/blocked
status/needs-decision
status/needs-info
status/needs-verification
testing
upstream
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Blocks
Reference
sixfold-space/madtea#441
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
Some command surfaces carry a NAME that does not match what they DO. The name misleads a caller until it fails or behaves unexpectedly.
Seed example (found while landing #432): the
--allow-ff-onlyflag onmadtea repo editis named for fast-forward-only merging, but it binds toallow_rebase_update- the Gitea field for "update a PR branch by rebase," a different setting. #432 corrected the flag's help text but kept the binding, and added the correctly named--allow-fast-forward-onlyfor the real merge style. So the repo now has a flag whose NAME still implies ff-only while its behavior is rebase-update.Ask
Sweep every command surface for the same class of defect - a name that does not describe its behavior:
internal/cmd/).internal/mcp/,internal/opspec/).For each mismatch, record the name, what it actually does, and a proposed fix (rename to the accurate name, or correct the binding). Renames are governed by ADR 0016 (one canonical name per operation, enforced by the
internal/paritytest) and are compatibility decisions, so the deliverable is the audit plus proposals; each rename is then decided against the naming rules.Acceptance
--allow-ff-onlyis the first entry.Related
Ran the sweep. Result: the seed (
--allow-ff-only) is already fixed, one true live name-vs-binding mismatch, plus two parity defects that will trip callers. Total actionable: 3, plus one cross-surface decision.Method
Two surfaces enumerated:
Flags().StringVar/BoolVar/IntVar/...) acrossinternal/cmd/(518 registrations, read in full). Compared each NAME against its description and bound variable, then traced the high-risk mutation surfaces (repo edit/create, pr merge, branch protection, release, push-mirror) throughinternal/service/andinternal/gitea/client_*.goto the actual API JSON field.internal/opspec/paramdocs.goin full (the source of record for MCP param docs, ADR 0008) plus thejson:"..."tags and Required/Optional lists ininternal/mcp/tools_*.go.docs/api-specs/forgejo-15.0.json/gitea-1.26.1.json. Governing docs read first: ADR 0016 anddocs/contributing/naming-conventions.md.Not fully covered: I did not trace all 518 flags individually to their API field, only the high-risk mutation surfaces plus a name/description eyeball of the tail. Positionals were spot-checked, not swept. Raw-passthrough params (
data,json_fields) bypass name-checking by design.Mismatches
1.
repo edit --allow-ff-only->allow_rebase_update(SEED, ALREADY FIXED). Resolved inb8a03828: the current tree registers--allow-rebase-update->allow_rebase_update(internal/cmd/repo/update.go:175) and the correctly-named--allow-fast-forward-only->allow_fast_forward_only_merge(:176). The founding example no longer exists.2. MCP
madt_repos push_mirror_idbinds to the mirror NAME, not an ID (TRUE mismatch, live).PushMirrorID string json:"push_mirror_id"(internal/mcp/tools_repos.go:122, Required at:391/:401) is passed straight intoGET/DELETE /push_mirrors/{name}(internal/gitea/client_pushmirror.go:25-31). The API keys a push mirror by remote name; the CLI already models it as a<name>positional (internal/cmd/repo/pushmirror.go:77-101); the param's own description says "mirror name" (internal/opspec/paramdocs.go:635). A caller who trusts the name and passes a number gets a 404. Proposed rebind: rename the MCP arg topush_mirror_name(plainnameis taken by the repo-name field, so keep the prefix). This aligns MCP with both the API path segment and the CLI positional.3.
release --prereleasevs--pre-release(CLI inconsistency, live).release create --prerelease(no hyphen, matches gh;release_create.go:152) butrelease list --pre-release(release_list.go:112) andrelease edit --pre-release(release_update.go:135) are hyphenated. MCP is consistent (prereleaseeverywhere). A caller who learned--prereleaseon create gets "unknown flag" on list. gh uses--prereleaseon every subcommand. Proposed: rename--pre-release->--prereleaseon list and edit.4. PR-merge param names diverge CLI vs MCP (cross-surface, a decision). CLI
pr merge --admin/--auto/--match-head-commit(gh-canonical;internal/cmd/pr/merge.go:133-135) vs MCPforce_merge/auto_merge/head_commit_id(internal/mcp/tools_prs.go:69-70,paramdocs.go:489). None misleads on its own; the defect is cross-surface inconsistency (ADR 0016: one canonical name per operation on both surfaces). This is the one I want to settle deliberately: align MCP to the gh/CLI canon (admin/auto/match_head_commit), or record these as deliberate MCP-clarity deviations the way theview/branches-remoteamendments do. I lean align foradmin/auto(gh muscle-memory is the whole point of ADR 0016), but it is a compatibility call. The parity test does not currently catch param-name divergence (it enforces action-name parity) - that mechanical backstop is #444.Sanctioned short forms (NOT mismatches)
--allow-merge->allow_merge_commits,--allow-squash->allow_squash_merge,--allow-rebase*(exact),--allow-fast-forward-only->allow_fast_forward_only_merge,pr merge --ff-only-> strategy "fast-forward-only" (the correct use of ff-only, and the contrast that proves the seed was specific to repo edit). Plus the ADR-sanctioned surface conventions (view/get,branches-remote, forge-plural nouns,q).Coverage gaps / borderline (flagged, not promoted)
madt_orchestrate max_failures: same name, different default across surfaces (CLI 0 = unlimited; MCP 0 = default 2). Documented, not deceptive, but a real gotcha.repo search --topicis a boolean (mirrors the Giteatopicfield) but reads like it wants a value;--starred-byis an int user ID. Both API-anchored, defensible, but plausible caller traps.repo edit --no-wiki/--no-packages/... are disable-only (cannot re-enable via CLI; only MCPhas_wikienables) - a gh-parity + capability gap, not a name-vs-behavior mismatch.Decision
These are public-surface renames, so they are compatibility changes, and they sit on top of #423 (converge-and-reject vs keep a deprecation alias). Which do you want to land, and under which #423 policy: (2)
push_mirror_id->push_mirror_name, (3)--pre-release->--prerelease, and the (4) PR-merge align-or-record call? Say the set and I will land them (or wire #444 to enforce it mechanically).Decided. Ground truth was checked against the Forgejo API spec, gh, and the CURRENT tea CLI (
gitea.com/gitea/tea, HEAD 2026-09-08). Note: the GitHubgo-gitea/teamirror is frozen at 2019 and was unusable - the live source had to be read directly.Landing:
push_mirror_id->push_mirror_name. It binds to the mirror NAME (GET/DELETE /push_mirrors/{name},internal/gitea/client_pushmirror.go:25-31); the current name invites a 404. Converge-rename per the #423 policy (the old name rejects with a teaching redirect).--pre-release->--prereleaseonrelease listandrelease edit, matchingrelease createand gh. Converge-rename.force_merge->admin,auto_merge->auto,head_commit_id->match_head_commit. One canonical name across both surfaces (ADR 0016). tea offers no precedent (it exposes only--style/--title/--messageand omits these three), so gh is the only established CLI vocabulary. Old MCP names reject with a teaching redirect; changelog-flag the break (0.x, ADR 0028).Item 1 (the
--allow-ff-onlyseed) was already fixed (b8a03828).The mechanical parity backstop for param-name divergence (the parity test only enforces action names today) is #444.
Clearing needs-decision.