Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
71 changes: 49 additions & 22 deletions static/app/views/settings/projectSeer/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,7 @@ describe('ProjectSeer', () => {
it('hides Scan Issues toggle when triage-signals-v0 feature flag is enabled', async () => {
const projectWithFeatureFlag = ProjectFixture({
features: ['triage-signals-v0'],
autofixAutomationTuning: 'medium', // Already enabled, so no auto-enable PUT
});

render(<ProjectSeer />, {
Expand Down Expand Up @@ -583,7 +584,7 @@ describe('ProjectSeer', () => {
const projectWithFlag = ProjectFixture({
features: ['triage-signals-v0'],
seerScannerAutomation: true,
autofixAutomationTuning: 'off',
autofixAutomationTuning: 'medium', // Already enabled, so no auto-enable PUT
});

const {unmount} = render(<ProjectSeer />, {
Expand Down Expand Up @@ -620,30 +621,16 @@ describe('ProjectSeer', () => {
).not.toBeInTheDocument();
});

it('maps values correctly: off=unchecked, others=checked', async () => {
const {unmount} = render(<ProjectSeer />, {
organization,
outletContext: {
project: ProjectFixture({
features: ['triage-signals-v0'],
seerScannerAutomation: true,
autofixAutomationTuning: 'off',
}),
},
});

expect(
await screen.findByRole('checkbox', {name: /Auto-Trigger Fixes/i})
).not.toBeChecked();
unmount();

it('toggle is always checked when triage-signals-v0 flag is enabled', async () => {
// When flag is on, the toggle is always checked regardless of stored value
// because we default to ON for triage signals users
render(<ProjectSeer />, {
organization,
outletContext: {
project: ProjectFixture({
features: ['triage-signals-v0'],
seerScannerAutomation: true,
autofixAutomationTuning: 'high',
autofixAutomationTuning: 'medium',
}),
},
});
Expand All @@ -666,30 +653,70 @@ describe('ProjectSeer', () => {
project: ProjectFixture({
features: ['triage-signals-v0'],
seerScannerAutomation: true,
autofixAutomationTuning: 'off',
autofixAutomationTuning: 'medium', // Start with enabled so no auto-enable
}),
},
});

const toggle = await screen.findByRole('checkbox', {name: /Auto-Trigger Fixes/i});
expect(toggle).toBeChecked();

// Toggle OFF
await userEvent.click(toggle);

await waitFor(() => {
expect(projectPutRequest).toHaveBeenCalledWith(
expect.any(String),
expect.objectContaining({data: {autofixAutomationTuning: 'medium'}})
expect.objectContaining({data: {autofixAutomationTuning: 'off'}})
);
});

// Toggle back ON
await userEvent.click(toggle);

await waitFor(() => {
expect(projectPutRequest).toHaveBeenCalledWith(
expect.any(String),
expect.objectContaining({data: {autofixAutomationTuning: 'off'}})
expect.objectContaining({data: {autofixAutomationTuning: 'medium'}})
);
});
});

it('respects existing off setting for orgs with flag enabled', async () => {
render(<ProjectSeer />, {
organization,
outletContext: {
project: ProjectFixture({
features: ['triage-signals-v0'],
seerScannerAutomation: true,
autofixAutomationTuning: 'off', // Existing org with it disabled
}),
},
});

// Toggle should be unchecked, respecting the existing 'off' setting
expect(
await screen.findByRole('checkbox', {name: /Auto-Trigger Fixes/i})
).not.toBeChecked();
});

it('defaults to ON for new orgs (undefined value)', async () => {
render(<ProjectSeer />, {
organization,
outletContext: {
project: ProjectFixture({
features: ['triage-signals-v0'],
seerScannerAutomation: true,
autofixAutomationTuning: undefined, // New org
}),
},
});

// Toggle should be checked for new orgs
expect(
await screen.findByRole('checkbox', {name: /Auto-Trigger Fixes/i})
).toBeChecked();
});
});

describe('Auto Create PR Setting', () => {
Comment on lines +712 to 722
Copy link

Choose a reason for hiding this comment

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

Bug: Existing autofixAutomationTuning values are silently downgraded to 'medium' when the feature flag is enabled.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

When the triage-signals-v0 feature flag is enabled, existing autofixAutomationTuning values (e.g., 'high', 'low') are converted to a boolean true for UI display. If the user does not interact with the form, the getData function converts this true back to 'medium' upon submission, silently downgrading the original setting without user consent. This is evident from removed tests and new test fixtures explicitly using 'medium' to avoid this scenario.

💡 Suggested Fix

Ensure getData preserves existing autofixAutomationTuning string values when the toggle is displayed as checked, rather than unconditionally converting them to 'medium'.

🤖 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: static/app/views/settings/projectSeer/index.spec.tsx#L653-L722

Potential issue: When the `triage-signals-v0` feature flag is enabled, existing
`autofixAutomationTuning` values (e.g., `'high'`, `'low'`) are converted to a boolean
`true` for UI display. If the user does not interact with the form, the `getData`
function converts this `true` back to `'medium'` upon submission, silently downgrading
the original setting without user consent. This is evident from removed tests and new
test fixtures explicitly using `'medium'` to avoid this scenario.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 3611850

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that's the point of the feature.

Expand Down
6 changes: 5 additions & 1 deletion static/app/views/settings/projectSeer/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,11 @@ function ProjectSeerGeneralForm({project}: {project: Project}) {
},
];

// When triage signals flag is on, toggle defaults to checked unless explicitly 'off'
// - New orgs (undefined): shows checked, persists on form interaction
// - Existing orgs with 'off': shows unchecked, preserves their choice
const automationTuning = isTriageSignalsFeatureOn
? (project.autofixAutomationTuning ?? 'off') !== 'off'
? project.autofixAutomationTuning !== 'off'
Copy link
Member

Choose a reason for hiding this comment

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

Why does this branch return a boolean and the other branch return a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the feature flag is on "Auto-Trigger Fixes" is a toggle and if it's off it's a dropdown. Since it's a toggle when it's on I used bool here and I convert it to a string later. I could make it a string though a toggle being a string might be weird.

: (project.autofixAutomationTuning ?? 'off');

return (
Expand All @@ -346,6 +349,7 @@ function ProjectSeerGeneralForm({project}: {project: Project}) {
initialData={{
seerScannerAutomation: project.seerScannerAutomation ?? false,
// Same DB field, different UI: toggle (boolean) vs dropdown (string)
// When triage signals flag is on, default to true (ON)
autofixAutomationTuning: automationTuning,
automated_run_stopping_point: preference?.automation_handoff
? 'cursor_handoff'
Expand Down
Loading