Repository navigation
feat(cli): Declare and lint a completion kind for every flag - #1759
Merged
Merged
Conversation
Every visible value-taking flag now says how shell completion handles its value, and test/lib/completion-coverage.test.ts fails, naming the command and flag, for any flag without a kind. There is no allow-list. - --fields completes the output schema's field paths, nested ones in dot notation, one comma-separated item at a time, leaving out the fields already listed (new choice-list kind, choiceListParser). - Shared parsers get kinds through PARSER_TARGETS: numberParser and sanitizeQuery complete nothing; parseCursorFlag, parseLogSort, parseSpanDepth, parseInstallationMethod, and parseReleaseChannel complete the values they accept. parseCursorFlag moves to arg-parsing.ts so completion-target.ts can import it without a cycle. - A command's own validating parser keeps its checks when wrapped: choiceParser(values, parse) completes values (sort, status, format, method, datasets), textParser(parse) completes nothing (ports, limits, thresholds). - Every parse: String flag uses a named parser: paths complete files or directories, --platform completes platforms, org list --account completes accounts, --scope/--features/--type complete their lists, --timezone completes tz names, and the rest are parseText.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
The viewer integration tests import `commands/local/server.ts`, which now reaches `lib/arg-parsing.ts` through the tagged flag parsers in `completion-target.ts`. That module imports the generated grammar, so a fresh CI checkout with only the API schema generated failed to resolve `org-project-target-grammar.js`. `ensure-generated.mjs` now takes the groups to check, and the viewer test asks for the schema and the grammar.
BYK
requested changes
Oct 30, 2026
BYK
left a comment
BYK
left a comment
Member
There was a problem hiding this comment.
Stopped at 41 files as it is apparent that you left many String parsers at parseText even though it is easy to see they are not random/free-form text. Let's do this properly.
Resolve the list-command.ts import conflict: keep main's removal of the createRequire route lookup and this branch's import of parseCursorFlag from arg-parsing.ts. Add the flagNames field that main's extractor now returns to the branch's new introspect test. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
Add sentry-cli/require-completion-parser. In src/commands/ and src/lib/ it rejects, on command positionals and parsed flags, the parsers that can never carry a completion kind: built-ins (String, Number, Boolean, BigInt, Date, parseInt, parseFloat, JSON.parse, Number.parseInt, ...), inline functions, and boundArguments without a kind. Named parsers pass lint; completion-coverage.test.ts still checks at runtime that each one is tagged. Parameters that must stay without a kind go in lint-rules/completion-exemptions.json, with file, command, parameter and reason. The lint rule and the coverage test both read it, and the test fails on an entry whose parameter is covered or gone. The list starts empty: no current parameter needs it, and the rule finds nothing in the current code. An oxlint-disable comment silences only the lint rule. The coverage test cannot see comments, so the parameter still needs a kind or an entry. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
sentry-junior
Bot
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
require-completion-parser named any parameter with a `placeholder` as a positional (`<method>`), but flags such as `api --method` declare placeholders too, and the coverage test names them `--method`. An exemption written as `--method` never matched in the lint rule, and one written as `<method>` was flagged as stale by the test. Name a parameter by its property key unless that key is an array positional's `parameter`, and fall back to the placeholder only for positionals. Add RuleTester cases for an exempt flag with a placeholder and an exempt array positional.
BYK
requested changes
Nov 1, 2026
BYK
left a comment
BYK
left a comment
Member
There was a problem hiding this comment.
Stopped at 41 files as it is apparent that you left many String parsers at parseText even though it is easy to see they are not random/free-form text. Let's do this properly.
You have not addressed this comment of mine?
Many flags parsed with parseText take values that are not free text: things that exist in Sentry, git refs, fixed sets, or a grammar. Give them kinds that complete them: - Cached entities as flag values (environments, releases, teams, team:<id> owners, monitors, alert workflows, native integrations, installed Sentry Apps and their issue-link form fields, DSNs, project lists), scoped to the org the command's target names. completion-seed now lets one response seed several kinds (teams give --team and --owner). - issue merge --into completes issues; issue archive --until completes its grammar (forever, auto, 10x, 10u, durations, 10x/5m). - Git refs, commit SHAs, remote owner/repo names, and --commit REPO@SHA lists from the repository in the shell's directory. - api endpoints complete API schema paths one segment at a time, with cached orgs and projects for their placeholders; schema completes API resources; auth --url completes stored accounts' servers. - Fixed sets from the modules that validate them: metric alert time windows, dashboard grid columns and widths, preprod VCS providers (Sentry's SUPPORTED_VCS_PROVIDERS), tracemetrics aggregations, Local loopback hosts, sourcemap extensions, event send --user keys. Shells no longer add a space after a suggestion that continues (name=, repo@, id:), as they already did after org/.
Finish the audit of values still parsed as free text: - The hidden --project every command takes completes cached projects of the target's org, like the hidden --org completes orgs. - issue tags <key> completes the tag keys issue details list (their `tags`), cached from both scoped and unscoped issue lookups. - dashboard widget add/edit --query and --sort complete the aggregate function names and aliases validateAggregateNames accepts.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 342cd58. Configure here.
COMPLETION_VALUES walked the API schema when completion-target.ts was imported, which every command does. Only generate:completions needs the paths; the __complete fast path reads them from the generated JSON. Move the walk to getApiCompletionPaths in api-schema.ts, called by the generator (and the test that checks the generated file).
BYK
approved these changes
Nov 1, 2026
BYK
enabled auto-merge (squash)
November 1, 2026 15:55
local serve and local run --host bind any address, not only loopback. Complete the loopback hosts first (the only ones that keep the UI actions), then 0.0.0.0 and the address of each network adapter from os.networkInterfaces(), described by adapter. IPv6 link-local addresses are left out: they cannot be bound without a zone. The daemon's --host, which must be loopback, still completes only the loopback hosts.
sentry-junior Bot
added a commit
that referenced
this pull request
Nov 1, 2026
Brings in #1773 (@sentry/logger) and #1759 (flag completion kinds). Conflicts: - .craft.yml, merge-jobs.yml, release.yml: keep main's logger release workspace, branch, packaging, and product; keep the removal of the separate mcp release (the CLI release publishes @sentry/mcp-server). - packages/cli/package.json: depend on @sentry/logger and @sentry/mcp-core, not the retired @sentry/mcp-server workspace. - pnpm-lock.yaml: regenerated with pnpm install. Follow-ups for the merged tree: - `sentry mcp` string flags parse with parseText, as #1759's lint rule requires. - completion-target.ts now imports commands/local/loopback.ts, so allow that leaf module in the mcp-server bundle's CLI-only input check. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
This was referenced Nov 1, 2026
BYK
added a commit
that referenced
this pull request
Nov 1, 2026
…#1782) This follows up #1759. The field and aggregate flags that were still `text` now complete the fields of the dataset that `--dataset` names, plus its aggregate functions. **Now completing** - `explore --field` / `--sort`: fields, then `fn()`. Once a function name is typed in full, it also completes `fn(field)` for every field, so `p95<TAB>` inserts `p95(` and lists `p95(span.duration)` and the rest. `p95(span.d<TAB>` finishes the call. - `alert metrics create|edit --aggregate`: the same calls, without bare fields. - `dashboard widget add|edit --query` / `--sort`: the shorthand form, `fn` and then `fn:field`. These used to complete every dataset's function names. Now they complete only the names of the widget's dataset, with the aliases (`spm`, `tpm`, …) that resolve to them. - `dashboard widget add|edit --group-by`: fields. **Where the values come from** - Spans, logs, and trace metrics: attribute keys, as the new cache kinds `span-attribute`, `log-attribute`, and `metric-attribute`. They are seeded from `GET /organizations/{org}/trace-items/attributes/`. The seed table can now also match on query parameters, so the `dataset` parameter decides which kind a response fills. The stored value is the attribute's `key`, because the API queries by `key` (`tags[foo,number]` for custom non-string attributes). I checked this against `as_attribute_key` in `src/sentry/api/endpoints/organization_trace_item_attributes.py` in getsentry/sentry. The description is the attribute type, with the display name in front of it when the name differs from the key. - Errors: the cached issue tag keys (`tag-key`). - Replays: the fixed lists the CLI already validates against. `--field` uses `listSupportedReplayFields()` and `--sort` uses `REPLAY_SORT_FIELDS`. - Aggregate functions: `aggregateFunctionsOf` in `types/dashboard.ts`. `validateAggregateNames` now uses the same function, and it replaces `AGGREGATE_FUNCTION_NAMES`. **How it works** - A new `DatasetCompletion` tag on the parser describes what completes for each dataset, the command's `--dataset` aliases, and its default dataset. - The generator writes this per flag. To keep the JSON smaller, it is keyed only by `--name`; aliases are mapped back to it at completion time. - The completer now gets `flagValues` from the words typed before the cursor, so it sees `--dataset spans`, `-d spans`, and `--dataset=spans`. - With no `--dataset`, a command falls back to its default: errors for `explore`, spans for `widget add`. `widget edit` keeps the existing widget's dataset, and `alert` has no default, so those two complete every dataset. - The fast path still makes no API calls and loads nothing new. **Still completing nothing** - A descending `-field`: a word that starts with `-` completes flag names. - Trace metric aggregates (`sum(value,metric,type,unit)`): they name a metric rather than a field. Only trace metric fields complete. - Crash-rate alerts (`--dataset metrics|sessions`) and `issue` / `preprod-app-size` widgets. - Spans, logs, and trace metrics complete nothing until `sentry attribute list` has run for the org, since that command is the only caller of the attributes endpoint. Errors complete nothing until some issue details have been fetched. **Open questions** - The CLI has no local validator for alert `--aggregate`, so it reuses the widget function lists: errors functions for `errors`/`events`, spans functions for `spans`/`logs`. Should alerts get their own, narrower list? - Should `explore` (or a light background call) also seed attribute keys, so completion does not depend on running `attribute list` first? - Each kind keeps at most 500 cached entries per org, the existing limit. Orgs with many custom span attributes will only see the most recently seen 500. <!-- junior-request-attribution:start --> via @BYK. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3Ad44a6d899e425504b37476161fdb6ebe --> -- [View Session in Sentry](https://sentry.sentry.io/explore/conversations/agent%3Ad44a6d899e425504b37476161fdb6ebe/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: BYK <126780+BYK@users.noreply.github.com>
BYK
added a commit
that referenced
this pull request
Nov 1, 2026
Follow-up to #1759. `local session view|close|reset|retention`, `local envelope list|view`, and `local event list|view` now complete the session: IDs of the running daemon's sessions, described by label and state (`web (open)`), then labels that only one session has (the commands accept those too). **Where the sessions come from.** Sessions exist only in the daemon's memory, and `sentry __complete` makes no network calls. So the daemon now keeps a small `local-sessions.json` next to its discovery state. It is written atomically with owner-only permissions, the same way as `local-control-plane.json`. The daemon rewrites it when a session is created, closed, or expires, and removes it on shutdown. It holds only `id`, `label`, `state`, and `expiresAt`. The serializer picks those fields explicitly, so capabilities, cwd, and telemetry never reach the file. The completer: - ignores the file once the PID that wrote it has exited - leaves out retained sessions past their expiry and any malformed entries - loads its reader with a dynamic import, so the eager completion path is unchanged A stale entry can only suggest a session that the command then reports as not found. I chose this over calling the daemon's `/sessions` endpoint from the completer. That would put a loopback request on every TAB press, add a timeout to tune, and make the completer read the control token. The snapshot keeps the fast path file-only. The cost is one small file write per session lifecycle change. Activity updates don't trigger a write, because every ingested envelope records activity. **What's not covered.** The second word of `local envelope view <session> <envelope>` / `local event view <session> <event>` still completes nothing. Envelope and event IDs exist only in the daemon's retention buffers. Mirroring them would mean a disk write on every ingested envelope, plus decoding for events, on the daemon's hot path. Labels with whitespace are also left out of the suggestions, because they don't stay one shell word. Open question: should the snapshot also carry `cwd`, so the description can tell apart same-named sessions from different directories? I left it out to keep the file minimal. <!-- junior-request-attribution:start --> via @BYK. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3Affd815547d5bcdd5631e5dc512897f8a --> -- [View Session in Sentry](https://sentry.sentry.io/explore/conversations/agent%3Affd815547d5bcdd5631e5dc512897f8a/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: BYK <126780+BYK@users.noreply.github.com>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1751, which did this for positionals. Now every visible value-taking flag declares how shell completion handles it, and both lint and the test suite fail when a new positional or flag doesn't.
What changed
test/lib/completion-coverage.test.tsnow checks every visible, non-boolean flag on every visible command, not just positionals. A failure names the command and the flag.sentry-cli/require-completion-parser(insrc/commands/andsrc/lib/). It rejects parsers on positionals and parsed flags that can never carry a completion kind: built-ins (String,Number,Boolean,BigInt,Date,parseInt,parseFloat,JSON.parse, …), inline functions, andboundArgumentswithout akind. Named parsers pass lint; the coverage test still checks at runtime that each one is tagged. It reports nothing on the current code.lint-rules/completion-exemptions.json(file, command, parameter, reason). The lint rule and the coverage test both read it, and the test fails on an entry that no longer needs exempting. It starts empty. Anoxlint-disable-next-line sentry-cli/require-completion-parser -- reasoncomment silences only the lint rule; the coverage test cannot see comments, so the parameter still needs a kind or an entry.choice-listkind for comma-separated values.--fields id,ti<TAB>offersid,title, leaves out fields already chosen, and handles nested dot paths.buildFieldsFlagtags it once, and the generator writes each command's schema field names.PARSER_TARGETSnow covers the shared numeric, cursor, and limit parsers (text). It also covers fixed-value parsers such as sort, status, format, span depth, install method, HTTP method, and release channel. Their values come from the modules that define them; the lists are not duplicated.--platformcompletes from the platform list. Path flags complete as files or directories.--queryuses issue-query where the query is an issue search.parse: Stringis replaced with a named, tagged parser fromcompletion-target.ts. AddedtextParser(parse), andchoiceParsernow accepts an optionalparse.list-command.tstoarg-parsing.tsto avoid an import cycle.packages/cli/AGENTS.mdto cover flags, the lint rule, and the allow-list.parseText):--environment,--release,--team,--owner(team:<id>),--workflow,--integration,--app,issue link --field(name=valuefrom the apps' issue-link forms),--dsn,monitor run <slug>,--projectlists, the hidden--project,issue merge --into(issues),issue tags <key>(tag keys from issue details).issue archive --until(forever,auto,10x,10u, durations,10x/5m);api <endpoint>(schema paths one segment at a time, with cached orgs and projects for placeholders);schemaresources.owner/reporemotes, and--commit REPO@SHAlists, from the repo in the shell's directory.--query/--sortaggregate functions, preprod VCS providers, tracemetrics aggregations, sourcemap extensions,event send --userkeys,auth --urlstored servers.local serve/local run --hostcomplete the loopback hosts, then0.0.0.0and each network adapter's address; the daemon's loopback-only--hostcompletes loopback hosts.name=,repo@,id:).Visible non-boolean flags: 466. Without a kind: 447 before, 0 after.
Verification
pnpm run typecheck,pnpm run lint(root and CLI), andpnpm run test:ci-projects(124/124, includes the lint-rule tests) all pass.test/lib test/commands test/types test/script): 12,733 pass, 7 fail. The 7 are sandbox-only filesystem and permission failures in files this PR doesn't touch:completions,upgrade,delta-upgrade,db/resilient,dsn/code-scanner, andinit/tools/file-changes.Stringfails both the lint rule and the coverage test. With an allow-list entry for it, both pass. Keeping the entry after restoring the parser fails the coverage test as stale. A lint comment alone passes lint but still fails the coverage test.JSON.parse, inline arrow and method parsers, positionals in lib helpers,boundArgumentswithoutkind, and allow-list hits and misses.completion-target.tsdoesn't walk the API schema.origin/main(no rebase). It conflicted only on thelist-command.tsimports, and theintrospecttest now expects main's newflagNamesfield.Still text, on purpose
--query,--where), JSON objects (alert issues --condition/--action/--filter,--trigger), crontab schedules, globs, URLs, headers,api --fieldbodies.--dist,--app-id. Local sessions live only in the daemon's memory, so completing them would mean calling the daemon from the fast path.explore --field/--sort, widget--group-by,alert metrics --aggregate) depend on--datasetand the org's attributes, which the CLI doesn't cache.--fieldsflags belong to commands that have no output schema.via @BYK.
--
View Session in Sentry