Fix everything the batch 2 sweep found: 105 verified findings in 24 layers

[jmrplens]

The batch 2 sweep of 42 tool packages produced 123 candidate findings. Each was investigated against the tree and then put to an independent agent whose job was to break it. 18 did not survive that, for reasons recorded below; 105 did. This issue tracks fixing all of them.

Nothing here is deferred, declined or accepted as a known limitation. The ordering below is about what can physically land before what, not about what is worth doing.

The two hard constraints that shape the order

A gate cannot land before the code it judges is clean, because it would fail on packages the batch that introduced it never touched. That is why the canonical ID work is report first, fix in tranches, gate last.

And the prepared sweep pull request does not grow, so every finding that belongs to already-swept code becomes a layer above it rather than an amendment to it.

The spine: canonical action IDs

This is far larger than the first count suggested. About 536 of the 2801 published cross-link entries and 44 of 755 card hints name something no surface resolves, across 56 packages. Batch 2 fixed eleven of them and missed roughly as many inside its own scope, because an agent verifying an ID with --tool-search is doing a substring match over search text rather than asking the resolver.

It reaches the documentation too, and there the worst case is not prose. site/src/content/docs/operations/ci-cd.mdx and its Spanish mirror teach a pipeline_schedule. prefix for six actions, in a table row and the paragraph under it. There is no pipeline_schedule domain; the catalog holds pipeline.schedule_list and the rest. A published page teaches a whole wrong family in two languages.

The plan

1. Batch 2 sweep: 42 packages (already prepared, does not grow) (large, 9 findings)
   This is the pull request that exists. It holds the 41 sweep commits and must not take another line, so the only findings assigned to it are the ones whose correct fix is no change at all: the ten ID families it really did repair, the production-unreachable guards it correctly left in place with a test beside each, the seven pages.* IDs it fixed silently, and five separate checks that came back clean (the archive doc line was already true before the fix, the milestones WebURL edit is byte-identical output, the two unreported production edits are sound, securityfindings sends its filters correctly, and audit_doc_tool_names --check is green). Naming them here is what stops the next reader re-opening them. Everything else the review found lands above it.

2. The sweep brief and the survivor taxonomy (small, 8 findings)
   Documentation only, and first because it is the only layer whose entire output is input to the next batch. It puts the missing condition on section 5 (consolidate two blocks of ID constants where they disagree, not wherever they exist), states the one rule that retires a whole class of served prose (text that names another action names the canonical ID, because every surface resolves that and no surface resolves all the others), and corrects docs/development/testing/README.md, which says three kinds over five bullets and has no entry for an error branch that cannot fail for the type in hand. The four class findings that are review norms rather than gates go here too: a refusal test names ForbiddenHandler, a family defect is greped before it is reported, a non-test file is edited in a sweep only for a named defect, and a probe is restored with git stash rather than git checkout, which silently reverted a verified fix. One pull request because it is one document set and one audience, and splitting it would mean an agent reading half a rule.

3. An em-dash step in CI, on the diff and on the pull request body (small, 3 findings)
   Its own pull request because it is a workflow change with no Go in it, and it goes this low because the class it prevents is produced by authoring, which is what the next batch is. Three steps: the pull request title and body, which is the copy a squash merge makes permanent and which no later commit can clean; the added lines of git diff origin/main...HEAD, net rather than per-commit so a stacked branch that cleans up at its tip still passes; and optionally the message bodies as an author-side warning. It carries the two already-fixed findings as its record: the tree half of the class is done, and the seven survivors in five commit messages are left alone deliberately because this repository squash merges.

4. cmd/audit_action_ids: the oracle, reporting and not yet failing (medium, 3 findings)
   One command, loaded through cmd/internal/goprogram like the four gates that already share it, holding two rules that report and do not gate: every RelatedActions entry and every toolutil.HintAction first argument against the catalog's ID set, and every unexported constant the type checker records no use of. It lands before any fix because it is what turns each of the next three tranches from a judgement call into a diff a reviewer can check, and because the 611-site worklist currently lives in a job temp directory rather than in the repository. Three findings that are decisions about the command's shape ride with it: scope the ID rule to HintAction and the spec field rather than to every dotted literal (the literal form needs about a hundred permanent declarations on day one and a reader learns to skip a list like that, which this repository has written down about its own sdk_graphql refusals); read constant values from the type checker, which folds the two concatenated IDs no regex can; and do not extend the golden snapshots, which cannot carry related actions at all since the field never appears in tools/list. It must build the catalog at Ultimate and at GitLab.com together, or Orbit's six IDs read as dead.

5. Tests that assert less than they claim (small, 3 findings)
   Three corrections to what batch 2 itself left, grouped because they are the same defect in the test layer and all three sit in packages the batch touched: a guard that derives both sides of its comparison from the same constant so the domain it was written to pin can still move, a doc comment claiming an assertion about the overwrite option that its body never makes, and two boundary survivors filed as unkillable when the documented answer for their shape is to delete the redundant guard and assert the property instead. It sits below the ID work because it touches no constant the tranches rename, and above the oracle because the tautological guard is exactly the thing the oracle replaces.

6. Canonical IDs, tranche 1: the invented domains (large, 14 findings)
   The tranches partition packages, never defects, so that no package is edited in two layers of a stack. This one takes every package that publishes a plausible domain the catalog has never held: commits and mrcontextcommits (commit.), pipelineschedules (pipeline_schedule.), milestones and groupmilestones, labels and grouplabels, members and groupmembers, epics, todos, namespaces, keys, workitems, the deployment.list family across deployments, environments, freezeperiods and clusteragents, and the three template packages whose sibling ciyamltemplates already spells it right. The card hints move with their own package's spec constants, which is why the two hint findings are listed here rather than in a tranche of their own: internal/tools/milestones/markdown.go and action_specs.go must change together, and one of them carries a comment asserting these are "the one form every surface resolves" over five IDs that resolve nowhere. The repository-wide measurements are listed here because this is where the worklist is consumed, and they only reach zero at tranche 3. No generated artifact moves: related actions and hints appear in no snapshot and in no llms file.

