Repository navigation
ref(cli): Type-check command yields against their output config - #1854
Merged
Merged
Conversation
buildCommand, buildListCommand and buildDeleteCommand now infer the output type from `output` and require every CommandOutput that `func` yields to carry it. `NoInfer` keeps the yields from widening the type, so an untyped `yield new CommandOutput(data)` that doesn't match the formatter is a compile error. Commands without an output config may only yield ClearScreen. Fixes #1852 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
requested changes
Nov 3, 2026
BYK
left a comment
BYK
left a comment
Member
There was a problem hiding this comment.
Not covered: output.schema is still GenericSchema, not tied to T. I tried requiring GenericSchema<unknown, T | item of T> and got 36 errors: list commands whose schema describes the item while T is the ListResult envelope, and generated @sentry/api/valibot schemas whose optionality differs from the @sentry/api types. Tying the schema to T needs its own decision about the envelope and exclude cases.
Well, what's the decision we need to make here or how can we cover this?
The existing yield new CommandOutput(…) annotations aren't needed anymore, but I left them in place.
Why, isn't this just noise?
Without jsonTransform, the yielded data (each item for array data) must
now satisfy output.schema, so a field the schema documents but the
command doesn't produce, or documents with another type, is a compile
error. With jsonTransform the schema describes the transform's item,
which the types can't see, so it stays unchecked.
The check found these mismatches, now fixed:
- alert issues/metrics edit: type the update responses with their
@sentry/api types. The endpoints return `enabled`, not `status`, so
`alert metrics edit` always reported "active"; derive status from
`enabled` in both commands.
- feedback resolve/spam/unresolve: return SentryFeedback from a shared
updateFeedbackStatus helper.
- release create: document `projects` as either full projects or the
dry run's `{ slug }` objects.
- trial list: document the fields the command emits (name, status,
daysRemaining) instead of the raw API trial, which listed reasonCode.
Also drop the CommandOutput<T> yield annotations the builders made
redundant, keeping the ones that stop discriminant literals widening to
string.
Co-Authored-By: BYK <burak.kaya@sentry.io>
BYK
approved these changes
Nov 3, 2026
BYK
enabled auto-merge (squash)
November 3, 2026 16:55
BYK
added a commit
that referenced
this pull request
Nov 3, 2026
Fixes #1860, the follow-up from #1854. #1854 checked `output.schema` against the yielded data, but only for commands without `jsonTransform`. With a transform, the schema describes the transform's item rather than `T`, and every transform returned `unknown`, so there was nothing to check against. The builders now infer the transform's return type. The item is each element of a `{ data: [...] }` envelope or an array, or else the result itself. Every field the schema documents must exist on that item with a matching type. Optionality isn't compared, since `--fields` makes items `Partial`. A transform that returns `unknown` no longer matches a schema. On a mismatch, the error lists only the mismatched fields under `~outputMismatch` (e.g. `{ "~outputMismatch": { b: number } }`), not the whole schema. To make the results typed: - `jsonTransformListResult` returns `ListJson<T>` (`ListEnvelope<T> | Partial<T>[]`). - The hand-built list envelopes (agent-conversation, dashboard, dashboard revisions, events, replay, span, trace) are typed as `ListEnvelope<Item>`. - `log list` returns `ListEnvelope<LogLike> | Partial<LogLike>`. - The single-object views (event, issue, feedback, project, replay) drop their `Record<string, unknown>` annotations and let the spread's type through, using `filterFieldsIfSet`. - `OrgListConfig` takes the schema's type as a third parameter and checks the entity against it, so team, repo, monitor, and integration list are covered. `buildOrgListCommand` itself only sees a generic `TWithOrg` and can't check anything, so it erases the schema type there. - `issue list` and `project list` used `OutputConfig<T>` as an annotation or with `satisfies`. That erased the schema's type, so the check passed without comparing anything. Both are now inferred. The check found these mismatches: - **`span list`**: the schema documented the API's field names (`id`, `span.op`, `span.duration`, `timestamp`, `project`, `trace`). The command emits the names `spanListItemToFlatSpan` gives them (`span_id`, `op`, `duration_ms`, `start_timestamp`, `project_slug`, `trace_id`). As a result, `--help`, `--fields` completion, and the skill reference listed fields that never appear in the output. A new `FlatSpanSchema` documents the emitted fields, and `FlatSpan` is now derived from it. A regression test checks that every key in the JSON output is a documented field. It fails against the old schema. - **`IssueEvent`**: the hand-written type said `metadata` could be `null`. The schema, built from `@sentry/api`, says it can't. `IssueEvent` is now the `GroupEventsResponseDict` item type. - **`ReplayListItem` / `ReplayDetails`**: these typed `project_id` as `string | number`, but `lib/api/replays.ts` normalizes it to a string. `normalizeReplayProjectId` now returns that type. The regenerated `span.md` skill reference is included. `packages/cli/AGENTS.md` describes the check and how to type a transform. Transforms without a schema (`api`, `issue merge`, `trace view`, `span view`, and others) are unchanged. To confirm the check catches drift, I built throwaway commands that each break one way: - a transform's envelope item is missing a field; - an envelope item has a field of the wrong type; - a transform returns `unknown`; - a single-object transform is missing a field; - an array transform is missing a field; - an `OrgListConfig` entity is missing a field. All six failed to compile. The matching versions, and a transform without a schema, compiled. These weren't kept as tests, because `tsc` only covers `src/`. <!-- 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>
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 #1852, the follow-up from #1830.
buildCommandtookoutput?: OutputConfig<any>and typedfuncas yieldingunknown, so nothing tied what a command yields to what itshumanformatter andjsonTransformexpect. #1830's derived types only helped where the yield or the formatter was annotated.buildCommand,buildListCommand, andbuildDeleteCommandnow take anOUTPUTtype parameter, inferred fromoutput.funcmust yieldCommandYield<OUTPUT>(CommandOutput<OUTPUT> | ClearScreen).func's side is wrapped inNoInfer, so a mismatched yield is an error instead of wideningOUTPUT. Commands with no output config getOUTPUT = never, so aCommandOutputyielded there (which the wrapper would silently drop) is now an error too. The list and delete builders reuse the exportedSentryCommandFunctioninstead of their own copies.Since
Tis now real instead ofany,jsonExcludeis checked as well. It neededJsonExcludeKey<T>, which handles the two shapesapplyJsonExcludealready supports: array data (trial liststripsdisplayNamefrom each item) and union data (help,project create).trial start'shandlePlanTrialyieldedunknown. It now yieldsCommandYield<TrialStartOutput>, using the type its formatter already took.output.schemais checked too, when there's nojsonTransformThe builders also infer the schema's type. Without
jsonTransform, the data (each item, for array data) must satisfyInferOutput<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 coversjsonExclude'd human-only fields.With
jsonTransform, the schema describes the transform's item (what--fieldsworks on), notT, and every transform returnsunknown, 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 returnedRecord<string, unknown>, so they couldn't satisfy schemas built from@sentry/api/valibot. They now returnUpdateOrganizationWorkflowResponse/UpdateOrganizationDetectorResponse. That showed both endpoints returnenabled, notstatus. Because of this,alert metrics editalways reportedstatus: "active", even for a disabled detector. Both commands now derivestatusfromenabled. A regression test is added.feedback resolve/spam/unresolve: these yieldedSentryIssueagainstSentryFeedbackSchema. A sharedupdateFeedbackStatusnow returnsSentryFeedback.release create: the schema now documentsprojectsas either full release projects or the--dry-run{ slug }objects.trial list: the schema was the raw APIProductTrialSchema, which documentedreasonCode(never emitted) and missedname,status,daysRemaining.TrialListEntrySchemanow 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 inreact-native/xcode.tsandschema.ts: they build discriminated-union literals (kind/mode), and without the annotation the discriminant widens tostringand the check fails.packages/cli/AGENTS.mdnow 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
jsonExcludekey, a disagreeingjsonTransform, a factory renderer, and a schema field the data lacks), and each one failed to compile. The repo has no type-test harness (tsconly coverssrc/), so these were not added as permanent tests.via @BYK.
--
View Session in Sentry