Skip to content

Narrow a cross-link to the session that is shown it - #849

Merged
jmrplens merged 1 commit into
plan-l12from
plan-l13
Sep 20, 2026
Merged

jmrplens merged 1 commit into
plan-l12from
plan-l13

Conversation

@jmrplens

@jmrplens jmrplens commented Sep 20, 2026 •

Copy link
Copy Markdown
Owner

The RelatedActions of an ActionSpec are written against the whole catalog, and every session is served a narrower one. Nothing reconciled the two, so a Free instance was handed cross-links to Premium actions it does not serve, and an --exclude-tools deployment links to actions it had removed. Following one answers "unknown action" plus near-miss suggestions, which reads as a spelling correction and teaches a model the capability is absent, the same misdiagnosis withheldActionMessage already exists to prevent.

The number in the plan, 114 ids in 28 packages, is a count of every licensed-only id named from anywhere at Ultimate, and most of those vanish with the licensed action that names them. Measured on the Free catalog the harm is four links in three packages, all of them Free actions naming Premium siblings: group.group_milestone_merge_requests to group.group_milestone_burndown, merge_request.approval_config and merge_request.approval_reset to merge_request.approval_rules and approval_state, and project.mirror_list to project.pull_mirror_get. The two examples the plan cites, merge_request.dependencies_list and project.approval_rule_create, are named only by licensed siblings and are not among them.

Registry.publishedRelatedActions decides by what following the link would answer, which is the only thing that costs a model anything. A link that resolves is published under its canonical id, whatever spelling it was written in. A link a filter withheld is kept, because asking for that one produces the narrowing and its remedy rather than a typo correction, and dropping it would leave a model concluding the server cannot do it at all. A link that is neither is dropped: the tier filter runs before the catalog is built and so never reaches the withheld lists, and naming an excluded tool would contradict the exclusion as well as mislead. Find, describe and the catalog search all publish the same list.

The second half is the spelling. 60 related entries in runners and runnercontrollers named individual tool names, which resolve for gitlab_execute_action because the decoration makes the tool name an alias, and which gitlab_find_action lists under no name at all. They are canonical action ids now, declared beside the ids the Markdown hints already use and covered by the same test, and the two packages' own cross-link tests check the ids rather than the tool names.

cmd/audit_action_ids settles the question its doc.go deferred to "the layer that fixes the cross-links". An alias is judged by where it is written: in a RelatedActions entry or a HintAction argument it is a finding with the canonical id named beside it as the fix, and in a Usage line or a description it stays reported apart, because naming an alias can be the substance of the sentence. issue.update's usage says that dynamic execute also accepts issue.close and issue.reopen, which is true and useful and would be a defect under one rule for both. The tree reports zero findings under the new rule and the two prose aliases apart. The work list is schema version 2, since the field set is unchanged and the counts are not.

Two shapes had to keep the audit's eyes. Its walker cannot fold a list a projection narrows, so its pass-through rule now accepts a call handed the value an ID list hangs off as well as one handed the list, with the negative half pinned by a fixture. And resetScopedRegTokenEntry takes each sibling as two plain strings whose id halves keep the "related" prefix, because the walker folds a helper's arguments back to the constants its callers pass and follows a parameter out to its callers only when the parameter's name says it carries action ids; tidied into a struct, those two cross-links leave the audit's sight.

The semantic half of the same finding is recorded rather than gated. Whether snippet.get is the right snippet is a claim about the object an action reaches, which no set of names carries, so it is a review and doc.go now says so instead of leaving it open.

docs/development/testing/testing.md would move: the dynamic package gains three test functions, 269 to 272, and the total 9758 to 9761. No golden snapshot, llms file or manifest moves, since no served description changed and no committed artifact carries related actions.

(cherry picked from commit c72a1a029b0f64256f8bcb07cb4b260b6c50aab1)

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @jmrplens, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 1 day and 13 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 10 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: jmrplens/gitlab-mcp-server/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 584d07c5-73e1-42ad-943b-27a7a7717b0f

📥 Commits

Reviewing files that changed from the base of the PR and between ae0897b and bfa0b52.