7. Canonical IDs, tranche 2: the owner package used as the domain (medium, 2 findings)
   The second spelling family, and one pull request because every edit in it is the same mechanical substitution: strip the owning package's name and put the catalog domain in its place. accessrequests, commitdiscussions, instancevariables, impersonationtokens, issuestatistics, cicatalog, mrapprovals, repositorysubmodules, groupprotectedenvs, mrdraftnotes and projectimportexport. Two of them are worth naming on their own because the same file already holds the right answer beside the wrong one: mrdraftnotes spells five siblings mr_review.* correctly and its own four as mrdraftnotes.*, and projectimportexport renders its wrong export_download ID into the export-status card, which is the exact moment a model is deciding whether to download.

8. Canonical IDs, tranche 3: bare names, tool names, and one home for the block (large, 7 findings)
   The remaining two spellings plus the convention that stops the class reforming. Bare action names with no domain at all (notifications, groupmembers, groupimportexport, mrapprovalsettings), and individual tool names in a field that carries IDs (runners and runnercontrollers, 56 entries between them). The tool-name form is kept in the same pull request as the bare-name form and not in a tranche of its own because they are one decision: the field publishes a catalog ID, and the fact that an alias table happens to resolve the runner names today is a trap rather than a defence, since one new spec claiming one of those names puts it in ambiguousAliases and kills eighteen cross-links at once. It also settles where the constant block lives, moving the six markdown.go-homed blocks into action_specs.go, merging sidekiq's two, and writing the rule down, which is what section 5 of the brief should have said. batch-packages-still-wrong closes here because this is the layer after which no package batch 2 touched still carries a wrong ID.

9. Dead constants, and the rule that can see them (medium, 7 findings)
   The five known dead constants and the tree-wide sweep the new rule names, landed with the rule turned on in the same pull request. That pairing is deliberate and is the one place I depart from "fix first, gate second": the rule is the only thing that can enumerate its own class, since staticcheck's unused treats a const group as one unit and the action-ID block is the prevailing shape here, so the extent cannot be known before the pass exists and the deletions it names are mechanical. Two of the five are not simply deleted: projectaliases should gain its create cross-link, because all four of its descriptions promise gitlab_create_project_alias while none of its related lists offers it, and projectmirrors' delete constant belongs on the get action for the same reason. It goes above the tranches because renaming a constant can make another one dead.

10. adminspecs names the owner package of each admin action (medium, 1 findings)
    Its own pull request, and strictly before the deletion above it, because the true owner names are sitting in the twenty-three files that deletion removes and reading them out of the tree is better than reading them out of git history. Giving adminOptions an owner argument sharpens ninety-two actions from a package grain that cannot be joined to anything and retires the declaredSilentOwners entry that exists only to keep R-PATH quiet about it. Separating it from the deletion is what keeps the next layer's new rule honest: that rule must join on the call through the type checker rather than on OwnerPackage, precisely because this change makes OwnerPackage stop being zero for those packages.

11. Orphan ActionSpecs: carry the metadata across, delete the copies, make the audit say so (large, 10 findings)
    One pull request because the three parts cannot be separated without a red build. Deleting the twenty-three unreachable ActionSpecs makes cmd/audit_catalog_first/main_test.go fail, since two call sites there assert that twenty-one of them are spec-backed, so the test lists have to be rewritten in the same change; and the new rule (a package that declares an ActionSpecs no file calls is a gap) cannot land with an empty declaration table without turning that same test package red before the deletion arrives. Before anything is deleted the metadata is diffed and the better copy kept in adminspecs: applications disagrees word for word on all four actions, sidekiq's usage strings are the ones batch 2 wrote tests to pin, and the guidance and tags the consolidation dropped are recovered here rather than mourned later. The five related-action IDs the dead copies publish die with them, and three exported symbols inside those files are still called from adminspecs, so the deletion is surgical rather than wholesale. It also strikes health from issue 831's list of twenty-four, which is a function value with no call parentheses and whose ActionSpecs back gitlab_server, the one tool a model reaches for when connectivity is already broken; the settings copy dies here too, and its "full application settings" wording needs no edit because the layer above makes the sentence true. Finally it adds the audit-catalog-first step to make analyze, so the command a reader is pointed at is one that runs.

12. One alias table (medium, 1 findings)
    toolutil's commonActionAliases is dead for roughly ninety of its ninety-three entries: the only caller rewrites when routes[canonical] exists, and that map is keyed by the bare action name, so every dotted target can never be found. Either reduce the canonical value to its action-name half before the lookup, or retire the table in favour of actioncompat, which the dynamic registry actually consults. It has to be settled here, one layer below the gate, because the gate's truth set is the catalog IDs plus the registered aliases, and a gate cannot be written against an alias table nobody has decided the meaning of. A test should hold every surviving entry to firing on some real group, since a table nothing can reach reads as coverage.

13. A cross-link the caller cannot run (medium, 2 findings)
    The decision the gate cannot make for itself, and therefore its own layer immediately below it. Nothing filters RelatedActions when the catalog is filtered, so a Free instance is handed 114 licensed IDs, and a read_api credential or an --exclude-tools operator produces the same effect through a different filter. Either drop a withheld ID from the projection where the registry already knows which actions were withheld, or leave it and rely on withheldActionMessage, which explains a narrowing rather than calling it unknown. The second is defensible; what is not defensible is that it is currently an accident rather than a decision. The semantic half of the same finding is recorded here too: a gate can check membership and can never check that snippet.get names the right snippet, so that stays a review question and is written down as one.

14. Docs and site: the IDs they teach (small, 2 findings)
    The documentation half of the same class, and it has to land before the gate above it for the same reason the code did. The EN and ES CI/CD pages assert a pipeline_schedule prefix outright and then spell six IDs under it, which is a published user-facing page teaching a whole wrong family in two languages; epics.md and security.md name work_item.get and work_item.update where the catalog holds issue.work_item_*; upstream-bugs.md names commit.get_signature. It is one pull request with the optional project-discovery enrichment because both are edits to the same doc set in the same pass, and because the Spanish mirror must move with the English on every line or the two drift. The code these pages were written from is already fixed by tranche 1, so this layer only makes the prose agree with it.

15. The catalog-ID gate, turned on (medium, 4 findings)
    The flip, and the last layer required before batch 3. Four findings, one pull request, because they are one question asked of four readers that each answered it differently: the ActionSpec field, which nothing validated; the card hint, which no spec carries and so no catalog rule can see; the discovery audit, whose sibling matcher was written to accept both the right spelling and the wrong one and whose substring fallback accepts almost anything; and audit_doc_tool_names, whose token regex only matches gitlab_* and so cannot see a canonical ID at all. Splitting them would mean four pull requests each proving one reader now agrees while the others still do not. Two constraints the implementation must keep: demand a catalog ID rather than resolvability, or the tool-name family passes; and build the catalog at Ultimate and GitLab.com, or Orbit's family reports as dead. The documentation rule needs its own small allow table for file names, GraphQL field paths and JSON keys, seeded from the entries this review already triaged.

16. Wrong resource, not wrong spelling: scoped cross-links, circular hints, self-references (medium, 9 findings)
    Everything the gate above cannot see, collected because they share exactly that property: each names something that exists and is the wrong thing. snippetnotes sends every project-snippet-note cross-link, both See-also clauses, the 404 hint and the parameter guidance to the personal-snippet tools; awardemoji has the same mistake plus two dead siblings; the three storage-move packages each answer a 404 by naming the action that just returned it, and the three must move together or the family diverges; gitlab_list_resource_group_upcoming_jobs is the only tool of 1085 that offers itself as its own next step; and gitlab_discover_project names meta tools in one paragraph and individual tools in the next, so whichever surface a model is on half of the one description whose whole job is steering is unusable. This layer moves served descriptions, so the generated artifacts go stale here and are refreshed once at the tip of the stack, which is what FRESHNESS already arranges.

17. notifications: the level a caller may send (small, 2 findings)
    Two findings that cannot be separated without leaving the tool worse than it is. The published enum offers global on notification_global_update, which client-go refuses before a request is built, so the value can never reach GitLab; and an unrecognised level is dropped silently, the PUT goes out without it, and the tool answers with a success card showing the level GitLab still has. Fix only the enum and a mistyped level still no-ops as a success; fix only the handler and the surface still advertises a value that is now an error. The scope-specific list has to be passed in from the three call sites so the message and the enum cannot drift, and the description on the shared eventFields must move with them, since it repeats the six-value list on every surface and is the copy a model actually reads.

18. releaselinks: stop promising external (small, 4 findings)
    One pull request for one word in seven places plus the fixtures that made it survivable. The six descriptions and the doc comment promise external, docs/reference/tools/releases.md repeats it, and the live oracle from GitLab 19.3.1 says the Link entity exposes five fields and not that one. The direction matters and is a deliberate reversal of one report's advice: adding the field would publish a bool that is false on every response from every instance, which is the class issue 580 was about, so the word goes and client-go's stale ReleaseLink.External is recorded in upstream-bugs.md instead. The fourteen test fixtures that feed "external":true go in the same change, because a fixture GitLab cannot produce is the thing that lets a green test prove nothing. It also promotes the richer guidance the duplicate block carried, which told a model not to loop link_create once per asset and is precisely the case a model gets wrong.

19. settings: answer with what GitLab sent (medium, 4 findings)
    One pull request because one change answers all four findings and no smaller change answers any of them honestly. Reading the captured response into the map (ADR-0021, the SDK call unchanged) stops dropping 253 of the keys the instance sent and stops inventing 29 it never mentioned; it makes the card's coverage sentence true without editing a word of it, since len(values) becomes GitLab's own count instead of a constant property of the pinned SDK; and it is the same shape as the write-side fix, where 264 of GitLab's 660 settable params are currently discarded by an unmarshal into an option struct and the tool reports success for a change that was never sent. The write side must refuse rather than drop, which is exactly the condition this repository has written down for reaching for the explicit v2 API: refusing is the safe answer because the fail-open branch reports a reconfiguration that did not happen.

20. repository archive echoes the path it filtered (small, 1 findings)
    Small and alone because it is the only finding that adds a field to a published output type, and it completes a fix batch 2 already made: the handler now sends the subdirectory, and neither the output nor the card reports it back, so a caller has to parse the URL's query string to learn whether its path was honoured. The markdown comment saying all three values are the caller's own arguments echoed back needs to say four. Before the fix the card looked identical whether the path was applied or silently dropped, which is part of why the defect survived.

21. audit_discovery_completeness: a parameter-guidance check that can fire (small, 1 findings)
    Its own layer, and above the orphan deletion by necessity: the check is dead by construction today, because the central fill adds the canonical scope defaults to every spec before the auditor reads it, so the one gate a reader would expect to notice that seventeen admin actions publish no binding guidance reports zero and always will. Rewritten to ask whether an action's guidance consists only of centrally-filled scope defaults while it carries a required non-scope identifier, it would name those seventeen, which is why the guidance recovered during the orphan deletion has to be in place first or the rule lands red for a reason that is already being fixed below it.

22. audit_surface_quality can fail, and two rules it can now answer (medium, 4 findings)
    One pull request because the three rules share one walk and the -check flag is the precondition for all of them. Giving the command an exit code is free at the metadata view (zero violations today) but requires the report-only envelope section to be excluded, which reports ten registration problems and would fail on day one. With that in place: the fixture filler's two slice elements are currently indistinguishable, because the path's index is thrown away before the sentinel is built, so the one piece of machinery that renders all 580 formatters with a populated list cannot tell a correct render from one that prints the first row twice; passing the auditor its own Text hook fixes that and gives the constant-index class an exact detector, which is a better answer than forking gremlins for an operator it lacks and running mutation testing in CI, which nothing does. The annotation half of the metadata-dispatch class comes free with the exit code, and the structural half gets a small wrapper that parses gobco's never-false output on changed packages only, since instrumenting 179 packages is about three hours serially. The declared-Edition rule goes here too, since it is the same read of the same registered surface, and it is the check that would have caught a spec declaring premium beside a description saying Ultimate at the moment it was written.

23. R-PATH sees values, not only names (medium, 2 findings)
    Both findings are the same structural blind spot in the same seam, so they are one pull request: the request inventory records which field names a body carried and nothing about the values, and which endpoint a path reached and nothing about how many distinct identifiers reached it. The first rule reports an option field whose json tag lacks omitempty while the live record marks the matching param optional on that route, which over this batch's 113 option types separates exactly one defect from eight benign cases with no declaration at all. The second needs a recorder change (count the distinct raw values per placeholder, merged as a max across shards) and must stay report-only for a long while, because most rows will be 1 and failing would fail everything. Together they cost one field in the shard record and one regeneration of the committed inventory, which is why doing them apart would mean regenerating it twice.

24. client-go: omitempty on the package-protection update, and the record of it here (small, 2 findings)
    Off the stack, because the fix is not ours to make: both string fields of the update option struct are tagged without omitempty, so every partial update sends two explicit nulls and no local edit can suppress it, the tag being what decides. The merge request goes from the gitlab-community fork on a jmrp- branch and is linked from the umbrella issue. What lands here is the record: an entry in docs/development/upstream-bugs.md naming the handler that carries it, which mentions neither field today, and one line on the unit test's comment saying its assertion pins an upstream shape and is expected to go red when the shape is fixed. It can land at any point; it is last only because nothing else waits on it.

What must land before the next 47 packages are swept

• The sweep brief and the survivor taxonomy (PR order 2). Section 5 of /root/.claude/jobs/93d02f99/tmp/sweep-brief.md tells an agent to consolidate agreeing constant blocks with no condition attached, and that instruction alone produced the securitycategories blocking finding plus restructurings in six packages; forty-seven agents will produce forty-seven more. docs/development/testing/README.md says "Three kinds" over five bullets, so an agent that stops counting at three never reaches the two bullets that matter most (a guard a second guard hides, and the tool artifact), and it has no entry at all for the shape batch 2 actually hit in settings. Both documents are read by every sweep agent before it starts; this is the cheapest item on the list and the only one that is pure input to the next batch.

• The em-dash CI step (order 3). Eleven of the batch's forty-two commits added an em dash and one late commit cleaned the diff; on a batch of forty-seven that is a dozen more cleanup commits, or a dozen more that nobody catches. It must be diff-scoped over git diff origin/main...HEAD added lines plus the pull request title and body, since the tree carries about 2000 already and a whole-tree check fails on day one. The pull request body is the half that cannot be fixed later: a squash merge makes it main's commit message.

• cmd/audit_action_ids as a report-only command (order 4). Every agent in batch 3 will meet the canonical-ID class, because 536 of the 2801 published cross-link entries are wrong and the class is near-universal rather than exceptional. Today an agent verifies an ID with --tool-search, which is a substring match over search text and not the resolver, and batch 2 shows what that costs: agents fixed eleven packages, missed an equal number inside their own scope, and wrote tests pinning wrong values in snippetnotes and a tautological guard in runnercontrollerscopes. One command that folds constants through the type checker and prints every wrong ID with file and line turns a judgement call into a check. It also has to regenerate the worklist: the 611-site list lives in a job temp directory, not in the repository.

• The three ID fix tranches and the alias settlement (orders 6, 7, 8 and 12). This is not a preference for tidiness before new work: the 56 packages carrying wrong IDs and the 47 packages waiting to be swept overlap heavily (commits, todos, users, workitems, vulnerabilities, tags and the label, member and milestone families are all in both), so leaving them would put two efforts on the same constant lines in a stack that rebases each layer on the one below. Batch 2 already showed the split-brain result, where mrnotes and mrdraftnotes carry the right spelling in markdown.go and the wrong one in action_specs.go and a reader cannot tell which is authoritative.

• The catalog-ID gate flipped on (order 15), with the docs fixes below it (order 14). A gate that fails on code outside the batch that introduced it cannot land, which is exactly why the fixes come first and the flip comes last; but the flip must still be below batch 3, or the forty-seven packages add to the class while it is being drained. The flip carries all four "nothing checks this" findings at once, because they are one question asked of four readers: the spec field, the card hint, the discovery audit and the documentation scanner. The gate must demand a catalog ID and not mere resolvability, or the runners tool-name family stays invisible.

• The orphan ActionSpecs deletion with its audit-test rewrite (orders 10 and 11). systemhooks, terraformstates, topics and usagedata all sort after snippetstoragemoves, so they are in the next forty-seven, and all four declare an ActionSpecs no surface reaches. Batch 2 already paid this cost once: the sidekiq agent found three LIVED mutants in usage strings, judged them real, and wrote tests to kill them, on text no model will ever read. Leaving the dead copies in place buys four more packages of that, and the deletion cannot be separated from cmd/audit_catalog_first/main_test.go, which currently asserts that twenty-one of the orphans are spec-backed and turns red the moment the declarations go.

• The dead-constant rule in the same command (order 9). staticcheck's unused treats a const group as one unit, so none of the five dead constants batch 2 left behind is visible to any linter, and the action-ID block is the prevailing shape in these packages. Five in forty-one packages is the rate; forty-seven more packages is six more nobody can see. The rule is a few lines on a loader four gates already share, and its deletions are mechanical, so it gates from the moment it lands.

What refutation killed

• snippetnotes sends a model to the personal-snippet reads, and the sweep's new test pins the wrong values
  The wrong ID is real: internal/tools/snippetnotes/action_specs.go:66-67 declares actionSnippetGet = "snippet.get" and actionSnippetList = "snippet.list" while every input in snippet_notes.go:15-52 carries a required project_id and the handlers call the project-scoped SDK methods; the catalog's own descriptions confirm the split (snippet.get = "Get a single personal snippet by ID", snippet.project_get = "Get a single project snippet"). But the two claims that make this expensive are both false. (1) "The new test makes the wrong value the expected value, so the fix has to change a test added as a guard" is wrong: internal/tools/snippetnotes/action_specs_test.go:156, :166 and :176 assert wantRelated against the CONSTANTS actionSnippetGet/actionSnippetList, not against literals, so changing the two constants at action_specs.go:66-67 fixes the spec and the test in one edit and no wantRelated row moves. Their fixSketch's "update the three wantRelated rows in the same commit" is unnecessary work. (2) "It gets a clean, wrong answer rather than an error, which is the failure mode nothing reports" is almost certainly wrong: snippet.get is GET /snippets/:id over the caller's own personal snippets, so a project snippet id sent there returns 404, not a different object. The finding stands as a wrong cross-link; it is not the silent-wrong-object class it is sold as, which matters because that framing is what gives it its severity and what finding gate-would-miss-semantic-and-withheld builds its "(1) is the dangerous one" on.

