Repository navigation
feat(tools): expose the suspect commit in get_issue_details - #1339
Merged
Merged
Conversation
get_issue_details already fetches the event, so its ID is in hand for the events/committers endpoint. Surface the first commit's id, message, author, and suspectCommitType in the structured payload instead of requiring a separate analyze_issue_with_seer call for a question the API already answers.
Render the suspect commit in Markdown as well as structured responses. Report unexpected commit lookup failures while keeping issue details available, and cover output modes and failure handling with regression tests. Co-Authored-By: GPT-6 <noreply@openai.com>
Reuse the structured-content test helper, consolidate lookup failure cases, and remove redundant absence checks and an unused type. Bind event fixtures to the requested endpoint with a unique ID so event-selection regressions cannot use shared fallback data. Co-Authored-By: GPT-6 <noreply@openai.com>
Replace unreachable transaction success cases with the actual lookup failure in the existing transaction test. Cover the commit author email fallback and nullable message with a realistic response, and clarify that lookup returns the current issue suspect commit. Co-Authored-By: GPT-6 <noreply@openai.com>
BYK
pushed a commit
that referenced
this pull request
Sep 30, 2026
…ags patch (#1340) Global flags before the subcommand (`sentry --verbose issue list`) used to be relocated to the tail of argv by `argv-hoist.ts`, because Stricli only parses flags at the leaf command and treats a global flag in a route position as an unknown subcommand. This teaches Stricli's route scanner about a fixed allow-list of Sentry global flags via the existing `@stricli/core` patch (same mechanism as the `-H` removal), so `buildRouteScanner` recognizes `--verbose`, `--json`, `--org`, `--project`, `--log-level`, `--fields` (and the `-v` alias, plus `=`-inline/value forms) at any route depth and forwards them to the leaf command instead of failing route resolution. The patch also drops Stricli's built-in `-v`=version alias so `-v` stays the CLI's `--verbose` alias at every position; `--version` remains the version flag. `argv-hoist.ts` is deleted. The two transforms Stricli can't do at arbitrary route depth stay as thin app-boundary glue in `argv-glue.ts`: `--version` normalization (Stricli only prints it at argv[0]) and the `--help --json` rewrite to the `help` command (Stricli intercepts `--help` and ignores `--json`). **Scope note:** the last checklist item from the issue — upstreaming the top-level-flags behavior to Stricli — is intentionally left out of this PR; the local patch stays until/unless accepted. ## Testing - `pnpm exec vitest run test/lib/argv-glue.test.ts test/lib/argv-glue.integration.test.ts test/commands/help.test.ts test/commands/bash-hook.test.ts` (77 passed) - `pnpm run check:patches`, `tsc --noEmit`, `biome check` on changed files — all clean - Manual smoke via `run(app, preprocessArgv(argv))`: `issue list --help --json` and `--help --json` emit structured JSON; `cli --version` and `--version` print the version; `--verbose/--org/-v` before or between route segments reach the leaf; `-- passthru` is not consumed as a global flag Closes #1339 <!-- ## Plan Root cause: the argv-hoist preprocessor exists only because Stricli parses flags at the leaf level and rejects global flags placed before the subcommand as unknown route segments. buildRouteScanner already special-cases --help/--helpAll/--version while walking the route tree, so it's the natural hook for a global-flag allow-list. Changes: 1. @stricli/core patch, regenerated via `pnpm patch`, edits both dist/index.js and dist/index.cjs: add SENTRY_TOP_LEVEL_*_FLAGS sets + matchSentryTopLevelFlag(); in scanner.next(), before route resolution, push a recognized global flag to unprocessedInputs (value flags latch the next token as their value). Placed after the --help interception; guarded by !target so leaf/-- handling is unchanged. Also drop the built-in -v=version alias in runApplication. The token set is hardcoded (minified dist can't import GLOBAL_FLAGS); global-flags.ts documents the coupling. 2. check-patches.ts: add requiredMarker (presence) support; assert matchSentryTopLevelFlag present and the stale -v version check gone in both bundles. 3. Delete argv-hoist.ts. New argv-glue.ts keeps isVersionRequest + rewriteHelpJsonRequest and a thin preprocessArgv (no hoisting). Wire cli.ts, rename hoistedArgs -> normalizedArgs, update global-flags.ts docs. 4. Tests: delete the two argv-hoist test files; add argv-glue.test.ts and argv-glue.integration.test.ts (run(app,...), hermetic via bash-hook and `cli defaults`). --> --------- Co-authored-by: jared-outpost[bot] <jared-outpost[bot]@users.noreply.github.com>
mr-danya
pushed a commit
to mr-danya/sentry-mcp
that referenced
this pull request
Oct 6, 2026
…ags patch (getsentry#1340) Global flags before the subcommand (`sentry --verbose issue list`) used to be relocated to the tail of argv by `argv-hoist.ts`, because Stricli only parses flags at the leaf command and treats a global flag in a route position as an unknown subcommand. This teaches Stricli's route scanner about a fixed allow-list of Sentry global flags via the existing `@stricli/core` patch (same mechanism as the `-H` removal), so `buildRouteScanner` recognizes `--verbose`, `--json`, `--org`, `--project`, `--log-level`, `--fields` (and the `-v` alias, plus `=`-inline/value forms) at any route depth and forwards them to the leaf command instead of failing route resolution. The patch also drops Stricli's built-in `-v`=version alias so `-v` stays the CLI's `--verbose` alias at every position; `--version` remains the version flag. `argv-hoist.ts` is deleted. The two transforms Stricli can't do at arbitrary route depth stay as thin app-boundary glue in `argv-glue.ts`: `--version` normalization (Stricli only prints it at argv[0]) and the `--help --json` rewrite to the `help` command (Stricli intercepts `--help` and ignores `--json`). **Scope note:** the last checklist item from the issue — upstreaming the top-level-flags behavior to Stricli — is intentionally left out of this PR; the local patch stays until/unless accepted. ## Testing - `pnpm exec vitest run test/lib/argv-glue.test.ts test/lib/argv-glue.integration.test.ts test/commands/help.test.ts test/commands/bash-hook.test.ts` (77 passed) - `pnpm run check:patches`, `tsc --noEmit`, `biome check` on changed files — all clean - Manual smoke via `run(app, preprocessArgv(argv))`: `issue list --help --json` and `--help --json` emit structured JSON; `cli --version` and `--version` print the version; `--verbose/--org/-v` before or between route segments reach the leaf; `-- passthru` is not consumed as a global flag Closes getsentry#1339 <!-- ## Plan Root cause: the argv-hoist preprocessor exists only because Stricli parses flags at the leaf level and rejects global flags placed before the subcommand as unknown route segments. buildRouteScanner already special-cases --help/--helpAll/--version while walking the route tree, so it's the natural hook for a global-flag allow-list. Changes: 1. @stricli/core patch, regenerated via `pnpm patch`, edits both dist/index.js and dist/index.cjs: add SENTRY_TOP_LEVEL_*_FLAGS sets + matchSentryTopLevelFlag(); in scanner.next(), before route resolution, push a recognized global flag to unprocessedInputs (value flags latch the next token as their value). Placed after the --help interception; guarded by !target so leaf/-- handling is unchanged. Also drop the built-in -v=version alias in runApplication. The token set is hardcoded (minified dist can't import GLOBAL_FLAGS); global-flags.ts documents the coupling. 2. check-patches.ts: add requiredMarker (presence) support; assert matchSentryTopLevelFlag present and the stale -v version check gone in both bundles. 3. Delete argv-hoist.ts. New argv-glue.ts keeps isVersionRequest + rewriteHelpJsonRequest and a thin preprocessArgv (no hoisting). Wire cli.ts, rename hoistedArgs -> normalizedArgs, update global-flags.ts docs. 4. Tests: delete the two argv-hoist test files; add argv-glue.test.ts and argv-glue.integration.test.ts (run(app,...), hermetic via bash-hook and `cli defaults`). --> --------- Co-authored-by: jared-outpost[bot] <jared-outpost[bot]@users.noreply.github.com>
This branch was successfully 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
2 participants
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.
Summary
Fixes #1249 by calling the events/committers endpoint using the event ID already fetched and exposing the suspect commit in both structured and Markdown responses from
get_issue_details.Key Changes
getEventCommittersto the API client, backed byCommitterSchemaandCommittersResponseSchema.Breaking Changes