📒 Files selected for processing (16)
  • CLAUDE.md
  • cmd/audit_action_ids/doc.go
  • cmd/audit_action_ids/report.go
  • cmd/audit_action_ids/report_test.go
  • cmd/audit_action_ids/source.go
  • cmd/audit_action_ids/source_test.go
  • docs/development/tool-surfaces-and-action-core.md
  • internal/tools/action_specs_test.go
  • internal/tools/dynamic/register.go
  • internal/tools/dynamic/register_test.go
  • internal/tools/runnercontrollers/action_specs.go
  • internal/tools/runnercontrollers/action_specs_test.go
  • internal/tools/runnercontrollers/markdown.go
  • internal/tools/runners/action_specs.go
  • internal/tools/runners/action_specs_test.go
  • internal/tools/runners/markdown.go

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the tooling The audit and generator commands under cmd/, and the Makefile targets that run them label Sep 20, 2026
@jmrplens
jmrplens added this pull request to stack #863 September 20, 2026 10:16
@jmrplens jmrplens added this to the 3.1.0 milestone Sep 20, 2026
@github-actions github-actions Bot added the v3.1.0 Targeted at the 3.1.0 release label Sep 20, 2026
@jmrplens
jmrplens force-pushed the plan-l13 branch 2 times, most recently from 07e1c20 to fbe40ce Compare September 20, 2026 14:43
@jmrplens
jmrplens removed this pull request from stack #863 September 20, 2026 19:40
@jmrplens
jmrplens added this pull request to stack #866 September 20, 2026 19:40
The `RelatedActions` of an `ActionSpec` are written against the whole
catalog, and every session is served a narrower one. Nothing reconciled
the two, so a Free instance was handed cross-links to Premium actions it
does not serve, and an `--exclude-tools` deployment links to actions it
had removed. Following one answers "unknown action" plus near-miss
suggestions, which reads as a spelling correction and teaches a model
the capability is absent, the same misdiagnosis `withheldActionMessage`
already exists to prevent.

The number in the plan, 114 ids in 28 packages, is a count of every
licensed-only id named from anywhere at Ultimate, and most of those
vanish with the licensed action that names them. Measured on the Free
catalog the harm is four links in three packages, all of them Free
actions naming Premium siblings: `group.group_milestone_merge_requests`
to `group.group_milestone_burndown`, `merge_request.approval_config` and
`merge_request.approval_reset` to `merge_request.approval_rules` and
`approval_state`, and `project.mirror_list` to
`project.pull_mirror_get`. The two examples the plan cites,
`merge_request.dependencies_list` and `project.approval_rule_create`,
are named only by licensed siblings and are not among them.

`Registry.publishedRelatedActions` decides by what following the link
would answer, which is the only thing that costs a model anything. A
link that resolves is published under its canonical id, whatever
spelling it was written in. A link a filter withheld is kept, because
asking for that one produces the narrowing and its remedy rather than a
typo correction, and dropping it would leave a model concluding the
server cannot do it at all. A link that is neither is dropped: the tier
filter runs before the catalog is built and so never reaches the
withheld lists, and naming an excluded tool would contradict the
exclusion as well as mislead. Find, describe and the catalog search all
publish the same list.

The second half is the spelling. 60 related entries in `runners` and
`runnercontrollers` named individual tool names, which resolve for
`gitlab_execute_action` because the decoration makes the tool name an
alias, and which `gitlab_find_action` lists under no name at all. They
are canonical action ids now, declared beside the ids the Markdown
hints already use and covered by the same test, and the two packages'
own cross-link tests check the ids rather than the tool names.

`cmd/audit_action_ids` settles the question its doc.go deferred to "the
layer that fixes the cross-links". An alias is judged by where it is
written: in a `RelatedActions` entry or a `HintAction` argument it is a
finding with the canonical id named beside it as the fix, and in a
`Usage` line or a description it stays reported apart, because naming an
alias can be the substance of the sentence. `issue.update`'s usage says
that dynamic execute also accepts `issue.close` and `issue.reopen`,
which is true and useful and would be a defect under one rule for both.
The tree reports zero findings under the new rule and the two prose
aliases apart. The work list is schema version 2, since the field set is
unchanged and the counts are not.

Two shapes had to keep the audit's eyes. Its walker cannot fold a list a
projection narrows, so its pass-through rule now accepts a call handed
the value an ID list hangs off as well as one handed the list, with the
negative half pinned by a fixture. And `resetScopedRegTokenEntry` takes
each sibling as two plain strings whose id halves keep the "related"
prefix, because the walker folds a helper's arguments back to the
constants its callers pass and follows a parameter out to its callers
only when the parameter's name says it carries action ids; tidied into a
struct, those two cross-links leave the audit's sight.

The semantic half of the same finding is recorded rather than gated.
Whether `snippet.get` is the right snippet is a claim about the object
an action reaches, which no set of names carries, so it is a review and
doc.go now says so instead of leaving it open.

`docs/development/testing/testing.md` would move: the dynamic package
gains three test functions, 269 to 272, and the total 9758 to 9761. No
golden snapshot, llms file or manifest moves, since no served
description changed and no committed artifact carries related actions.

(cherry picked from commit c72a1a029b0f64256f8bcb07cb4b260b6c50aab1)
@sonarqubecloud

Copy link
Copy Markdown

@jmrplens
jmrplens merged commit 6cefaba into main Sep 20, 2026
37 of 41 checks passed
@jmrplens
jmrplens deleted the plan-l13 branch September 20, 2026 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tooling The audit and generator commands under cmd/, and the Makefile targets that run them v3.1.0 Targeted at the 3.1.0 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant