Skip to content

fix(rendering): honor suppressWarnings in settled build output #470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JoshKappler wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(rendering): honor suppressWarnings in settled build output #470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

- Added `extraArgs` as a first-class session-default value. Repo config or runtime defaults can now carry common `xcodebuild` flags (for example `-skipPackagePluginValidation` or `-disableAutomaticPackageResolution`) so they don't need repeating on every build or test call. Per-call `extraArgs` replace matching configured flags or build settings and append after non-matching defaults, while an explicit empty array (`extraArgs: []`) clears the defaults for a single call. The session management tools show, set, sync, and clear `extraArgs` alongside the other defaults.

### Fixed

- Fixed `suppressWarnings` being ignored in settled build, build-run, and test output. The flag was honored only while streaming, so warnings still reached the final MCP tool response ([#447](https://github.com/getsentry/XcodeBuildMCP/issues/447)).

## [2.6.2]

### Fixed
Expand Down
17 changes: 17 additions & 0 deletions src/utils/renderers/__tests__/cli-text-renderer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,23 @@ describe('cli-text-renderer', () => {
);
});

it('omits settled build warnings when suppressWarnings is set', () => {
const structuredOutput = buildOutput({
diagnostics: {
warnings: [{ message: 'unused variable "foo"', location: 'App.swift:12' }],
errors: [],
},
});

const shown = renderCliTextTranscript({ structuredOutput });
expect(shown).toContain('Warnings (1):');
expect(shown).toContain('unused variable');

const suppressed = renderCliTextTranscript({ structuredOutput, suppressWarnings: true });
expect(suppressed).not.toContain('Warnings (1):');
expect(suppressed).not.toContain('unused variable');
});

it('renders non-interactive test progress durably and deduplicates repeated counts', () => {
const output = renderCliTextTranscript({
items: [
Expand Down
37 changes: 37 additions & 0 deletions src/utils/renderers/__tests__/domain-result-text.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,29 @@
import { describe, expect, it } from 'vitest';
import type {
BuildResultDomainResult,
LaunchResultDomainResult,
UiActionResultDomainResult,
} from '../../../types/domain-results.ts';
import { renderDomainResultTextItems } from '../domain-result-text.ts';

function buildResultWithDiagnostics(): BuildResultDomainResult {
return {
kind: 'build-result',
didError: false,
error: null,
summary: { status: 'SUCCEEDED' },
artifacts: {},

Check failure on line 15 in src/utils/renderers/__tests__/domain-result-text.test.ts

View workflow job for this annotation

GitHub Actions / build-and-test (24.x) 

Type '{}' is not assignable to type 'BuildResultArtifacts'.
diagnostics: {
warnings: [{ message: 'unused variable "foo"', location: 'App.swift:12' }],
errors: [{ message: 'cannot find "bar" in scope', location: 'App.swift:20' }],
},
};
}

function sectionTitles(items: ReturnType<typeof renderDomainResultTextItems>): string[] {
return items.flatMap((item) => (item.type === 'section' ? [item.title] : []));
}

function uiActionResult(action: UiActionResultDomainResult['action']): UiActionResultDomainResult {
return {
kind: 'ui-action-result',
Expand Down Expand Up @@ -114,6 +133,24 @@
`);
});

it('renders build warnings by default', () => {
const titles = sectionTitles(renderDomainResultTextItems(buildResultWithDiagnostics()));

expect(titles).toContain('Warnings (1):');
expect(titles).toContain('Errors (1):');
});

it('omits build warnings when suppressWarnings is set, keeping errors', () => {
const titles = sectionTitles(
renderDomainResultTextItems(buildResultWithDiagnostics(), undefined, {
suppressWarnings: true,
}),
);

expect(titles).not.toContain('Warnings (1):');
expect(titles).toContain('Errors (1):');
});

it('renders batch UI action results', () => {
expect(
renderDomainResultTextItems(
Expand Down
2 changes: 2 additions & 0 deletions src/utils/renderers/cli-text-renderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ function createCliTextProcessor(options: CliTextProcessorOptions): TranscriptRen
const structuredItems = renderDomainResultTextItems(
structuredOutput.result,
structuredOutput.renderHints,
{ suppressWarnings },
);
const replayItems =
sawIncomingHeaderEvent && structuredItems[0]?.type === 'header'
Expand All @@ -456,6 +457,7 @@ function createCliTextProcessor(options: CliTextProcessorOptions): TranscriptRen
const structuredItems = renderDomainResultTextItems(
structuredOutput.result,
structuredOutput.renderHints,
{ suppressWarnings },
);
const replayItems = structuredItems.filter((item) => {
if (sawIncomingHeaderEvent && item.type === 'header') return false;
Expand Down
26 changes: 20 additions & 6 deletions src/utils/renderers/domain-result-text.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ export type TextRendererBlock =

export type TextRenderableItem = RenderItem | TextRendererBlock;

export interface DomainResultTextOptions {
suppressWarnings?: boolean;
}

const SESSION_DEFAULT_KEYS = [
'projectPath',
'workspacePath',
Expand Down Expand Up @@ -248,6 +252,7 @@ function createMarkedDiagnosticLines(

function createStandardDiagnosticSections(
diagnostics: BasicDiagnostics | TestDiagnostics | undefined,
suppressWarnings = false,
): SectionTextBlock[] {
const sections: SectionTextBlock[] = [];
if (!diagnostics) {
Expand All @@ -265,7 +270,7 @@ function createStandardDiagnosticSections(
),
);
}
if (diagnostics.warnings.length > 0) {
if (!suppressWarnings && diagnostics.warnings.length > 0) {
sections.push(
createSection(
`Warnings (${diagnostics.warnings.length}):`,
Comment on lines 270 to 276

@sentry sentry Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The suppressWarnings flag is not passed to createSpecialCaseItems, causing warnings to be displayed for install-result, launch-result, and stop-result failures even when suppression is requested.
Severity: MEDIUM

Suggested Fix

Extract the suppressWarnings option from the options object at the beginning of the renderDomainResultTextItems function, before createSpecialCaseItems is called. Then, pass the suppressWarnings flag through the call chain to createSpecialCaseItems and subsequently to createFailureStatusWithDiagnostics to ensure it is respected.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/utils/renderers/domain-result-text.ts#L270-L276

Potential issue: The `suppressWarnings` option is not honored for certain failure types,
such as `install-result`, `launch-result`, and `stop-result`. This occurs because the
`renderDomainResultTextItems` function calls `createSpecialCaseItems` to handle these
results before it extracts the `suppressWarnings` flag from its options. Consequently,
`createSpecialCaseItems` calls `createFailureStatusWithDiagnostics` without the flag,
leading to `createStandardDiagnosticSections` rendering warnings even when
`suppressWarnings: true` is specified. This behavior contradicts the intended
functionality of suppressing all tool and build warnings.

Did we get this right? 👍 / 👎 to inform future reviews.

@JoshKappler JoshKappler Jul 11, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I checked this against the result constructors and I don't think the path is reachable.

Every install-result, launch-result, and stop-result is built with an empty warnings array. buildInstallSuccess, buildLaunchSuccess, and buildStopSuccess hardcode warnings: [], and the failure builders (buildInstallFailure, buildLaunchFailure, buildStopFailure) call createBasicDiagnostics({ errors: [...] }), which normalizes the absent warnings to []. swift_package_stop builds its stop-result the same way.

createStandardDiagnosticSections only emits the Warnings section when diagnostics.warnings.length > 0, so those three kinds never render one whether or not the flag is set. Threading suppressWarnings into createSpecialCaseItems would be dead code today.

The kinds that actually populate warnings are build, build-run, and test, and those are the ones this PR threads the flag into. If you would rather have the flag wired through the special-case path anyway so it cannot drift if those results ever start carrying warnings, say so and I will extend it.

Expand Down Expand Up @@ -2013,8 +2018,11 @@ function createCleanResultItems(
return items;
}

function createDiagnosticSections(result: ToolDomainResult): SectionTextBlock[] {
return createStandardDiagnosticSections(getResultDiagnostics(result));
function createDiagnosticSections(
result: ToolDomainResult,
suppressWarnings: boolean,
): SectionTextBlock[] {
return createStandardDiagnosticSections(getResultDiagnostics(result), suppressWarnings);
}

function createSummaryBlock(result: ToolDomainResult): SummaryTextBlock | null {
Expand Down Expand Up @@ -2072,8 +2080,12 @@ function createTestDiscoveryProgress(

function createBuildLikeDiagnosticItems(
result: Extract<ToolDomainResult, { kind: 'build-result' | 'build-run-result' | 'test-result' }>,
suppressWarnings: boolean,
): TextRenderableItem[] {
return createStandardDiagnosticSections('diagnostics' in result ? result.diagnostics : undefined);
return createStandardDiagnosticSections(
'diagnostics' in result ? result.diagnostics : undefined,
suppressWarnings,
);
}

function createBuildRunSyntheticStepItems(
Expand Down Expand Up @@ -2576,12 +2588,14 @@ export function createNextStepsBlock(
export function renderDomainResultTextItems(
result: ToolDomainResult,
hints?: RenderHints,
options?: DomainResultTextOptions,
): TextRenderableItem[] {
const specialCaseItems = createSpecialCaseItems(result, hints);
if (specialCaseItems) {
return specialCaseItems;
}

const suppressWarnings = options?.suppressWarnings ?? false;
const items: TextRenderableItem[] = [];
if (
result.kind === 'build-result' ||
Expand All @@ -2599,7 +2613,7 @@ export function renderDomainResultTextItems(
}
}
const tailItems = createBuildLikeTailItems(result);
items.push(...createBuildLikeDiagnosticItems(result));
items.push(...createBuildLikeDiagnosticItems(result, suppressWarnings));
if (result.kind === 'build-run-result') {
items.push(...createBuildRunSyntheticStepItems(result));
}
Expand All @@ -2623,7 +2637,7 @@ export function renderDomainResultTextItems(
return items;
}

items.push(...createDiagnosticSections(result));
items.push(...createDiagnosticSections(result, suppressWarnings));
const summary = createSummaryBlock(result);
if (summary) {
items.push(summary);
Expand Down
Loading