• Two documentation pages name work_item.get and work_item.update, which the catalog does not hold
  Both sites are real: docs/reference/tools/epics.md:74 names work_item.get and docs/concepts/security.md:273 names work_item.update, against a catalog that holds issue.work_item_get and issue.work_item_update. But the claim that "this is the backtick-prose form only" and the implied count of two is wrong, and the reason it is wrong is their own method: they scanned for tokens and then, like my first pass, filtered by catalog domain prefix, which by construction cannot see an invented domain. Scanning instead by action-name TAIL over all 233 tracked Markdown files finds more. The worst is site/src/content/docs/operations/ci-cd.mdx:281 and :284, mirrored verbatim in site/src/content/docs/es/operations/ci-cd.mdx: a table row asserts "pipeline_schedule. prefix on the dynamic surface" and the paragraph under it spells out six IDs, pipeline_schedule.schedule_list, pipeline_schedule.schedule_run, pipeline_schedule.schedule_take_ownership, pipeline_schedule.schedule_list_triggered_pipelines, pipeline_schedule.schedule_create_variable, pipeline_schedule.schedule_edit_variable. The catalog holds pipeline.schedule_list and so on; there is no pipeline_schedule domain. That is a published user-facing page, in two languages, teaching a whole wrong prefix for a family, and it is the same wrong spelling the Go side carries (pipelineschedules is one of the 56). Also docs/development/upstream-bugs.md:2533 names commit.get_signature against repository.commit_signature. With the site pages included this is not a low-severity docs nit, and the fix list is four files, not two. Their cleared-surface claims I did re-check and they hold: no "action": "x.y" JSON example anywhere in docs/, README.md or site/ names a dead ID, and no Go description string names a dotted ID at all.

• Roughly seventeen ParameterGuidance entries were written and are served nowhere
  The substance survives and is if anything larger than reported, but the counting does not. I dumped every SemanticRole in the 23: 24 entries, and only THREE of them are scope-suggestive by the repository's own predicate (clusteragents project_id, errortracking project_id, dependencyproxy group_id). isScopeSuggestiveParameterName at internal/toolutil/action_spec.go:493-505 covers ref/branch/tag/sha/path/iid plus project_id/group_id/user_id/instance_id/namespace_id/milestone_id/epic_id; id, agent_id, key_id, version, topic_id, plan_name, name, value, title, scopes, redirect_uri are none of those. So 21 non-scope entries are lost, not the 'roughly seventeen' of the title and not the 'seven scope-suggestive' of the evidence. Everything else checks out: adminspecs declares ParameterGuidance in exactly two places (:284 appearance_update hex colours, :318 terraform_state_unlock), the keys are real input fields (topics.GetInput.TopicID json topic_id at internal/tools/topics/topics.go:93, appearance UpdateInput.Title json title at internal/tools/appearance/appearance.go:89, planlimits PlanName at plan_limits.go:31/62), and guidance is served at internal/tools/dynamic/register.go:163 and internal/resources/tool_manifest.go:917. Note too that appearance is the sharpest case: the orphan's appearance_update usage string is byte-identical to adminspecs' and the orphan carries all three guidance entries while adminspecs kept two, so title was dropped in a copy that was otherwise transcribed verbatim.

• cmd/audit_catalog_first reports all twenty-three as spec-backed with zero catalog actions
  The mechanism is exactly as described and I reproduced the output: cmd/audit_catalog_first/main.go:226 sets HasMetaSpecs from source.HasActionSpecsFunction, :242 ors it into HasIndividualTools, the action counts join on spec.OwnerPackage at :809-819, and a run with -output - prints surface_classification 'spec-backed', has_meta_specs true, has_individual_tools true, action_spec_count 0, meta_group '' for all 23 (I printed the topics row in full). What I dispute is the impact and therefore the severity. 'Both facts are already published to dist/action-spec-coverage.json for downstream readers' is false: dist/ is gitignored (.gitignore:13), nothing under dist/ is tracked, and by the investigator's own next finding nothing runs the command, so the file exists only on a machine where somebody ran it by hand and there are no downstream readers. The defect is a missing invariant in a record almost nobody reads, not a published false claim; medium, not high.

• The exact predicate that isolates the twenty-three with no false positives
  I ran the audit and reproduced the numbers exactly: over 180 domains, has_meta_specs && action_spec_count == 0 yields 26 (the 23 plus health, projectdiscovery, elicitationtools) and adding the three zero-checks yields precisely the 23 and nothing else. So the predicate is verified. I disagree with the fix sketch on two counts the investigator did not consider. First, 'start it EMPTY' cannot be done: with the gate in assertCoverageInvariants and an empty table, buildCoverageReport returns an error, cachedCoverageReport t.Fatalfs, and the whole cmd/audit_catalog_first test package turns red the moment the gate lands. The gate and the deletion have to be one change, or the table has to be seeded and drained, which is the thing the sketch warns against. Second, the predicate rests on OwnerPackage, and finding adminspecs-owner-package proposes to change exactly that: if admin actions start declaring topics/settings/sidekiq as their owner, action_spec_count becomes non-zero for all 23 and this gate silently stops firing while the dead functions are still there. The two proposals cancel each other and neither says so. The stricter go/packages form the sketch mentions in passing is the one that survives both; it should be the recommendation, not the footnote.

• A package protection rule update that names only an access level is refused by GitLab: client-go sends both string fields as explicit null
  The mechanism is real and I reproduced every step: client-go v3 protected_packages.go:85-90 tags both *string fields json:"package_name_pattern" / json:"package_type" with no omitempty; gitlab.go:1017-1025 marshals the whole option struct as the JSON body for PATCH; protectedpackages/protected_packages.go:150-156 sets them only when non-empty, and opts is never nil, so an access-level-only update does send two explicit nulls. docs/development/gitlab-api-live.json marks both optional on the PATCH route (I dumped the record: only id and package_protection_rule_id carry required:true). The 404-pinned hint at :165 is also real, so a 422 gets no hint at all. Three things the investigator overstated. (1) Severity high is not supportable: one action in one package, the caller can always work around it by resending both strings, and the values are visible from gitlab_list_package_protection_rules. Medium. (2) "The 422 is caused by the null, not by a GitLab requirement" rests entirely on a comment at test/e2e/gitlab/common/packages_test.go:262-264. The e2e test itself always sends package_type (line 269), so the failing shape is exercised by nothing in this tree, on any instance. The live record plus the comment point the same way, but nobody has observed an omitted package_type succeeding, because client-go cannot omit it. State that as inference. (3) The fixSketch ("read the rule from ListPackageProtectionRules and fill the omitted field") is worse than the defect: it turns every partial update into a read-modify-write with no concurrency control, it adds a round trip to every call, and List is paginated (protected_packages.go:87 builds ListPackageProtectionRulesOptions), so a rule past the first page needs a walk the sketch does not mention. The honest local fixes are the two cheap ones: drop omitempty from the two jsonschema tags in UpdateInput (protected_packages.go:37-38) so the surface says what the wire requires, and add a 422 branch to the hint chain.

• The unit test this branch added pins the null body and states a rationale the repository's own e2e test contradicts
  The facts check out: protected_packages_test.go:530-555 on the committed branch asserts the body is exactly the two nulls plus the two access levels, and its doc comment justifies that with "null is a field GitLab leaves alone where "" is a pattern that matches nothing", which the e2e comment at packages_test.go:262-264 contradicts. But "the test now defends the defect" overstates it. What the test asserts is a true and useful characterisation of what client-go emits, and a characterisation test of third-party behaviour is supposed to fail when the third party changes: an upstream omitempty landing and this test going red is the test doing its job, not a trap. Only the rationale sentence is wrong, and only that sentence needs changing. Severity low, comment-only, not medium.

• mrcontextcommits sends commits as an unconditional pointer with no local required-field guard, unlike every sibling handler
  The guard asymmetry is real and I confirmed it is unique in the batch. Scanning every &in./&input. taken into an option struct across the 41 packages gives five sites: mrcontextcommits:91 and :135 (unguarded), projectserviceaccounts:345-346 (guarded at :330-334 by explicit name and len(Scopes) checks), protectedenvs:258 (guarded at :250), securityattributes:557 (guarded by validateIDList at :548, which rejects len==0 at security_attributes.go:366-369). So mrcontextcommits is the only one, exactly as claimed. The impact is where it breaks. {"commits":null} is not reachable by any surface: the dynamic gate refuses an absent required param (dynamic/register.go:954-963), the meta gate does the same (toolutil/meta_tool.go:2616), and the individual surface's schema marks commits required. Only {"commits":[]} gets through. Grape's presence validator tests key presence, not blankness, so an empty array satisfies requires :commits and GitLab almost certainly answers 201 with an empty list rather than the 400 the finding assumes. The investigator asserts "comes back as its 400, wrapped with the hint ... which names a cause that is not the cause" with no evidence for the 400 at all. What is actually wrong is smaller: a no-op call that reports success. Worth the two-line guard, not worth a follow-up PR of its own.

• severity and report_type are lowercased for the caller, state and scanner are not, and only state is an enum GitLab matches exactly
  Every fact is right and none of them adds up to a defect. securityReportFindings takes severity and reportType as [String!] and state as [VulnerabilityState!] (gitlab-schema.graphql:22665); VulnerabilityState's four members are uppercase (:32137-32142); lowercased (security_findings.go:188-194) is applied only to severity and reportType at :348 and :354. But the served input schema documents all four filters in uppercase (security_findings.go:307-311, "Filter by state: DETECTED, CONFIRMED, DISMISSED, RESOLVED"), and the documented spelling works for every one of them. No caller following the schema it was given can reach the failure. The stated impact — "a model that learns from the severity filter that case does not matter" — is speculation about a caller deviating from the schema: nothing in the served surface reveals that severity is lowercased on the way out, since lowercased is internal and the description says uppercase. So this is an asymmetry in how forgiving two filters are, which is a nit, not a bug. Uppercasing state would be safe and cheap if someone is in the file anyway, but it does not deserve a follow-up PR and should not be filed as a confirmed defect.

• metadata, planlimits and sidekiq each export an ActionSpecs nothing calls, and two of the three have drifted from what is served
  The core fact holds: metadata.ActionSpecs, planlimits.ActionSpecs, sidekiq.ActionSpecs, securefiles.ActionSpecs and settings.ActionSpecs have no caller outside their own package, and the served specs are built in internal/tools/adminspecs/action_specs.go (:134-139, :173-176, :265). Two pieces of the evidence are wrong, and both cut against the recommendation. (1) 'Only securefiles is declared' is false. main_test.go:410-414 and main_test.go:367-386 call the SAME helper, assertSourceSpecBackedDomains (main_test.go:521-532); securefiles is not held to anything metadata, planlimits, settings and sidekiq are not. (2) That helper is not silent about the dead builder - it is precisely an assertion about it. domainCoverage.HasMetaSpecs is seeded from source.HasActionSpecsFunction (cmd/audit_catalog_first/main.go:226) and HasIndividualTools from the same flag (:242); since the served specs carry OwnerPackage "adminspecs", the catalog contributes zero actions to these packages, so both flags depend only on the presence of func ActionSpecs in the package source. Delete the four builders and classifySurface (main.go:930-955) returns no-gitlab-surface, and TestBuildCoverageReport_AdminPlatformDomainsAreSpecBacked fails. So deletion costs a gate edit; 'deletion is the cheaper answer' is backwards. (3) 'byte-identical' for metadata is not quite true: adminMetadataGetSpec inherits OwnerPackage "adminspecs" from adminOptions while metadataOptions sets "metadata", and OwnerPackage is exactly the field R-PATH joins the request inventory on. Usage, aliases, tags, related and description do match. The drift claims for planlimits and sidekiq I did verify and they are accurate.

