Skip to content

fix: add reset db on command, expose error on e2e tests #1627

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

Merged
lucaseduoli merged 11 commits into from
May 19, 2026
Merged

fix: add reset db on command, expose error on e2e tests #1627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5d35c79
fix factory reset not deleting data
lucaseduoli May 19, 2026
4dbd153
make onboarding errors pop up on e2e tests
lucaseduoli May 19, 2026
1af64ef
style: ruff format (auto)
autofix-ci[bot] May 19, 2026
765f57f
added check for error when uploading
lucaseduoli May 19, 2026
28c61a2
added check if its on first step to rollback, not only if its complete
lucaseduoli May 19, 2026
dfa64c8
reset correctly
lucaseduoli May 19, 2026
7229142
update timeout for uploading document
lucaseduoli May 19, 2026
0a83f13
remove misclick
lucaseduoli May 19, 2026
ab7c3a8
fixed lint
lucaseduoli May 19, 2026
75cc67b
fix mypy errors
lucaseduoli May 19, 2026
92658d1
style: ruff format (auto)
autofix-ci[bot] May 19, 2026
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
11 changes: 11 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,7 @@ factory-reset: ## Complete reset (stop, remove volumes, clear data, remove image
echo " - Remove all volumes"; \
echo " - Delete langflow-data directory"; \
echo " - Delete config directory"; \
echo " - Delete data directory (database and session configs)"; \
echo " - Delete JWT keys (private_key.pem, public_key.pem)"; \
echo " - Remove OpenRAG images"; \
echo ""; \
Expand All @@ -559,6 +560,16 @@ factory-reset: ## Complete reset (stop, remove volumes, clear data, remove image
rm -rf config; \
echo "$(PURPLE)config removed$(NC)"; \
fi; \
if [ -d "data" ]; then \
echo "Removing data..."; \
rm -rf data; \
echo "$(PURPLE)data removed$(NC)"; \
fi; \
if [ -n "$$OPENRAG_DATA_PATH" ] && [ -d "$$OPENRAG_DATA_PATH" ]; then \
echo "Removing $$OPENRAG_DATA_PATH..."; \
rm -rf "$$OPENRAG_DATA_PATH"; \
echo "$(PURPLE)$$OPENRAG_DATA_PATH removed$(NC)"; \
Comment on lines +568 to +571

@coderabbitai coderabbitai Bot May 19, 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.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Add a hard safety guard before deleting OPENRAG_DATA_PATH.

rm -rf "$$OPENRAG_DATA_PATH" currently trusts a raw env path. A mis-set value (e.g. /, ., or project root) can wipe unrelated data.

