fix(improve-kandev): lock workflow, block submit during bootstrap, hide None mode (#885)
This commit is contained in:
@@ -14,6 +14,10 @@ import (
|
||||
taskservice "github.com/kandev/kandev/internal/task/service"
|
||||
)
|
||||
|
||||
// remoteResolver resolves a local repo's origin remote into provider/owner/name.
|
||||
// Injectable so tests can avoid touching real git directories.
|
||||
type remoteResolver func(localPath string) (provider, owner, name string)
|
||||
|
||||
// Constants identifying the canonical kandev repository and the workflow
|
||||
// template loaded from apps/backend/config/workflows/improve-kandev.yml.
|
||||
const (
|
||||
@@ -46,17 +50,21 @@ type Handler struct {
|
||||
// gh resolves the authenticated user's login and write access. Defaults
|
||||
// to a gh-CLI shell-out; tests can substitute a fake.
|
||||
gh GitHubInfo
|
||||
// resolveRemote resolves a local repo path's origin remote. Defaults to
|
||||
// service.ResolveGitRemoteProvider; tests can substitute a fake.
|
||||
resolveRemote remoteResolver
|
||||
}
|
||||
|
||||
// NewHandler constructs a Handler. version is embedded into bundle metadata.
|
||||
func NewHandler(taskSvc *taskservice.Service, cloner Cloner, version string, log *logger.Logger) *Handler {
|
||||
return &Handler{
|
||||
taskSvc: taskSvc,
|
||||
cloner: cloner,
|
||||
log: log,
|
||||
version: version,
|
||||
snapshot: func() []buffer.Entry { return buffer.Default().Snapshot() },
|
||||
gh: newDefaultGitHubInfo(),
|
||||
taskSvc: taskSvc,
|
||||
cloner: cloner,
|
||||
log: log,
|
||||
version: version,
|
||||
snapshot: func() []buffer.Entry { return buffer.Default().Snapshot() },
|
||||
gh: newDefaultGitHubInfo(),
|
||||
resolveRemote: taskservice.ResolveGitRemoteProvider,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -118,21 +126,7 @@ func (h *Handler) httpBootstrap(c *gin.Context) {
|
||||
}
|
||||
|
||||
ctx := c.Request.Context()
|
||||
localPath, err := h.cloner.EnsureCloned(ctx, repoCloneURL, repoOwner, repoName)
|
||||
if err != nil {
|
||||
h.log.Error("improve-kandev: clone failed", zap.Error(err))
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to clone kandev repo"})
|
||||
return
|
||||
}
|
||||
|
||||
repo, err := h.taskSvc.FindOrCreateRepository(ctx, &taskservice.FindOrCreateRepositoryRequest{
|
||||
WorkspaceID: req.WorkspaceID,
|
||||
Provider: repoProvider,
|
||||
ProviderOwner: repoOwner,
|
||||
ProviderName: repoName,
|
||||
DefaultBranch: defaultBranch,
|
||||
LocalPath: localPath,
|
||||
})
|
||||
repo, err := h.resolveOrCloneRepo(ctx, req.WorkspaceID)
|
||||
if err != nil {
|
||||
h.log.Error("improve-kandev: repository upsert failed", zap.Error(err))
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to register kandev repository"})
|
||||
@@ -182,6 +176,94 @@ func (h *Handler) httpBootstrap(c *gin.Context) {
|
||||
})
|
||||
}
|
||||
|
||||
// resolveOrCloneRepo returns the workspace's kandev repository, preferring an
|
||||
// existing entry — even one the user added themselves with no provider info —
|
||||
// over cloning a managed copy into ~/.kandev/repos. Order:
|
||||
// 1. Match by provider info (workspace + github + kdlbs + kandev).
|
||||
// 2. Match by scanning workspace repos whose origin remote resolves to
|
||||
// kdlbs/kandev; backfill provider info on the match.
|
||||
// 3. Fall back to cloning into the managed location and registering it.
|
||||
func (h *Handler) resolveOrCloneRepo(ctx context.Context, workspaceID string) (*taskmodels.Repository, error) {
|
||||
if existing, err := h.taskSvc.GetRepositoryByProviderInfo(ctx, workspaceID, repoProvider, repoOwner, repoName); err != nil {
|
||||
return nil, err
|
||||
} else if existing != nil {
|
||||
return existing, nil
|
||||
}
|
||||
|
||||
repos, err := h.taskSvc.ListRepositories(ctx, workspaceID)
|
||||
if err != nil {
|
||||
// Listing failed — log and fall through to clone path; we'd rather
|
||||
// risk a duplicate than fail the bootstrap entirely.
|
||||
h.log.Warn("improve-kandev: list repositories failed; falling back to clone", zap.Error(err))
|
||||
} else if match := findKandevRepoByLocalRemote(repos, h.resolveRemote); match != nil {
|
||||
h.backfillKandevProviderInfo(ctx, match)
|
||||
return match, nil
|
||||
}
|
||||
|
||||
localPath, err := h.cloner.EnsureCloned(ctx, repoCloneURL, repoOwner, repoName)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return h.taskSvc.FindOrCreateRepository(ctx, &taskservice.FindOrCreateRepositoryRequest{
|
||||
WorkspaceID: workspaceID,
|
||||
Provider: repoProvider,
|
||||
ProviderOwner: repoOwner,
|
||||
ProviderName: repoName,
|
||||
DefaultBranch: defaultBranch,
|
||||
LocalPath: localPath,
|
||||
})
|
||||
}
|
||||
|
||||
// findKandevRepoByLocalRemote returns the first repo with a local path whose
|
||||
// origin remote resolves to kdlbs/kandev. Skips rows that already declare
|
||||
// non-matching provider info to avoid hijacking unrelated entries.
|
||||
func findKandevRepoByLocalRemote(repos []*taskmodels.Repository, resolve remoteResolver) *taskmodels.Repository {
|
||||
if resolve == nil {
|
||||
return nil
|
||||
}
|
||||
for _, r := range repos {
|
||||
if r == nil || r.LocalPath == "" {
|
||||
continue
|
||||
}
|
||||
if r.Provider != "" && (r.Provider != repoProvider || r.ProviderOwner != repoOwner || r.ProviderName != repoName) {
|
||||
continue
|
||||
}
|
||||
p, o, n := resolve(r.LocalPath)
|
||||
if p == repoProvider && o == repoOwner && n == repoName {
|
||||
return r
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// backfillKandevProviderInfo fills missing provider/owner/name on an existing
|
||||
// repo so subsequent lookups by provider info hit the fast path. Failures are
|
||||
// logged but non-fatal — we still return the matched repo to the caller.
|
||||
func (h *Handler) backfillKandevProviderInfo(ctx context.Context, repo *taskmodels.Repository) {
|
||||
if repo.Provider == repoProvider && repo.ProviderOwner == repoOwner && repo.ProviderName == repoName {
|
||||
return
|
||||
}
|
||||
provider, owner, name := repoProvider, repoOwner, repoName
|
||||
branch := repo.DefaultBranch
|
||||
if branch == "" {
|
||||
branch = defaultBranch
|
||||
}
|
||||
if _, err := h.taskSvc.UpdateRepository(ctx, repo.ID, &taskservice.UpdateRepositoryRequest{
|
||||
Provider: &provider,
|
||||
ProviderOwner: &owner,
|
||||
ProviderName: &name,
|
||||
DefaultBranch: &branch,
|
||||
}); err != nil {
|
||||
h.log.Warn("improve-kandev: backfill provider info failed",
|
||||
zap.String("repository_id", repo.ID), zap.Error(err))
|
||||
return
|
||||
}
|
||||
repo.Provider = provider
|
||||
repo.ProviderOwner = owner
|
||||
repo.ProviderName = name
|
||||
repo.DefaultBranch = branch
|
||||
}
|
||||
|
||||
// emuBlockedMessage explains the EMU-restriction case to the contributor in
|
||||
// terms they can act on. Surfaced in the dialog when ForkStatusBlockedEMU is
|
||||
// returned.
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
taskmodels "github.com/kandev/kandev/internal/task/models"
|
||||
)
|
||||
|
||||
// fakeGitHubInfo is a configurable GitHubInfo for unit tests. Each method
|
||||
@@ -125,6 +126,86 @@ func TestIsEMULogin(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestFindKandevRepoByLocalRemote(t *testing.T) {
|
||||
resolver := func(path string) (string, string, string) {
|
||||
switch path {
|
||||
case "/home/u/kandev":
|
||||
return "github", "kdlbs", "kandev"
|
||||
case "/home/u/fork":
|
||||
return "github", "alice", "kandev"
|
||||
case "/home/u/other":
|
||||
return "github", "kdlbs", "other"
|
||||
}
|
||||
return "", "", ""
|
||||
}
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
repos []*taskmodels.Repository
|
||||
want string // matched repo ID, or "" for no match
|
||||
}{
|
||||
{name: "empty list", repos: nil, want: ""},
|
||||
{
|
||||
name: "skips entries without local path",
|
||||
repos: []*taskmodels.Repository{
|
||||
{ID: "no-path", LocalPath: ""},
|
||||
},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "matches remote owner/name with no provider info",
|
||||
repos: []*taskmodels.Repository{
|
||||
{ID: "match", LocalPath: "/home/u/kandev"},
|
||||
},
|
||||
want: "match",
|
||||
},
|
||||
{
|
||||
name: "skips fork (different owner)",
|
||||
repos: []*taskmodels.Repository{
|
||||
{ID: "fork", LocalPath: "/home/u/fork"},
|
||||
},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "skips repo already wired to a different provider",
|
||||
repos: []*taskmodels.Repository{
|
||||
{ID: "wired", LocalPath: "/home/u/kandev", Provider: "github", ProviderOwner: "someone", ProviderName: "kandev"},
|
||||
},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "returns first match when multiple candidates",
|
||||
repos: []*taskmodels.Repository{
|
||||
{ID: "first", LocalPath: "/home/u/kandev"},
|
||||
{ID: "second", LocalPath: "/home/u/kandev"},
|
||||
},
|
||||
want: "first",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := findKandevRepoByLocalRemote(tc.repos, resolver)
|
||||
if tc.want == "" {
|
||||
if got != nil {
|
||||
t.Errorf("expected no match, got %q", got.ID)
|
||||
}
|
||||
return
|
||||
}
|
||||
if got == nil || got.ID != tc.want {
|
||||
t.Errorf("got = %v, want id = %q", got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestFindKandevRepoByLocalRemote_NilResolver(t *testing.T) {
|
||||
repos := []*taskmodels.Repository{{ID: "x", LocalPath: "/p"}}
|
||||
if got := findKandevRepoByLocalRemote(repos, nil); got != nil {
|
||||
t.Errorf("nil resolver must return nil, got %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsGHNotFound(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
|
||||
@@ -32,6 +32,16 @@ export type BootstrapState =
|
||||
|
||||
type ImproveKind = "bug" | "feature";
|
||||
|
||||
// Surfaced in the dialog's submit-button tooltip while the bootstrap probe is
|
||||
// in flight, so the disabled state has an explanation the user can act on
|
||||
// instead of the usual missing-field reasons.
|
||||
function bootstrapBlockedReason(bootstrap: BootstrapState): string | null {
|
||||
if (bootstrap.kind === "loading" || bootstrap.kind === "idle") {
|
||||
return "Preparing kandev repository…";
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
const PLACEHOLDERS: Record<ImproveKind, string> = {
|
||||
bug: "E.g. When I open the kanban board, the column header text overlaps the task count badge on screens narrower than 1200px...",
|
||||
feature:
|
||||
@@ -90,6 +100,7 @@ export function CreateModeView(props: CreateModeViewProps) {
|
||||
}}
|
||||
onSuccess={props.onTaskCreated}
|
||||
lockedFields={{ repository: true, branch: true, workflow: true }}
|
||||
submitBlockedReason={bootstrapBlockedReason(bootstrap)}
|
||||
descriptionPlaceholder={PLACEHOLDERS[kind]}
|
||||
aboveDescriptionSlot={<KindTabs kind={kind} onChange={handleKindChange} />}
|
||||
extraFormSlot={
|
||||
|
||||
@@ -166,3 +166,42 @@ describe("computeDisabledReason (default)", () => {
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("computeDisabledReason (submitBlockedReason)", () => {
|
||||
const REASON = "Preparing kandev repository…";
|
||||
|
||||
it("returns the supplied reason for start-task even when nothing is missing", () => {
|
||||
expect(computeDisabledReason(makeProps({ submitBlockedReason: REASON }), KIND_START)).toBe(
|
||||
REASON,
|
||||
);
|
||||
});
|
||||
|
||||
it("returns the supplied reason for default in create mode", () => {
|
||||
expect(computeDisabledReason(makeProps({ submitBlockedReason: REASON }), KIND_DEFAULT)).toBe(
|
||||
REASON,
|
||||
);
|
||||
});
|
||||
|
||||
it("returns the supplied reason for update mode (overrides title check)", () => {
|
||||
expect(
|
||||
computeDisabledReason(
|
||||
makeProps({ submitBlockedReason: REASON, hasTitle: false }),
|
||||
KIND_UPDATE,
|
||||
),
|
||||
).toBe(REASON);
|
||||
});
|
||||
|
||||
it("still suppresses the reason while a submission is in flight", () => {
|
||||
expect(
|
||||
computeDisabledReason(
|
||||
makeProps({ submitBlockedReason: REASON, isCreatingTask: true }),
|
||||
KIND_START,
|
||||
),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("ignores empty/null reason and falls back to normal logic", () => {
|
||||
expect(computeDisabledReason(makeProps({ submitBlockedReason: null }), KIND_START)).toBeNull();
|
||||
expect(computeDisabledReason(makeProps({ submitBlockedReason: "" }), KIND_START)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -231,6 +231,13 @@ export type TaskCreateDialogFooterProps = {
|
||||
onUpdateWithoutAgent: () => void;
|
||||
onCreateWithoutAgent: () => void;
|
||||
onCreateWithPlanMode?: () => void;
|
||||
/**
|
||||
* Externally-supplied reason that the submit buttons are disabled (e.g. an
|
||||
* async bootstrap step from a feature wrapper hasn't finished yet). When set,
|
||||
* every submit variant is disabled and the tooltip shows this string instead
|
||||
* of the usual missing-field reason.
|
||||
*/
|
||||
submitBlockedReason?: string | null;
|
||||
};
|
||||
|
||||
function isMissingWorkflowCtx(
|
||||
@@ -290,6 +297,7 @@ export function computeDisabledReason(
|
||||
kind: ButtonKind,
|
||||
): string | null {
|
||||
if (props.isCreatingTask) return null;
|
||||
if (props.submitBlockedReason) return props.submitBlockedReason;
|
||||
if (kind === "update") return props.hasTitle ? null : REASON_TITLE;
|
||||
if (kind === "default" && props.isSessionMode) return sessionDefaultReason(props);
|
||||
const base = baseReason(props);
|
||||
@@ -308,9 +316,10 @@ function computeFooterState(props: TaskCreateDialogFooterProps) {
|
||||
const showStartTask =
|
||||
(props.isCreateMode && (props.hasDescription || props.isPassthroughProfile)) ||
|
||||
Boolean(props.isEditMode && props.agentProfileId);
|
||||
const altDisabled = computeBaseDisabled(props);
|
||||
const blocked = Boolean(props.submitBlockedReason);
|
||||
const altDisabled = computeBaseDisabled(props) || blocked;
|
||||
const splitDisabled = altDisabled || !props.agentProfileId;
|
||||
const defaultDisabled = props.isSessionMode ? !props.agentProfileId : altDisabled;
|
||||
const defaultDisabled = (props.isSessionMode ? !props.agentProfileId : altDisabled) || blocked;
|
||||
|
||||
const disabledReason = computeDisabledReason(props, resolveButtonKind(props, showStartTask));
|
||||
|
||||
|
||||
@@ -214,6 +214,13 @@ type WorkflowSectionProps = {
|
||||
effectiveWorkflowId: string | null;
|
||||
onWorkflowChange: (value: string) => void;
|
||||
agentProfiles: AgentProfileOption[];
|
||||
/**
|
||||
* When true the picker is hidden entirely. Used by feature wrappers
|
||||
* (Improve Kandev) where the workflow is enforced and the user must not be
|
||||
* able to switch to a different one. The wrapper is responsible for
|
||||
* surfacing the workflow elsewhere (e.g. a steps preview).
|
||||
*/
|
||||
workflowLocked?: boolean;
|
||||
};
|
||||
|
||||
export const WorkflowSection = memo(function WorkflowSection({
|
||||
@@ -224,6 +231,7 @@ export const WorkflowSection = memo(function WorkflowSection({
|
||||
effectiveWorkflowId,
|
||||
onWorkflowChange,
|
||||
agentProfiles,
|
||||
workflowLocked,
|
||||
}: WorkflowSectionProps) {
|
||||
const [lastUsedWorkflowId, setLastUsedWorkflowId] = useState<string | null>(null);
|
||||
|
||||
@@ -240,6 +248,7 @@ export const WorkflowSection = memo(function WorkflowSection({
|
||||
);
|
||||
|
||||
if (!isCreateMode || isTaskStarted) return null;
|
||||
if (workflowLocked) return null;
|
||||
|
||||
if (!effectiveWorkflowId || workflows.length > 1) {
|
||||
return (
|
||||
|
||||
@@ -32,6 +32,7 @@ import {
|
||||
useDialogFormState,
|
||||
useTaskCreateDialogEffects,
|
||||
useDialogHandlers,
|
||||
useLockedFieldSync,
|
||||
useSessionRepoName,
|
||||
useTaskCreateDialogData,
|
||||
computeIsTaskStarted,
|
||||
@@ -89,6 +90,12 @@ interface TaskCreateDialogProps {
|
||||
extraFormSlot?: React.ReactNode;
|
||||
/** Optional render slot at the bottom of the dialog footer area. */
|
||||
bottomSlot?: React.ReactNode;
|
||||
/**
|
||||
* When set, every submit button is disabled and the tooltip surfaces this
|
||||
* exact reason (e.g. an async bootstrap step from a feature wrapper hasn't
|
||||
* completed yet). Takes precedence over the usual missing-field reasons.
|
||||
*/
|
||||
submitBlockedReason?: string | null;
|
||||
}
|
||||
|
||||
type DialogFormBodyProps = {
|
||||
@@ -126,7 +133,7 @@ type DialogFormBodyProps = {
|
||||
onToggleGitHubUrl?: () => void;
|
||||
onGitHubUrlChange: (v: string) => void;
|
||||
onToggleFreshBranch: (enabled: boolean) => void;
|
||||
onToggleNoRepository: () => void;
|
||||
onToggleNoRepository?: () => void;
|
||||
onWorkspacePathChange: (value: string) => void;
|
||||
enhance?: { onEnhance: () => void; isLoading: boolean; isConfigured: boolean };
|
||||
workflowAgentLocked: boolean;
|
||||
@@ -148,6 +155,8 @@ type DialogFormBodyProps = {
|
||||
bottomSlot?: React.ReactNode;
|
||||
/** Optional override for the description placeholder. */
|
||||
descriptionPlaceholder?: string;
|
||||
/** When true, hides the workflow picker so the enforced workflow can't be swapped. */
|
||||
workflowLocked?: boolean;
|
||||
};
|
||||
|
||||
function computeHasAllBranches(fs: DialogFormState): boolean {
|
||||
@@ -291,6 +300,7 @@ function DialogFormBody(props: DialogFormBodyProps) {
|
||||
effectiveWorkflowId={props.effectiveWorkflowId}
|
||||
onWorkflowChange={props.onWorkflowChange}
|
||||
agentProfiles={props.agentProfiles}
|
||||
workflowLocked={props.workflowLocked}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
@@ -461,6 +471,7 @@ function useTaskCreateDialogSetup(props: TaskCreateDialogProps) {
|
||||
workflows,
|
||||
isLocalExecutor: computed.isLocalExecutor,
|
||||
});
|
||||
useLockedFieldSync(open, workflowId, initialValues, fs);
|
||||
const handlers = useDialogHandlers(fs, repositories);
|
||||
const submitHandlers = useSubmitHandlersWiring({
|
||||
props,
|
||||
@@ -471,8 +482,12 @@ function useTaskCreateDialogSetup(props: TaskCreateDialogProps) {
|
||||
isSessionMode,
|
||||
isEditMode,
|
||||
});
|
||||
const guardedHandleSubmit = useGuardedSubmit(
|
||||
submitHandlers.handleSubmit,
|
||||
props.submitBlockedReason,
|
||||
);
|
||||
const handleKeyDown = useKeyboardShortcutHandler(SHORTCUTS.SUBMIT, (event) => {
|
||||
submitHandlers.handleSubmit(event as unknown as FormEvent);
|
||||
guardedHandleSubmit(event as unknown as FormEvent);
|
||||
});
|
||||
// Fresh-branch is single-row + local executor + not URL mode. The chip row
|
||||
// can hold any number of repos; we hide the toggle whenever the question
|
||||
@@ -496,21 +511,40 @@ function useTaskCreateDialogSetup(props: TaskCreateDialogProps) {
|
||||
submitHandlers,
|
||||
handleKeyDown,
|
||||
freshBranchAvailable,
|
||||
guardedHandleSubmit,
|
||||
enhance: useEnhanceForDialog(fs),
|
||||
handleJiraImport: useJiraImportHandler(fs),
|
||||
handleLinearImport: useLinearImportHandler(fs),
|
||||
};
|
||||
}
|
||||
|
||||
// Buttons are disabled when submitBlockedReason is set, but the form can still
|
||||
// be submitted via Enter; gate the submit path here so a wrapper's async
|
||||
// bootstrap step always finishes before any task is created.
|
||||
function useGuardedSubmit(
|
||||
handleSubmit: (e: FormEvent) => void,
|
||||
blockedReason: string | null | undefined,
|
||||
) {
|
||||
const blocked = Boolean(blockedReason);
|
||||
return useCallback(
|
||||
(e: FormEvent) => {
|
||||
if (blocked) e.preventDefault();
|
||||
else handleSubmit(e);
|
||||
},
|
||||
[blocked, handleSubmit],
|
||||
);
|
||||
}
|
||||
|
||||
export function TaskCreateDialog(props: TaskCreateDialogProps) {
|
||||
const { open, onOpenChange, initialValues, workspaceId } = props;
|
||||
const setup = useTaskCreateDialogSetup(props);
|
||||
const { fs, isSessionMode, isEditMode, isCreateMode, isTaskStarted } = setup;
|
||||
const { sessionRepoName, workflows, agentProfiles, snapshots, repositories } = setup;
|
||||
const { computed, handlers, handleKeyDown, freshBranchAvailable } = setup;
|
||||
const { handleSubmit, handleUpdateWithoutAgent, handleCreateWithoutAgent } = setup.submitHandlers;
|
||||
const repoLocked = !!props.lockedFields?.repository;
|
||||
const { handleUpdateWithoutAgent, handleCreateWithoutAgent } = setup.submitHandlers;
|
||||
const { handleCreateWithPlanMode, handleCancel } = setup.submitHandlers;
|
||||
const { handleJiraImport, handleLinearImport } = setup;
|
||||
const { handleJiraImport, handleLinearImport, guardedHandleSubmit } = setup;
|
||||
return (
|
||||
<Dialog open={open} onOpenChange={onOpenChange}>
|
||||
<DialogContent
|
||||
@@ -525,7 +559,7 @@ export function TaskCreateDialog(props: TaskCreateDialogProps) {
|
||||
initialTitle={initialValues?.title}
|
||||
/>
|
||||
</DialogHeader>
|
||||
<form onSubmit={handleSubmit} className="flex flex-col gap-4 overflow-hidden">
|
||||
<form onSubmit={guardedHandleSubmit} className="flex flex-col gap-4 overflow-hidden">
|
||||
<DialogFormBody
|
||||
isSessionMode={isSessionMode}
|
||||
isCreateMode={isCreateMode}
|
||||
@@ -554,12 +588,10 @@ export function TaskCreateDialog(props: TaskCreateDialogProps) {
|
||||
onAgentProfileChange={handlers.handleAgentProfileChange}
|
||||
onExecutorProfileChange={handlers.handleExecutorProfileChange}
|
||||
onWorkflowChange={handlers.handleWorkflowChange}
|
||||
onToggleGitHubUrl={
|
||||
props.lockedFields?.repository ? undefined : handlers.handleToggleGitHubUrl
|
||||
}
|
||||
onToggleGitHubUrl={repoLocked ? undefined : handlers.handleToggleGitHubUrl}
|
||||
onGitHubUrlChange={handlers.handleGitHubUrlChange}
|
||||
onToggleFreshBranch={handlers.handleToggleFreshBranch}
|
||||
onToggleNoRepository={handlers.handleToggleNoRepository}
|
||||
onToggleNoRepository={repoLocked ? undefined : handlers.handleToggleNoRepository}
|
||||
onWorkspacePathChange={handlers.handleWorkspacePathChange}
|
||||
enhance={setup.enhance}
|
||||
workflowAgentLocked={computed.workflowAgentLocked}
|
||||
@@ -570,6 +602,7 @@ export function TaskCreateDialog(props: TaskCreateDialogProps) {
|
||||
aboveDescriptionSlot={props.aboveDescriptionSlot}
|
||||
bottomSlot={props.bottomSlot}
|
||||
descriptionPlaceholder={props.descriptionPlaceholder}
|
||||
workflowLocked={props.lockedFields?.workflow}
|
||||
/>
|
||||
<DialogFooter className="border-t border-border pt-3 flex-col gap-3 sm:flex-row sm:gap-2">
|
||||
<TaskCreateDialogFooter
|
||||
@@ -592,6 +625,7 @@ export function TaskCreateDialog(props: TaskCreateDialogProps) {
|
||||
onUpdateWithoutAgent={handleUpdateWithoutAgent}
|
||||
onCreateWithoutAgent={handleCreateWithoutAgent}
|
||||
onCreateWithPlanMode={handleCreateWithPlanMode}
|
||||
submitBlockedReason={props.submitBlockedReason}
|
||||
/>
|
||||
</DialogFooter>
|
||||
</form>
|
||||
|
||||
@@ -117,7 +117,12 @@ export const WorkflowSelectorRow = memo(function WorkflowSelectorRow({
|
||||
return (
|
||||
<Popover open={open} onOpenChange={setOpen}>
|
||||
<PopoverTrigger asChild>
|
||||
<Button type="button" variant="ghost" className="w-auto justify-between cursor-pointer">
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
className="w-auto justify-between cursor-pointer"
|
||||
data-testid="workflow-selector-trigger"
|
||||
>
|
||||
<IconLogicBuffer className="h-3.5 w-3.5 shrink-0 text-muted-foreground" />
|
||||
<span className="truncate">{selectedWorkflow?.name ?? "Select workflow"}</span>
|
||||
<IconChevronDown className="ml-2 h-4 w-4 shrink-0 opacity-50" />
|
||||
|
||||
@@ -19,6 +19,12 @@ type BootstrapOverrides = {
|
||||
has_write_access?: boolean;
|
||||
fork_status?: ForkStatus;
|
||||
fork_message?: string;
|
||||
/**
|
||||
* When provided, the bootstrap route handler awaits this promise before
|
||||
* fulfilling the response. Tests use it to keep the dialog in its
|
||||
* `loading` state long enough to assert the disabled submit UI.
|
||||
*/
|
||||
bootstrapHold?: Promise<void>;
|
||||
};
|
||||
|
||||
async function mockImproveKandevApis(
|
||||
@@ -41,8 +47,11 @@ async function mockImproveKandevApis(
|
||||
}),
|
||||
);
|
||||
|
||||
await page.route(BOOTSTRAP_URL, (route) =>
|
||||
route.fulfill({
|
||||
await page.route(BOOTSTRAP_URL, async (route) => {
|
||||
if (overrides.bootstrapHold) {
|
||||
await overrides.bootstrapHold;
|
||||
}
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({
|
||||
@@ -60,8 +69,8 @@ async function mockImproveKandevApis(
|
||||
fork_status: forkStatus,
|
||||
...(overrides.fork_message ? { fork_message: overrides.fork_message } : {}),
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
await page.route(FRONTEND_LOG_URL, (route) =>
|
||||
route.fulfill({
|
||||
@@ -186,4 +195,98 @@ test.describe("Improve Kandev dialog", () => {
|
||||
await testPage.getByTestId("improve-kandev-blocked-close").click();
|
||||
await expect(blockedDialog).toBeHidden();
|
||||
});
|
||||
|
||||
test("locked mode hides workflow picker and source-mode switch (URL / None)", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
// Add a second workflow to the seeded workspace so the create dialog
|
||||
// would normally render the workflow selector (workflows.length > 1).
|
||||
// The locked-fields path in WorkflowSection must still suppress it.
|
||||
await apiClient.createWorkflow(seedData.workspaceId, "Extra Workflow", "simple");
|
||||
|
||||
await mockImproveKandevApis(testPage, seedData);
|
||||
|
||||
await testPage.goto("/");
|
||||
await testPage.getByTestId("improve-kandev-button").first().click();
|
||||
|
||||
const contribute = testPage.getByTestId("improve-kandev-proceed");
|
||||
await expect(contribute).toBeEnabled({ timeout: 10_000 });
|
||||
await contribute.click();
|
||||
|
||||
const createDialog = testPage.getByTestId("create-task-dialog");
|
||||
await expect(createDialog).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
// The workflow selector trigger must not render — it would let the user
|
||||
// switch away from the kandev contribution workflow that the bootstrap
|
||||
// probe already locked in.
|
||||
await expect(createDialog.getByTestId("workflow-selector-trigger")).toHaveCount(0);
|
||||
|
||||
// Source-mode switch must be hidden entirely when the repo is locked:
|
||||
// neither the URL nor the None ("scratch") modes can be reached, so the
|
||||
// dialog can only ever submit against the bootstrapped kandev repository.
|
||||
await expect(createDialog.getByTestId("toggle-github-url")).toHaveCount(0);
|
||||
await expect(createDialog.getByTestId("source-mode-scratch")).toHaveCount(0);
|
||||
await expect(createDialog.getByTestId("source-mode-workspace")).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("submit button stays disabled with bootstrap reason while bootstrap is in flight", async ({
|
||||
testPage,
|
||||
seedData,
|
||||
}) => {
|
||||
let releaseBootstrap: () => void = () => {};
|
||||
const bootstrapHold = new Promise<void>((resolve) => {
|
||||
releaseBootstrap = resolve;
|
||||
});
|
||||
|
||||
await mockImproveKandevApis(testPage, seedData, { bootstrapHold });
|
||||
|
||||
await testPage.goto("/");
|
||||
await testPage.getByTestId("improve-kandev-button").first().click();
|
||||
|
||||
const contribute = testPage.getByTestId("improve-kandev-proceed");
|
||||
await expect(contribute).toBeEnabled({ timeout: 10_000 });
|
||||
await contribute.click();
|
||||
|
||||
const createDialog = testPage.getByTestId("create-task-dialog");
|
||||
await expect(createDialog).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
// Banner that explains why the form is partially blocked.
|
||||
await expect(
|
||||
createDialog.getByText(/Preparing kandev repository in background/i),
|
||||
).toBeVisible();
|
||||
|
||||
// Fill in title and description so every other submit-blocking reason is
|
||||
// satisfied — the only remaining blocker should be the pending bootstrap.
|
||||
await createDialog.getByTestId("task-title-input").fill("Fix overlapping header");
|
||||
await createDialog
|
||||
.getByTestId("task-description-input")
|
||||
.fill("Steps to reproduce: open kanban on a narrow viewport.");
|
||||
|
||||
const submit = createDialog.getByTestId("submit-start-agent");
|
||||
await expect(submit).toBeDisabled();
|
||||
|
||||
// Hover the wrapper so the keyboard-shortcut tooltip (which carries the
|
||||
// disabled reason as its description) becomes visible.
|
||||
await createDialog.getByTestId("submit-start-agent-wrapper").hover();
|
||||
await expect(testPage.getByText(/Preparing kandev repository/i).first()).toBeVisible({
|
||||
timeout: 5_000,
|
||||
});
|
||||
|
||||
// Pressing the Cmd/Ctrl+Enter shortcut must also be gated by the bootstrap
|
||||
// guard — if it bypassed guardedHandleSubmit, the dialog would close and
|
||||
// a task would be created with an empty repositoryId.
|
||||
await createDialog.getByTestId("task-description-input").focus();
|
||||
await testPage.keyboard.press("ControlOrMeta+Enter");
|
||||
await expect(createDialog).toBeVisible();
|
||||
await expect(
|
||||
createDialog.getByText(/Preparing kandev repository in background/i),
|
||||
).toBeVisible();
|
||||
|
||||
// Releasing the bootstrap response transitions the dialog to "ready" and
|
||||
// the submit button must become actionable.
|
||||
releaseBootstrap();
|
||||
await expect(submit).toBeEnabled({ timeout: 10_000 });
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user