• Three packages carry a generic Usage and RelatedActions in the shared initializer that no action can ever be served
  The observation that every registered action overwrites the literal is correct - I counted 9 overrides against 9 specs in repository (8 switch cases plus repositoryCompareSpec at :23-29), 7 against 7 in milestones, 5 against 5 in mrnotes. The conclusion does not follow, and the sentence 'nothing tests it, because there is nothing to test' is false in two of the three packages. internal/tools/milestones/milestones_test.go:1582-1602 (TestMilestoneOptionsForAction_UnknownName_KeepsTheBaseMetadata) calls milestoneOptionsForAction with an unmapped name and asserts the generic Usage AND the non-empty base RelatedActions come back, then loops over ActionSpecs asserting no registered action falls through - so removing the literals breaks that test, exactly like the guards the investigator agrees are load-bearing. internal/tools/mrnotes/action_specs_test.go:25 names the literal as genericMRNoteUsage and asserts no action carries it. And the placeholder is not an inert string a maintainer might misread: cmd/internal/auditshared.IsGenericUsage (audit_shared.go:17-24) matches it by regexp and is read by cmd/audit_discovery_completeness/main.go:351 and cmd/audit_1to1/internal/metadata/analyze.go:102, both of which treat an empty Usage identically - so a tenth repository action falling through would be reported as an undecorated action either way, and deleting the literal buys nothing while losing the sentence. The only residue I would keep is that internal/tools/repository has no equivalent test (its repository_test.go asserts only that three named specs have non-empty Usage, at :1705-1719), so if anything is owed here it is a milestones-style test in repository, not a deletion.

• Three of the four commands the synthesis proposes extending run nowhere and one of them cannot fail
  Two of the three claims hold and the load-bearing one does not. Holds: cmd/audit_surface_quality/main.go:20-47 has no -check and no non-zero exit but for a bad -view, and I ran it - "Total violations: 0" today, so a -check is free (but it must exclude the envelope section, which reports "Registration problems | 10" right now and would fail on day one). Holds: no workflow under .github/workflows/ names any of the three. Does not hold: "every gate the synthesis proposes to add to these commands would be added to a report nobody runs". cmd/audit_catalog_first's rules already gate, through its own tests, which CI runs - .github/workflows/ci.yml:104 is go test -count=1 ... ./cmd/... ./internal/..., and cmd/audit_catalog_first/main_test.go:104 TestAuditCatalogFirstSource_CurrentProductionCodePasses calls auditCatalogFirstSource against the real tree via cmdutil.RepositoryRoot("../.."), while cachedCoverageReport at main_test.go:41-54 builds the real coverage report and the TestBuildCoverageReport_*AreSpecBacked family (main_test.go:256-464) asserts on it. So a rule added to auditCatalogFirstSource - which is exactly where the class-8 fix is proposed - fails CI the day it is written, with no Makefile or workflow change at all. Also partly wrong on audit_discovery_completeness: it does have a working gate, cmd/audit_discovery_completeness/main.go:165 (-check) and :178 (os.Exit(1)) with a -severity threshold; it is unwired, not unfailable. The practical consequence is that the finding's recommended ordering ("Do this before, not after, adding any new rule to them") is wrong for two of the three commands and only correct for audit_surface_quality.

• Class 0 (the request the handler builds is never decoded whole): gateable, and the cheap oracle is the request inventory, not gremlins
  The proposed rule fails on its own worked example. The fixSketch says: union the recorded body field names across a package's rows, and report an option field our schema can set that appears in no recorded body. I read the committed inventory on the branch (git show sweep-batch2-tail:docs/development/request-inventory.json): internal/tools/protectedpackages has a POST row with body [package_name_pattern, package_type] AND a POST row with body [minimum_access_level_for_delete, minimum_access_level_for_push, package_name_pattern, package_type], plus the matching PATCH pair. The union therefore contains all four fields and the rule reports nothing for the very package the class was named from. The finding's own prose gives a different rule - "the narrower row IS the unexercised-guard signature" - and that rule is not sound either, because the two rows are not one endpoint with two bodies: they differ in the path (/projects/:project_id/... versus /projects/myproject/...), which internal/testutil/request_shape.go:47-65 explains is what happens when a fixture uses a word-shaped identifier. Narrow rows are an artifact of which test issued which call, so every package with more than one test would carry them and the signal would be noise. A third problem: the helper named as the machinery, structs.CollectOutputPairings, is at cmd/audit_1to1/internal/structs/pairings.go:65 and is the OUTPUT pairing; there is no exported input-pairing collector in that package (only CollectPairs in analyze.go:1219), so the R-INPUT side would have to be built, which the cost estimate ("one read of a committed file") does not account for. The underlying class is real - the sweep's LIVED guards prove it - but the honest verdict is that the inventory cannot see it, because a guard that some test exercises and other tests do not still contributes its field to the union. make coverage-mutants (Makefile:631-694) remains the only detector, as the finding concedes in its last sentence, which is in tension with its own title.