🛡️ Suggested guard 
-	if [ -n "$$OPENRAG_DATA_PATH" ] && [ -d "$$OPENRAG_DATA_PATH" ]; then \
-		echo "Removing $$OPENRAG_DATA_PATH..."; \
-		rm -rf "$$OPENRAG_DATA_PATH"; \
-		echo "$(PURPLE)$$OPENRAG_DATA_PATH removed$(NC)"; \
-	fi; \
+	if [ -n "$$OPENRAG_DATA_PATH" ] && [ -d "$$OPENRAG_DATA_PATH" ]; then \
+		RESOLVED_PATH=$$(cd "$$OPENRAG_DATA_PATH" 2>/dev/null && pwd -P); \
+		if [ -z "$$RESOLVED_PATH" ] || [ "$$RESOLVED_PATH" = "/" ] || [ "$$RESOLVED_PATH" = "$$(pwd -P)" ] || [ "$$RESOLVED_PATH" = "$$HOME" ]; then \
+			echo "$(RED)Refusing to delete unsafe OPENRAG_DATA_PATH: $$OPENRAG_DATA_PATH$(NC)"; \
+			exit 1; \
+		fi; \
+		echo "Removing $$RESOLVED_PATH..."; \
+		rm -rf "$$RESOLVED_PATH"; \
+		echo "$(PURPLE)$$RESOLVED_PATH removed$(NC)"; \
+	fi; \
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ -n "$$OPENRAG_DATA_PATH" ] && [ -d "$$OPENRAG_DATA_PATH" ]; then \
echo "Removing $$OPENRAG_DATA_PATH..."; \
rm -rf "$$OPENRAG_DATA_PATH"; \
echo "$(PURPLE)$$OPENRAG_DATA_PATH removed$(NC)"; \
if [ -n "$$OPENRAG_DATA_PATH" ] && [ -d "$$OPENRAG_DATA_PATH" ]; then \
RESOLVED_PATH=$$(cd "$$OPENRAG_DATA_PATH" 2>/dev/null && pwd -P); \
if [ -z "$$RESOLVED_PATH" ] || [ "$$RESOLVED_PATH" = "/" ] || [ "$$RESOLVED_PATH" = "$$(pwd -P)" ] || [ "$$RESOLVED_PATH" = "$$HOME" ]; then \
echo "$(RED)Refusing to delete unsafe OPENRAG_DATA_PATH: $$OPENRAG_DATA_PATH$(NC)"; \
exit 1; \
fi; \
echo "Removing $$RESOLVED_PATH..."; \
rm -rf "$$RESOLVED_PATH"; \
echo "$(PURPLE)$$RESOLVED_PATH removed$(NC)"; \
fi; \
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` around lines 568 - 571, The Makefile removal step trusts
OPENRAG_DATA_PATH and may delete dangerous locations; add a hard safety guard
before the rm -rf call: verify OPENRAG_DATA_PATH is non-empty, is an absolute
path, is not "/" or "." or the project root or current working directory, and
(preferably) resides under an expected base directory (e.g., a dedicated data
dir) before running rm -rf; if any check fails, print an error and skip
deletion. Reference the OPENRAG_DATA_PATH variable and the rm -rf
"$$OPENRAG_DATA_PATH" invocation to locate where to add these pre-checks.

fi; \
if [ -f "keys/private_key.pem" ] || [ -f "keys/public_key.pem" ]; then \
echo "Removing JWT keys..."; \
rm -f keys/private_key.pem keys/public_key.pem 2>/dev/null || \
Expand Down
5 changes: 4 additions & 1 deletion frontend/app/onboarding/_components/onboarding-card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,10 @@ const OnboardingCard = ({
>
<div className="pb-6 flex items-center gap-4">
<X className="w-4 h-4 text-destructive shrink-0" />
<span className="text-mmd text-muted-foreground">
<span
data-testid="onboarding-error"
className="text-mmd text-muted-foreground"
>
{error}
</span>
</div>
Expand Down
57 changes: 57 additions & 0 deletions frontend/app/onboarding/_components/onboarding-upload.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const OnboardingUpload = ({ onComplete }: OnboardingUploadProps) => {
const [uploadedTaskId, setUploadedTaskId] = useState<string | null>(null);
const [shouldCreateFilter, setShouldCreateFilter] = useState(false);
const [isCreatingFilter, setIsCreatingFilter] = useState(false);
const [error, setError] = useState<string | null>(null);

const createFilterMutation = useCreateFilter();
const updateOnboardingMutation = useUpdateOnboardingStateMutation();
Expand Down Expand Up @@ -63,6 +64,41 @@ const OnboardingUpload = ({ onComplete }: OnboardingUploadProps) => {
matchingTask.status === "running" ||
matchingTask.status === "processing";

// Check if matching task failed or has error status
const failedTask =
matchingTask.status === "failed" || matchingTask.status === "error";

// Check if any file inside the task failed
const filesArray = matchingTask.files
? (Object.values(matchingTask.files) as {
status: string;
error?: string;
}[])
: [];
const hasFailedFile = filesArray.some(
(file) => file.status === "failed" || file.status === "error",
);

if (failedTask || hasFailedFile) {
let errorMessage = "Document ingestion failed. Please try again.";
if (matchingTask.error) {
errorMessage = matchingTask.error;
} else {
const failedFile = filesArray.find(
(file) =>
(file.status === "failed" || file.status === "error") && file.error,
);
if (failedFile?.error) {
errorMessage = failedFile.error;
}
}

setError(errorMessage);
setCurrentStep(null);
setUploadedTaskId(null);
return;
Comment on lines +96 to +99

@coderabbitai coderabbitai Bot May 19, 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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clear filter-intent state on failure to avoid retry cross-contamination.

After a failed ingestion/upload, shouldCreateFilter and uploadedFilename remain set. A later retry can accidentally create a filter for stale data.

Suggested fix 
     if (failedTask || hasFailedFile) {
       let errorMessage = "Document ingestion failed. Please try again.";
@@
       setError(errorMessage);
       setCurrentStep(null);
       setUploadedTaskId(null);
+      setShouldCreateFilter(false);
+      setUploadedFilename(null);
       return;
     }
@@
     } catch (error) {
@@
       // Reset on error
       setCurrentStep(null);
       setUploadedTaskId(null);
+      setShouldCreateFilter(false);
+      setUploadedFilename(null);
     } finally {

Also applies to: 251-253

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/app/onboarding/_components/onboarding-upload.tsx` around lines 96 -
99, On upload failure (the block that calls setError, setCurrentStep(null),
setUploadedTaskId(null)), also clear the filter-intent state by resetting
shouldCreateFilter and uploadedFilename to safe empty values (e.g., call
setShouldCreateFilter(false) and setUploadedFilename(null or empty string));
apply the same change in the other failure block around the setError at the
251-253 area so retries can't reuse stale shouldCreateFilter/uploadedFilename
state.

}

