Repository navigation
ref(cli): Derive command output types from their output schemas - #1830
Merged
Merged
Conversation
Fields the CLI adds to a command's JSON output (target context, dryRun and deleted flags, computed values such as url, trace, and dashboard data) were declared twice: once in the valibot OutputConfig.schema that drives --fields completion, --help, and the skill references, and once in the TypeScript type the command yields. Nothing checked that they agreed. Write those fields once as a valibot object and derive the TypeScript type from it (InferOutput, or WithOutputFields to join them onto an @sentry/api type), so the compiler keeps schema and data in sync. Covers alert, release, dashboard, project, org, auth, issue, event, feedback, and replay commands, and the org-scoped list builder, which now documents the orgSlug it adds. Document the pattern in packages/cli/AGENTS.md. Fixes #1824 Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
BYK
approved these changes
Nov 3, 2026
BYK
left a comment
BYK
left a comment
Member
There was a problem hiding this comment.
Looks good. Do we pre-compile these valibot schemas? If not we should.
BYK
marked this pull request as ready for review
November 3, 2026 12:22
Member
Let's file an issue for this.
Let's also file an issue for this. |
This was referenced Nov 3, 2026
Contributor
Author
|
Filed both follow-ups:
|
Member
|
Looks good. Do we pre-compile these valibot schemas? If not we should. |
BYK
added a commit
that referenced
this pull request
Nov 3, 2026
Fixes #1852, the follow-up from #1830. `buildCommand` took `output?: OutputConfig<any>` and typed `func` as yielding `unknown`, so nothing tied what a command yields to what its `human` formatter and `jsonTransform` expect. #1830's derived types only helped where the yield or the formatter was annotated. `buildCommand`, `buildListCommand`, and `buildDeleteCommand` now take an `OUTPUT` type parameter, inferred from `output`. `func` must yield `CommandYield<OUTPUT>` (`CommandOutput<OUTPUT> | ClearScreen`). `func`'s side is wrapped in `NoInfer`, so a mismatched yield is an error instead of widening `OUTPUT`. Commands with no output config get `OUTPUT = never`, so a `CommandOutput` yielded there (which the wrapper would silently drop) is now an error too. The list and delete builders reuse the exported `SentryCommandFunction` instead of their own copies. Since `T` is now real instead of `any`, `jsonExclude` is checked as well. It needed `JsonExcludeKey<T>`, which handles the two shapes `applyJsonExclude` already supports: array data (`trial list` strips `displayName` from each item) and union data (`help`, `project create`). `trial start`'s `handlePlanTrial` yielded `unknown`. It now yields `CommandYield<TrialStartOutput>`, using the type its formatter already took. ### `output.schema` is checked too, when there's no `jsonTransform` The builders also infer the schema's type. Without `jsonTransform`, the data (each item, for array data) must satisfy `InferOutput<typeof schema>`. So a field the schema documents but the command doesn't produce, or documents with another type, is a compile error. Extra fields on the data are fine, which covers `jsonExclude`'d human-only fields. With `jsonTransform`, the schema describes the transform's item (what `--fields` works on), not `T`, and every transform returns `unknown`, so there is nothing to check against. These commands stay unchecked; #1860 tracks covering them. The check found these mismatches: - **`alert issues edit` / `alert metrics edit`**: the API helpers returned `Record<string, unknown>`, so they couldn't satisfy schemas built from `@sentry/api/valibot`. They now return `UpdateOrganizationWorkflowResponse` / `UpdateOrganizationDetectorResponse`. That showed both endpoints return `enabled`, not `status`. Because of this, `alert metrics edit` always reported `status: "active"`, even for a disabled detector. Both commands now derive `status` from `enabled`. A regression test is added. - **`feedback resolve` / `spam` / `unresolve`**: these yielded `SentryIssue` against `SentryFeedbackSchema`. A shared `updateFeedbackStatus` now returns `SentryFeedback`. - **`release create`**: the schema now documents `projects` as either full release projects or the `--dry-run` `{ slug }` objects. - **`trial list`**: the schema was the raw API `ProductTrialSchema`, which documented `reasonCode` (never emitted) and missed `name`, `status`, `daysRemaining`. `TrialListEntrySchema` now describes what the command emits. The generated skill references (`release.md`, `trial.md`) are regenerated. ### Yield annotations The 69 `new CommandOutput<X>(…)` annotations the builders made redundant are removed. Four remain in `react-native/xcode.ts` and `schema.ts`: they build discriminated-union literals (`kind`/`mode`), and without the annotation the discriminant widens to `string` and the check fails. `packages/cli/AGENTS.md` now says when an annotation is still needed. ### Checks I built throwaway commands for each kind of drift (missing field, wrong field type, list and delete builders, a yield with no output config, a bad `jsonExclude` key, a disagreeing `jsonTransform`, a factory renderer, and a schema field the data lacks), and each one failed to compile. The repo has no type-test harness (`tsc` only covers `src/`), so these were not added as permanent tests. <!-- junior-request-attribution:start --> via @BYK. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:local%3Aweb%3Aa0f6384f4f41d70d7d4ccf6d --> -- [View Session in Sentry](https://sentry.sentry.io/explore/conversations/local%3Aweb%3Aa0f6384f4f41d70d7d4ccf6d/?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> Co-authored-by: BYK <burak.kaya@sentry.io>
Contributor
Author
|
Follow-up for precompiling output-schema data is in #1914. |
7 tasks done
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.
Fixes #1824 using option 2 from the issue: write the schema first and derive the types from it.
Fields the CLI adds to a command's JSON output were declared twice: once in
OutputConfig.schema, which feeds--fieldscompletion,--helpand the skill references, and once in the TypeScript type the command yields. Nothing kept the two in sync. These fields are target context,dryRun/deletedflags, and computed values likeurl,trace,periodand widgetdata.Now each command writes them once, as a valibot
objectof output fields. The schema spreads that object'sentries, and the type comes from it, so the compiler catches drift:InferOutput<typeof Fields>, when the output is all CLI fields.WithOutputFields<ApiType, typeof Fields>(new,src/types/output.ts), when the fields sit on or override an@sentry/apiobject. It uses a key-remapping mapped type, notOmit, becauseOmitdrops named keys from alooseObject's index-signature output.API fields still come from the generated
@sentry/api/valibotschemas, as in #1783.What moved
alert/output-schemas.ts. Delete commands now yield their JSON shape directly ({dryRun|deleted, org, [project], id, name}), so theirjsonTransforms are gone.ReleaseUpdateOutputFields, plus per-command fields for the--dry-runpreviews. Formatters take the derived type instead ofRecord<string, unknown>plus a cast.variant, and the TS types are inferred from it. Schema traversal and type display inlib/formatters/output.tstreatvariantlikeunion. Widget delete yields its JSON shape directly, and its transform is gone.buildOrgListCommand,project list):orgListItemSchemaandWithOrg<T>addorgSlugto both schema and type.org,event,trace,attachments,replayIds,activity,relatedIssues, …) are declared once in*OutputFields. Full event payloads stay opaque in the docs withopaque<SentryEvent>().AccountSummarySchemamoved tosrc/types/account.ts.lib/db/accounts.tstakesAccountSummaryfrom it with a type-only import, because the__completefast path loads that module and must not load valibot (test/script/cli-startup.test.ts).packages/cli/AGENTS.md: new "Declaring Command Output" section on how a new command declares its output.Generated docs
The only change is one new
orgSlugrow in each of theteam,repo,integrationandmonitorskill references. Those lists already putorgSlugon every item in their JSON output; the schema just didn't document it.Not covered
buildCommanddoesn't tieOutputConfig<T>to the typefuncyields, so the check depends on typing the formatter's parameter and theCommandOutput<T>yields. WiringTthroughbuildCommandwould be a separate, wider change.SentryFeedbackSchema,IssueEventSchema, the replay list schema,issue tags) are unchanged. Per the issue, API field descriptions wait on upstream@sentry/apisupport.Checks
pnpm run typecheckandpnpm run lint(CLI package) pass after rebasing onmain. Rootpnpm run lintpassed before the rebase.pnpm run test:unit: 13,058 passed, 7 failed. Six failures are permission tests that also fail on the base commit in this sandbox (completions, upgrade, resilient DB, code scanner, init file changes). The seventh was the startup-import regression above, now fixed.via @BYK.
--
View Session in Sentry