• The exact text to append to the sweep brief before the next batch of forty-seven
  The six classes are each real and I verified their factual claims: 15 packages in patterns[0] never read a request body; 9 in patterns[3] dispatch metadata on the action name; 23 packages export an unreferenced ActionSpecs (checked one by one, zero external references for all 23 adminspecs imports); snippetnotes really does cross-link snippet.get / snippet.list, which /tmp catalog dump shows are the PERSONAL snippet tools while the project ones are snippet.project_get / snippet.project_list; RelatedActions really does reach no committed artifact (internal/tools/testdata/tools_.json carry name, description, inputSchema, outputSchema, annotations only, and neither they nor llms.txt contain the string related_actions); the em-dash figure is 2056 across 406 files under internal/, not 2057/407. Three defects in the text as written. (1) It contradicts itself: section 6's metadata-dispatch paragraph tells an agent to 'delete it and pass the metadata as an argument, so the compiler refuses a spec without it', which is a production refactor of action_specs.go inside a test sweep, i.e. exactly the class the securitycategories reviewer filed as blocking and exactly what section 7's second rule forbids. (2) The canonical-ID paragraph frames the class as three defeating spellings with a handful of examples. Measured against the real catalog (1089 IDs from --tool-search a at tier ultimate), 480 RelatedActions entries in 51 packages name a string that is neither a catalog ID nor any alias literal anywhere in internal/tools nor an individual tool name, plus 56 more in runners and runnercontrollers that resolve only because those two put individualTool first in Aliases: 536 of the 2801 entries this shape publishes, about 19 percent. Told 'check every one' without being told the class is near-universal, an agent reads a hit as an exception. (3) 'a whole-tree check is impossible' is the wrong lesson and the seed's own gateGaps already give the right one: the check must be diff-scoped (git diff origin/main...HEAD, added lines plus the message body), which works today and is what I used. Separately, the front asked for text 'as short as it can be and still work'; the append is roughly 1,000 words onto a 1,529-word brief, a 70 percent increase to a document the front itself says gets skimmed.

• Twenty-three packages export an ActionSpecs nothing outside their own tests reads, not fifteen
  The count is right: all 23 packages imported by internal/tools/adminspecs/action_specs.go:5-27 declare a func ActionSpecs and have zero references to <pkg>.ActionSpecs anywhere outside their own directory (checked individually across internal/, cmd/ and test/). The settings drift is real too: adminspecs/action_specs.go:247-255 gives settings_get a usage sentence, five aliases, three related actions and a description, while settings/action_specs.go:28-49 gives it a generic usage, one alias and one related action, and nothing reads the latter. But the evidence sentence is false. There are TWO calls to assertSourceSpecBackedDomains, at main_test.go:367 (19 names: applications, appearance, appstatistics, broadcastmessages, bulkimports, clusteragents, customattributes, dbmigrations, features, health, license, metadata, namespaces, planlimits, settings, sidekiq, systemhooks, topics, usagedata) and at :410 (dependencyproxy, errortracking, securefiles, terraformstates), so 21 of the 23 are already named, not four; the seed's gateGaps carries the same error and the finding inherited it. And the helper (main_test.go:521-532) asserts a domain's SurfaceClassification is 'spec-backed' and that it has individual tools and meta specs -- it says nothing about an exported symbol with no caller, so 'extending assertSourceSpecBackedDomains' is the wrong place. A dead-exported-symbol gate is a new pass over the type checker's record, not a longer string list in a coverage-report assertion.

• A package whose ID constants are concatenated can never reach Not covered: 0
  I reproduced the measurement exactly: make coverage-mutants PKG=./internal/tools/runnercontrollerscopes through rgo prints 'Killed: 46, Lived: 0, Not covered: 5' with all five reported as NOT COVERED ARITHMETIC_BASE at action_specs.go:32:44, :33:44, :34:44, :35:44 and :36:44, which is the const block at action_specs.go:31-37 (catalogScopeList = runnerDomain + actionScopeList and four siblings). The finding's citation 'action_specs.go:32:36' names a column that does not exist; the lines are 32 through 36, column 44. Where I disagree is the severity and the impact. The brief's 'What done means' already reads 'Either Lived: 0 and Not covered: 0, OR every remaining survivor named with the shape it matches', so an agent is already given the exit, and step 3 already sends them to docs/development/testing/README.md, whose 'A tool artifact' bullet names package-level constant initializers explicitly and whose next paragraph works the 22 cmd/server instances of this exact mutation. A third statement of a fact stated twice is worth one sentence, not a section, and 'an agent that reads only the first clause cannot finish' describes an agent who skipped the sentence the clause is in.

• docs/development/request-inventory.json is stale: 13 new rows, 6 superseded. Nobody reported it and the gate is not deferrable
  Refuted outright. The branch tip 9b9f166 IS the regeneration the finding asks for: chore: record the requests the batch 2 sweep's new tests issue, +77/-3 lines in docs/development/request-inventory.json. The committed file now holds 1587 rows (python: len(d['requests'])), and 9b9f166^ held exactly 1580 — the very pair the finding reports as "1580 committed against 1587 recorded". Every row it names is present: internal/tools/notifications PUT /notification_settings carries all 19 body keys, and the four untemplated-segment rows are there too (internal/tools/projectaliases GET|DELETE /project_aliases/missing-alias, internal/tools/pages GET /projects/:project_id/pages/domains/shop.example.com, internal/tools/projecttemplates GET /projects/:project_id/templates/gitignores/Go, three internal/tools/snippetdiscussions .../discussions/d9/notes rows). The commit message even names the untemplated-segment limitation the finding offers as its fixSketch. The investigator measured a pre-tip state and reported it as the tip; the working tree is clean at 9b9f166.

• "Showing N of 424 settings GitLab returned" counts client-go's struct, not GitLab's answer, and the count is a constant
  Broken as written. The sentence the title and impact quote, "Showing N of 424 settings GitLab returned", is produced only by the shown >= total branch of settingsCoverage (internal/tools/settings/markdown.go:210-211: "Showing all %d settings GitLab returned."). Through the handler that branch cannot fire: values is always the 424-key map from the json.Marshal/Unmarshal round trip of *gl.Settings (settings.go:34-42), while shown counts the 27 curated keys in settingCategories (markdown.go:39ff — I counted the quoted keys: 27). The only sentence a caller sees is the :213 branch, "Showing 27 of 424 settings; the remaining keys are in the structured result" — which carries no "GitLab returned" attribution at all, and whose second half is true, because the structured result really does carry all 424. The "Showing all N settings GitLab returned" string appears only where the tests construct GetOutput by hand (settings_test.go:172, :254). The one true residue is that 424 is a property of the pinned SDK rather than of the instance, and markdown.go:32-38 already says the count was deliberately narrowed once before. The real defect is the next finding; this one should not be filed separately.