// If task is completed or has processed files, complete the onboarding step
if (!isTaskActive || (matchingTask.processed_files ?? 0) > 0) {
// Set to final step to show "Done"
Expand Down Expand Up @@ -169,6 +205,7 @@ const OnboardingUpload = ({ onComplete }: OnboardingUploadProps) => {

const performUpload = async (file: File) => {
setIsUploading(true);
setError(null);
try {
setCurrentStep(0);
const result = await uploadFile(file, true, true); // Pass createFilter=true
Expand All @@ -193,6 +230,7 @@ const OnboardingUpload = ({ onComplete }: OnboardingUploadProps) => {
const errorMessage =
error instanceof Error ? error.message : "Upload failed";
console.error("Upload failed", errorMessage);
setError(errorMessage);

// Dispatch event that chat context can listen to
// This avoids circular dependency issues
Expand Down Expand Up @@ -246,6 +284,25 @@ const OnboardingUpload = ({ onComplete }: OnboardingUploadProps) => {
exit={{ opacity: 0, y: -24 }}
transition={{ duration: 0.4, ease: "easeInOut" }}
>
<AnimatePresence mode="wait">
{error && (
<motion.div
key="error"
initial={{ opacity: 1, y: 0, height: "auto" }}
exit={{ opacity: 0, y: -10, height: 0 }}
>
<div className="pb-6 flex items-center gap-4">
<X className="w-4 h-4 text-destructive shrink-0" />
<span
data-testid="onboarding-upload-error"
className="text-mmd text-muted-foreground"
>
{error}
</span>
</div>
</motion.div>
)}
</AnimatePresence>
<Button
size="sm"
variant="outline"
Expand Down
41 changes: 33 additions & 8 deletions frontend/tests/utils/onboarding.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,19 @@ export async function completeOnboarding(
}

const isCompleted = await completedLocator.isVisible();
const isFirstStep = await page.getByTestId("openai-llm-tab").isVisible();

if (isCompleted) {
if (!reset) {
console.log("Onboarding already complete, skipping...");
return;
}
if (isCompleted && !reset) {
console.log("Onboarding already complete, skipping...");
return;
}

const needsRollback = reset && (isCompleted || !isFirstStep);

console.log("Onboarding complete and reset is true, rolling back...");
if (needsRollback) {
console.log(
"Onboarding complete or not on the first step, and reset is true, rolling back...",
);
const response = await page.request.post("/api/onboarding/rollback");
if (!response.ok()) {
const text = await response.text();
Expand Down Expand Up @@ -141,9 +146,18 @@ export async function completeOnboarding(
await page.getByTestId("onboarding-complete-button").click();

await expect(page.getByText("Thinking")).toBeVisible();
await expect(page.getByText("Done")).toBeVisible({

const doneLocator = page.getByText("Done");
const errorLocator = page.getByTestId("onboarding-error");

await expect(doneLocator.or(errorLocator)).toBeVisible({
timeout: isEmbedding ? 120000 : 60000,
Comment on lines +150 to 154

@coderabbitai coderabbitai Bot May 19, 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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Scope the "Done" locator to avoid false-positive waits.

Using page.getByText("Done") globally can resolve against a previously completed step, so these waits may pass before the current async action actually finishes.

Suggested fix 
-    const doneLocator = page.getByText("Done");
+    const doneLocator = page.getByText(/^Done$/).last();
     const errorLocator = page.getByTestId("onboarding-error");
@@
-  const uploadDoneLocator = page.getByText("Done");
+  const uploadDoneLocator = page.getByText(/^Done$/).last();
   const uploadErrorLocator = page.getByTestId("onboarding-upload-error");

Also applies to: 199-203

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/tests/utils/onboarding.ts` around lines 150 - 154, The global
page.getByText("Done") can match a previous step and cause false-positive waits;
update the waits that create doneLocator and its uses (the doneLocator variable
and the expect call that uses doneLocator.or(errorLocator)) to scope the "Done"
locator to the current step container (e.g., use the specific step locator or a
parent element's locator and call .getByText("Done") on that container) so the
check only resolves for the active step; apply the same change for the other
occurrence around lines 199-203 where page.getByText("Done") is used.

});

if (await errorLocator.isVisible()) {
const errorText = await errorLocator.innerText();
throw new Error(`Onboarding step failed: ${errorText}`);
}
};

// 1. LLM configuration
Expand Down Expand Up @@ -182,6 +196,17 @@ export async function completeOnboarding(
path.join(__dirname, "../assets", "test-document.md"),
);

await expect(page.getByText("Done")).toBeVisible({ timeout: 60000 });
const uploadDoneLocator = page.getByText("Done");
const uploadErrorLocator = page.getByTestId("onboarding-upload-error");

await expect(uploadDoneLocator.or(uploadErrorLocator)).toBeVisible({
timeout: 120000,
});

if (await uploadErrorLocator.isVisible()) {
const errorText = await uploadErrorLocator.innerText();
throw new Error(`Onboarding document upload failed: ${errorText}`);
}

await expect(page.getByTestId("onboarding-content")).toBeHidden();
}
4 changes: 4 additions & 0 deletions src/api/settings/endpoints.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1373,6 +1373,10 @@ async def remove_filter(filter_id: str | None):
current_config.knowledge.embedding_model = ""
current_config.onboarding.openrag_docs_ingested_version = None
current_config.onboarding.openrag_docs_remote_signature = None
current_config.onboarding.assistant_message = None
current_config.onboarding.selected_nudge = None
current_config.onboarding.card_steps = None
current_config.onboarding.upload_steps = None

embedding_only = body.embedding_only if body else False

Expand Down
Loading
Loading