qa: Rejected - fixes required

Issues found:
- Critical: Missing onRerunWizard prop in App.tsx

The "Re-run Wizard" button will not appear in Settings because
the callback is not passed to AppSettingsDialog.

See QA_FIX_REQUEST.md for details.

🤖 QA Agent Session 1
This commit is contained in:
AndyMik90
2025-12-15 17:50:58 +01:00
parent 5f989a4774
commit 5e56890f78
3 changed files with 279 additions and 2 deletions
@@ -0,0 +1,81 @@
# QA Fix Request
**Status**: REJECTED
**Date**: 2025-12-15T17:47:00Z
**QA Session**: 1
## Critical Issues to Fix
### 1. Missing `onRerunWizard` prop in App.tsx
**Problem**: The `AppSettingsDialog` component accepts an `onRerunWizard` callback prop, but this prop is not passed when rendering `AppSettingsDialog` in `App.tsx`. This causes the "Re-run Wizard" button to never appear in Settings.
**Location**: `src/renderer/App.tsx` lines 355-365
**Required Fix**:
Add the `onRerunWizard` prop to the `AppSettingsDialog` component:
```tsx
<AppSettingsDialog
open={isSettingsDialogOpen}
onOpenChange={(open) => {
setIsSettingsDialogOpen(open);
if (!open) {
setSettingsInitialSection(undefined);
}
}}
initialSection={settingsInitialSection}
onRerunWizard={() => {
// Reset onboarding state to trigger wizard
useSettingsStore.getState().updateSettings({ onboardingCompleted: false });
setIsSettingsDialogOpen(false);
setIsOnboardingWizardOpen(true);
}}
/>
```
**Alternative** (if you want to avoid directly calling store methods in JSX):
1. Create a handler function:
```tsx
const handleRerunWizard = useCallback(async () => {
// Reset onboarding state
await updateSettings({ onboardingCompleted: false });
// Close settings dialog
setIsSettingsDialogOpen(false);
// Open onboarding wizard
setIsOnboardingWizardOpen(true);
}, [updateSettings]);
```
2. Extract `updateSettings` from the store:
```tsx
const { updateSettings } = useSettingsStore();
```
3. Pass to the component:
```tsx
<AppSettingsDialog
...
onRerunWizard={handleRerunWizard}
/>
```
**Verification**: After implementing this fix:
1. Build the app without TypeScript errors
2. Launch the app
3. Open Settings (gear icon)
4. Verify "Re-run Wizard" button appears in the Application section (below Notifications)
5. Click the button
6. Verify Settings dialog closes
7. Verify Onboarding Wizard opens from step 1 (Welcome)
8. Complete or skip the wizard
9. Verify `onboardingCompleted` is set back to `true` when finished
## After Fixes
Once fixes are complete:
1. Commit with message: `fix: Add onRerunWizard prop to AppSettingsDialog (qa-requested)`
2. QA will automatically re-run
3. Loop continues until approved
@@ -512,6 +512,25 @@
"checks": []
}
},
"qa_signoff": null,
"last_updated": "2025-12-15T16:44:58.532145+00:00"
"qa_signoff": {
"status": "rejected",
"timestamp": "2025-12-15T17:47:00Z",
"qa_session": 1,
"report_file": "qa_report.md",
"tests_passed": {
"unit": "129/156 (27 pre-existing failures)",
"integration": "N/A",
"e2e": "N/A"
},
"issues_found": [
{
"type": "critical",
"title": "Missing onRerunWizard prop in App.tsx",
"location": "src/renderer/App.tsx:355-365",
"fix_required": "Add onRerunWizard callback prop to AppSettingsDialog that resets onboardingCompleted to false, closes settings, and opens onboarding wizard"
}
],
"fix_request_file": "QA_FIX_REQUEST.md"
},
"last_updated": "2025-12-15T17:47:00Z"
}
@@ -0,0 +1,177 @@
# QA Validation Report
**Spec**: 011-interactive-onboarding-wizard
**Date**: 2025-12-15T17:47:00Z
**QA Agent Session**: 1
## Summary
| Category | Status | Details |
|----------|--------|---------|
| Subtasks Complete | ✓ | 15/15 completed |
| Unit Tests | ✓ | 129/156 passing (27 failures are pre-existing, unrelated to onboarding) |
| Integration Tests | N/A | No integration test commands specified |
| E2E Tests | N/A | Not required per qa_acceptance |
| Browser Verification | ⚠️ | Manual verification needed - see critical issue |
| Database Verification | N/A | Not applicable |
| Third-Party API Validation | ✓ | Zustand usage follows documented patterns |
| Security Review | ✓ | No security vulnerabilities found |
| Pattern Compliance | ✓ | Code follows established patterns |
| Regression Check | ✓ | No new test failures introduced |
## Issues Found
### Critical (Blocks Sign-off)
1. **Missing `onRerunWizard` prop in App.tsx** - `src/renderer/App.tsx:355-365`
The `AppSettingsDialog` component accepts an optional `onRerunWizard` callback prop (added in `AppSettings.tsx:44`), but this prop is NOT passed when `AppSettingsDialog` is rendered in `App.tsx`.
**Impact**: The "Re-run Wizard" button in Settings is conditionally rendered (`{onRerunWizard && ...}`), so it will **NEVER appear** because the callback is undefined.
**Spec Requirement Violated**: "Wizard can be re-run from settings"
**Code Location**:
```tsx
// App.tsx lines 355-365
<AppSettingsDialog
open={isSettingsDialogOpen}
onOpenChange={(open) => {
setIsSettingsDialogOpen(open);
if (!open) {
setSettingsInitialSection(undefined);
}
}}
initialSection={settingsInitialSection}
// MISSING: onRerunWizard prop
/>
```
### Major (Should Fix)
None identified.
### Minor (Nice to Fix)
None identified.
## Recommended Fixes
### Issue 1: Missing `onRerunWizard` prop
**Problem**: The "Re-run Wizard" button in Settings will not appear because `onRerunWizard` callback is not passed to `AppSettingsDialog`.
**Location**: `src/renderer/App.tsx` lines 355-365
**Fix**: Add the `onRerunWizard` prop to `AppSettingsDialog` that:
1. Resets `onboardingCompleted` to false (so wizard shows)
2. Closes the settings dialog
3. Opens the onboarding wizard
Example fix:
```tsx
<AppSettingsDialog
open={isSettingsDialogOpen}
onOpenChange={(open) => {
setIsSettingsDialogOpen(open);
if (!open) {
setSettingsInitialSection(undefined);
}
}}
initialSection={settingsInitialSection}
onRerunWizard={() => {
// Reset onboarding state to trigger wizard
useSettingsStore.getState().updateSettings({ onboardingCompleted: false });
setIsSettingsDialogOpen(false);
setIsOnboardingWizardOpen(true);
}}
/>
```
**Verification**: After fix:
1. Open Settings
2. Verify "Re-run Wizard" button appears in Application section
3. Click the button
4. Verify wizard opens from step 1
## Verification Details
### TypeScript Compilation
- **Status**: ✓ PASS (for onboarding components)
- **Details**: No TypeScript errors in onboarding-related files
- **Pre-existing errors**:
- `terminal-name-generator.ts(176,58)` - type mismatch
- `Terminal.tsx(114,47)` - missing electronAPI method
- `useVirtualizedTree.test.ts(6,33)` - missing @testing-library/react
- `browser-mock.ts(131,7)` - missing mock properties
### Unit Tests
- **Status**: ✓ PASS (no regressions)
- **Results**: 27 failed | 129 passed (156 total)
- **Important**: ALL 27 failures are pre-existing issues:
- Electron mock missing `getAppPath` method
- Missing `@testing-library/react` dependency
- Flaky integration tests with timing/mocking issues
- **Verification**: No test failures reference onboarding components
### Security Review
- **Status**: ✓ PASS
- **Checks performed**:
- No `eval()` calls
- No `innerHTML` usage
- No `dangerouslySetInnerHTML`
- No hardcoded secrets/tokens
- No `window.location` manipulation
### Pattern Compliance
- **Status**: ✓ PASS
- **Patterns verified**:
- FullScreenDialog usage follows AppSettings.tsx pattern
- Zustand store follows existing settings-store.ts pattern
- OAuth configuration follows EnvConfigModal.tsx pattern
- Component structure follows existing onboarding-like components
### Third-Party API Validation (Context7)
- **Status**: ✓ PASS
- **Libraries checked**:
- **Zustand**: `create` store pattern used correctly
- State updates use proper functional pattern `set((state) => ({ ...state, ...updates }))`
- No deprecated APIs detected
## Files Changed Review
| File | Change | Status |
|------|--------|--------|
| `src/shared/types/settings.ts` | Added `onboardingCompleted?: boolean` | ✓ Correct |
| `src/shared/constants.ts` | Added `onboardingCompleted: false` to defaults | ✓ Correct |
| `src/renderer/stores/settings-store.ts` | Added migration logic | ✓ Correct |
| `src/renderer/App.tsx` | Added first-run detection and wizard | ⚠️ Missing `onRerunWizard` prop |
| `src/renderer/components/settings/AppSettings.tsx` | Added Re-run Wizard button | ✓ Correct |
| `src/renderer/components/onboarding/OnboardingWizard.tsx` | Main wizard component | ✓ Correct |
| `src/renderer/components/onboarding/WelcomeStep.tsx` | Welcome step | ✓ Correct |
| `src/renderer/components/onboarding/OAuthStep.tsx` | OAuth configuration step | ✓ Correct |
| `src/renderer/components/onboarding/GraphitiStep.tsx` | Graphiti configuration step | ✓ Correct |
| `src/renderer/components/onboarding/FirstSpecStep.tsx` | First spec creation step | ✓ Correct |
| `src/renderer/components/onboarding/CompletionStep.tsx` | Completion step | ✓ Correct |
| `src/renderer/components/onboarding/WizardProgress.tsx` | Progress indicator | ✓ Correct |
| `src/renderer/components/onboarding/index.ts` | Barrel export | ✓ Correct |
## Verdict
**SIGN-OFF**: ❌ REJECTED
**Reason**: Critical issue - The "Re-run Wizard" button will not appear in Settings because the `onRerunWizard` callback prop is not passed to `AppSettingsDialog` in `App.tsx`. This directly violates the spec requirement "Wizard can be re-run from settings".
**Next Steps**:
1. Coder Agent should fix the missing `onRerunWizard` prop in `App.tsx`
2. QA will re-verify after fix
3. If fixed correctly, sign-off will be granted
## QA Checklist Status
- [x] All unit tests pass (no regressions)
- [x] All integration tests pass (N/A)
- [x] All E2E tests pass (N/A)
- [ ] Browser verification complete - **BLOCKED by critical issue**
- [x] Database state verified (N/A)
- [x] No regressions in existing functionality
- [x] Code follows established patterns
- [x] No security vulnerabilities introduced