mirror of
https://github.com/Skyvern-AI/skyvern.git
synced 2026-07-09 16:09:13 +00:00
fix(credentials): edit-test corruption + browser-profile name reuse (partial unique index) (#7157)
Co-authored-by: AronPerez <aperez0295@gmail.com>
This commit is contained in:
parent
e2edbd9aea
commit
e116aaf35c
4 changed files with 81 additions and 5 deletions
|
|
@ -0,0 +1,51 @@
|
|||
"""browser profile name unique partial on deleted_at
|
||||
|
||||
Revision ID: e2d87251fee8
|
||||
Revises: aff1632dc377
|
||||
Create Date: 2026-07-07T15:35:51.042177+00:00
|
||||
|
||||
"""
|
||||
|
||||
from typing import Sequence, Union
|
||||
|
||||
import sqlalchemy as sa
|
||||
|
||||
from alembic import op
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision: str = "e2d87251fee8"
|
||||
down_revision: Union[str, None] = "aff1632dc377"
|
||||
branch_labels: Union[str, Sequence[str], None] = None
|
||||
depends_on: Union[str, Sequence[str], None] = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.drop_index(
|
||||
"uq_browser_profiles_org_name_user",
|
||||
table_name="browser_profiles",
|
||||
postgresql_where=sa.text("is_managed = false"),
|
||||
)
|
||||
op.create_index(
|
||||
"uq_browser_profiles_org_name_user",
|
||||
"browser_profiles",
|
||||
["organization_id", "name"],
|
||||
unique=True,
|
||||
postgresql_where=sa.text("is_managed = false AND deleted_at IS NULL"),
|
||||
sqlite_where=sa.text("is_managed = false AND deleted_at IS NULL"),
|
||||
)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.drop_index(
|
||||
"uq_browser_profiles_org_name_user",
|
||||
table_name="browser_profiles",
|
||||
postgresql_where=sa.text("is_managed = false AND deleted_at IS NULL"),
|
||||
)
|
||||
op.create_index(
|
||||
"uq_browser_profiles_org_name_user",
|
||||
"browser_profiles",
|
||||
["organization_id", "name"],
|
||||
unique=True,
|
||||
postgresql_where=sa.text("is_managed = false"),
|
||||
sqlite_where=sa.text("is_managed = false"),
|
||||
)
|
||||
|
|
@ -999,8 +999,13 @@ function CredentialsModal({
|
|||
}
|
||||
setAuthenticatorSaveError(null);
|
||||
|
||||
// If test passed, rename the temp credential instead of creating a new one
|
||||
if (testAndSave && testStatus === "completed" && testCredentialId) {
|
||||
const hasCompletedInlineTest =
|
||||
testAndSave && testStatus === "completed" && !!testCredentialId;
|
||||
|
||||
// Create mode: the temp credential created by the inline test already holds
|
||||
// the entered secret and the saved browser profile, so rename it in place
|
||||
// instead of creating a duplicate.
|
||||
if (!isEditMode && hasCompletedInlineTest && testCredentialId) {
|
||||
const url = testUrl.trim();
|
||||
const ctx = userContext.trim();
|
||||
renameCredentialMutation.mutate({
|
||||
|
|
@ -1014,6 +1019,20 @@ function CredentialsModal({
|
|||
return;
|
||||
}
|
||||
|
||||
// Edit mode: the inline test's profile is tied to the throwaway temp
|
||||
// credential, so we must overwrite the real credential's secret (below) and
|
||||
// re-run the test against it to attach a fresh profile. Delete the orphaned
|
||||
// temp credential — onSuccess resets the refs before the dialog close
|
||||
// handler would otherwise clean it up.
|
||||
if (isEditMode && hasCompletedInlineTest && testCredentialId) {
|
||||
const tempCredentialId = testCredentialId;
|
||||
getClient(credentialGetter)
|
||||
.then((client) => client.delete(`/credentials/${tempCredentialId}`))
|
||||
.catch(() => {
|
||||
// Best-effort cleanup
|
||||
});
|
||||
}
|
||||
|
||||
// Capture intent before mutation — state will be reset in onSuccess
|
||||
// In edit mode, only trigger a background test if the user actually changed
|
||||
// credentials, user_context, or URL — not just because the checkbox was pre-checked.
|
||||
|
|
@ -1025,7 +1044,8 @@ function CredentialsModal({
|
|||
saveIntentRef.current = {
|
||||
shouldTestAfterSave:
|
||||
testAndSave &&
|
||||
testStatus !== "completed" &&
|
||||
(testStatus !== "completed" ||
|
||||
(isEditMode && hasCompletedInlineTest)) &&
|
||||
testUrl.trim() !== "" &&
|
||||
hasEditModeChanges,
|
||||
saveBrowserSessionIntent: testAndSave,
|
||||
|
|
|
|||
|
|
@ -75,6 +75,11 @@ function useDeleteCredentialFolderMutation() {
|
|||
onSuccess: () => {
|
||||
queryClient.invalidateQueries({ queryKey: ["credential-folders"] });
|
||||
queryClient.invalidateQueries({ queryKey: ["credentials"] });
|
||||
toast({
|
||||
variant: "success",
|
||||
title: "Folder deleted",
|
||||
description: "The folder has been deleted.",
|
||||
});
|
||||
},
|
||||
onError: (error: Error) => {
|
||||
toast({
|
||||
|
|
|
|||
|
|
@ -1243,8 +1243,8 @@ class BrowserProfileModel(Base):
|
|||
"organization_id",
|
||||
"name",
|
||||
unique=True,
|
||||
postgresql_where=text("is_managed = false"),
|
||||
sqlite_where=text("is_managed = false"),
|
||||
postgresql_where=text("is_managed = false AND deleted_at IS NULL"),
|
||||
sqlite_where=text("is_managed = false AND deleted_at IS NULL"),
|
||||
),
|
||||
Index(
|
||||
"uq_browser_profiles_managed_segment",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue