feat(web): drag-and-drop reorder kanban task cards within a column - #3623
Merged
carlosflorencio merged 54 commits intoSep 13, 2026
Conversation
Add the frozen requirement/system-design pair for REQ-TASKS-KANBAN-TASK-REORDERING-001 (persisted, user-controlled task order within a workflow step, used as the real WIP-promotion priority order) and register both in docs/specs/tasks/README.md.
Implements REQ-TASKS-KANBAN-TASK-REORDERING-001's backend contract: Ordering primitives: - models.StepOrderLess implements AC.1's total ordering key (position ASC, priority rank, queued_at ASC falling back to created_at, created_at ASC, id ASC). - Every arrival into a step (creation, manual move, WIP promotion, automatic workflow transition) now gets a server-computed max(position)+1 (or 0 if empty) via assignArrivalPosition, ignoring any caller-supplied position (AC.28). A move naming the task's current step is not an arrival and leaves its position untouched. - workflow_steps gains an order_revision column (both the canonical definition and its runner-projection mirror) for the reorder conflict/apply-order contract. - New repoerrors.ErrStepChanged / ErrInvalidReorder sentinels. Reorder endpoint: - PUT /api/v1/workflow-steps/:id/tasks/reorder: validates band and id list, locks the step, rejects atomically with 409 step_changed on a membership mismatch (or 400 invalid_reorder when an id isn't a current member of the named step at all), otherwise renumbers the whole step densely from 0 with the admitted band first and bumps order_revision. - Publishes task.reordered (not task.moved, not one task.updated per task) with the whole step's both-band view; wired into the WS gateway's task notification subscriptions. Bulk-move order fix (F34, recorded in the task plan): - BulkMoveSelectedTasks re-derives AC.29's submission order server-side from each task's current source step (step ordinal ascending, tie on step id, admitted-then-queued within a step) instead of trusting caller-supplied order. The design's claim that per-call max()+1 alone preserves call order is false once a caller issues the calls concurrently, since there is then no server-observable issue order to preserve. - Corrects MoveTaskRequest's proto/pluginsdk doc comment, which documented the old (now-wrong) "position 0 = top of step" behavior. Existing fixtures that relied on CreateTask honoring an explicit Position to construct tie-break scenarios are updated: step order now ranks position ahead of priority, so a same-position tiebreak on priority is covered directly in models.TestStepOrderLess rather than via caller-supplied positions. Tests at all three layers (repository, service, HTTP handler), all green; golangci-lint changed-file scan clean.
Part of REQ-TASKS-KANBAN-TASK-REORDERING-001's frontend contract: - lib/kanban/task-order.ts: new compareStepOrder/StepOrderTask — the canonical AC.1 total-order comparator (position, priority rank, queuedAt falling back to createdAt, createdAt, id). Replaces compareTasksByCreatedDesc (newest-first, AC.35's compatibility note) at the board's three live display call sites: swimlane-kanban-content (desktop/tablet Kanban), swipeable-columns (mobile Kanban), and swimlane-graph2-content (Pipeline view, extracted into a new sortGraph2Tasks helper — was bare `position` with no tiebreak). swimlane-graph-content.tsx is unreachable (absent from the view registry) and is deliberately left untouched. - lib/kanban/wip-queue.ts: compareWipQueueTasks now delegates to compareStepOrder instead of maintaining its own copy, so AC.36's three-comparator alignment is enforced structurally. Also fixes the queuedAt fallback itself: an absent queuedAt now reads as that task's createdAt instead of sorting last. - lib/api/domains/kanban-api.ts + lib/types/http.ts: adds reorderStepTasks(), the HTTP-only request surface for a reorder (PUT /api/v1/workflow-steps/:id/tasks/reorder) and its response/error types, matching the frozen wire contract. - hooks/use-task-multi-select.ts: the kanban toolbar's bulkMove no longer fans out one moveTaskById call per selected task via Promise.allSettled (F34's frontend half) — a concurrent fan-out has no server-observable issue order, so the resulting position order depended on response timing rather than the selection. It now routes through useTaskWorkflowMove(), the same batch-endpoint path use-sidebar-multi-select.ts and the per-card multi-move dropdown already use, converging what were two divergent bulk-move implementations. New/updated tests cover the AC.1 comparator, the Pipeline view's step-index-then-AC.1 tiebreak, the reorder API client's success/409/400 shapes, and the multi-select hook's single-batched-call behavior. pnpm run typecheck and eslint clean on every touched file; full kanban-scoped vitest run (236 tests) green.
Adds the store/hook layer for REQ-TASKS-KANBAN-TASK-REORDERING-001's drag-to-reorder UI: a pure order-merge algorithm that folds a reordered visible subsequence back into a band's full membership without disturbing filtered-out tasks (.34), a useStepReorder hook that submits to the reorder endpoint with optimistic apply, step_changed reconciliation, and AC.20 failure handling, and the kanbanMulti state (order revision per step, in-flight band tracking) both the hook and the upcoming WS handler need for AC.16/.19/.25/.27's asymmetric revision gate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds the frontend handler for the task.reordered event so a reorder committed from another tab/client updates this board too. Applies the asymmetric revision gate (unsolicited events need a strictly-greater revision; a step with no recorded revision accepts the first order it sees) and skips a band currently being submitted by this client so an in-flight optimistic reorder isn't clobbered by its own echo. Ref: REQ-TASKS-KANBAN-TASK-REORDERING-001 (.16, .19, .25, .27)
Makes each rendered card a dnd-kit drop target (in addition to the existing column-level one) so a drop can resolve to a specific card, not just a step. A new pure classifyDrop() decides whether a drop is a same-band reorder, a same-step no-op, a cross-band reject, or the existing cross-step move, then the reorder case calls useStepReorder().reorderBand() with the band's new visible order. Also adds the AC.7 insertion-point indicator: a thin border on the hovered card's near edge while dragging within the same band, using each card's own useDroppable(isOver) plus the active card's index. Ref: REQ-TASKS-KANBAN-TASK-REORDERING-001 (.5, .6, .7, .9, .10, .11, .13)
Adds AC.12's bespoke Space/Enter pick-up, Arrow move, Escape-cancel keyboard flow for within-band card reordering, sharing the same reorderBand/merge machinery as pointer drag, plus an aria-live announcement of the card's band position on pick-up and each move. Also adds an explicit AC.34 test proving a board-filtered-out task's position survives a reorder, and splits kanban-card-content.tsx and swimlane-kanban-content.tsx into smaller modules to stay under the file/function line-count lint limits after the new prop threading. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
failedToReorderTasks and taskReorderErrorGeneric had only ever shipped in English; adds pt-pt and zh-cn translations and re-derives zh-hk/zh-tw from zh-cn via the standard i18n:zh-hant conversion, which also pulls in the traditional-Chinese catalogs' accumulated drift from their zh-cn source. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Desktop pointer drag-to-top and keyboard reorder both persist across reload, a mobile touch-drag equivalent, and a deterministic bulk-move ordering assertion (task-multi-select.spec.ts) that pins the F34 fix: selecting tasks in reverse order still lands them in target-step positions matching their source order, not click or network order. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A same-step move is not an arrival: MoveTaskWithOptions now ignores the caller-supplied position when the step doesn't change, so the test's expectation was stale.
…sition lock lockWorkflowStepForWrite's Postgres FOR UPDATE select failed CreateTask outright when the target step had no workflow_steps row, since a task's WorkflowStepID was never previously required to resolve to a real row on the bare creation path. A missing row has no concurrent writer to serialize against either, so treat it as nothing to lock instead of an error. Adds Postgres coverage for both the missing-row tolerance and that the lock still serializes concurrent arrivals into a real step.
BulkMoveSelectedTasks held the target step's arrival-position mutex across its whole dispatch loop, but assignArrivalPosition itself acquired that same mutex only after its own caller had already opened a database transaction. With SQLite's single-connection writer pool, an interloping arrival could hold the one connection while blocked on the mutex, while the batch (holding the mutex first) could never get a connection back to finish its own sequence — a lock-ordering deadlock. Move every arrival-position mutex acquisition to before its caller's BeginTx, across every entry point (task creation, admission, WIP promotion, and the bulk-move batch), so acquisition order is always mutex-then-connection.
…n reorder Fixes the 3 blockers and 1 minor finding from Review round 1 (AC.34 merge-band bug, AC.27 in-flight reconciliation, cross-step source-step locking, dead client-computed position) and adds the test-rigor coverage the review called out for AC.9, .21, .23, .25, .30, .31, .32, .33, .37, and .38.
Cross-step drag onto an occupied column now resolves the target step from the whole board, not just the dragged task's own step, so dropping on a foreign card no longer sends a task id as a step id. A plain (non-409) reorder failure now restores only the failed band's own pre-drag positions onto the live snapshot instead of rebuilding the whole snapshot from stale data, so it stops clobbering a sibling band or another step that changed during the in-flight window. BulkMoveTasks (the admin whole-step/workflow migration path) now derives arrival order and holds the arrival-position batch lock the same way BulkMoveSelectedTasks already does. A reorder naming a task that became hidden inside the read-to-commit window now reports the membership-change conflict (409 step_changed) instead of a malformed-request error (400 invalid_reorder), per AC.33/.26. Also closes two test-rigor residuals from the prior review: the same-band concurrency test now asserts the persisted order matches the higher revision, and BulkMoveSelectedTasks gets a mixed-band ordering test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BulkMoveSelectedTasks and BulkMoveTasks locked only the target step up front, then let each per-task MoveTask acquire the source step inside the loop. That fixes the acquisition order at target-then-source, which reverses the design's mandated ascending-step-id order whenever the source id sorts first, deadlocking against an ordinary single move running the opposite direction between the same two steps. Both bulk paths now lock the full distinct step set (target plus every source) in one ascending-ordered call before dispatching.
hasForeignReorderID classified any submitted id whose current workflow_step_id differed from the named step as foreign (400 invalid_reorder), which only correctly covered a hidden task. It also caught a task that left the step via an ordinary concurrent cross-step move, which AC.26 requires to resolve as the membership-change conflict (409 step_changed) instead — the server has no signal to tell "moved away during this race" apart from "was already elsewhere," so both must resolve the same way. Reserve invalid_reorder for ids matching no task row at all. Reproduced via the existing TestPostgresCrossStepMoveLocksSourceStepAgainstConcurrentReorder under -race + Postgres (8/15 failures before, 20/20 clean after) and pinned deterministically with a new unit test.
hasNonexistentReorderID checked task existence against the whole tasks table with no workspace scoping, so a real task id from a workspace the caller cannot see resolved to 409 step_changed instead of 400 invalid_reorder — a cross-tenant existence oracle under per-user auth, violating the "no existence leak" invariant authorizeTaskID/ authorizeWorkflowID are built around. A task in a different workspace also could never have raced into or out of this step's band, so scoping to the step's own workspace is the more correct general-case semantics, not just a security fix. Surfaced a latent seed-data gap in TestPostgresCrossStepMoveLocksSourceStepAgainstConcurrentReorder, which referenced a workflow id via raw SQL without ever creating that workflow's row; added the missing CreateWorkflow call to match every other reorder test's fixture pattern.
…icator A queued-task promotion locked only the destination step, not the feeder it leaves - a concurrent reorder of the feeder could read stale membership then, once paused behind nothing, clobber the promoted task's freshly assigned position with a stale feeder-scoped one. Mirrors the cross-step move's existing target+source locking. Proven with a deterministic Postgres race test using a new reorderPreWriteHook synchronization seam (natural goroutine racing almost never reproduces the bad interleaving). Also: the keyboard reorder insertion indicator compared draft-order indices against unmoved DOM indices, flagging the wrong row after more than one arrow press; picking up a card in a <2-member band could still issue a no-op reorder request on commit (AC.32); adds missing AC.22 coverage (reorder bypasses the active-session move restriction) and strengthens the AC.36 queued_at-tiebreak tests, which previously passed regardless of whether the created_at fallback was applied correctly; removes a dead position field from a move request the server already ignores. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…band Two Review-round-5 blockers on kanban reordering: - ArchiveTask/ArchiveTaskIfActive/DeleteTask took no step-scoped lock, so a concurrent ReorderStepTasks of the same step could straddle them: a task hidden or deleted mid-flight still got its position silently rewritten by the reorder's blind, unchecked renumbering write. They now take the same step-row lock ReorderStepTasks and PromoteQueuedTaskIfWorkflowStepHasCapacity already take before mutating step membership/position. - partitionWipTasks' admitted band was never sorted by step order (only the queued band was), so classifyDrop and the keyboard reorder handler computed fromIndex/toIndex against creation order instead of true position order once a step's positions diverged from creation order (i.e. after any prior reorder) — dragging a card could land it at the opposite end from the drop gesture. Backend fix verified with a deterministic Postgres race test (reorderPreWriteHook seam, mirrors task_promotion_lock_postgres_test.go) proving archive/delete now block behind a paused reorder instead of committing mid-flight. Frontend fix verified with unit coverage on the out-of-order-array case plus a second e2e drag in the same test (a single drag can't expose the bug, since a fresh column's creation order and step order coincide). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dnd-kit's default droppable measurement strips CSS transform before caching a droppable's rect (it exists to ignore dnd-kit's own drag-time sibling-shuffle animation). The virtualized column used transform: translateY() as its primary row-positioning mechanism, so every row's measured rect collapsed onto the same pre-transform position — reproducing only on a second same-session drag, when rows have actually moved from their initial slots. Position rows with top instead, which dnd-kit measures correctly.
lockTaskStepForWrite resolved which step to lock via an unlocked read, so a task moving between that read and the lock acquisition caused it to lock the stale step, defeating its own guarantee against a concurrent reorder. It now locks the candidate step first and re-confirms with a locked read, retrying if the task moved meanwhile, which preserves this package's step-lock-before-task-row-lock order. UnarchiveTask and UnarchiveTaskByCascade never got the step lock Build 8 added to their three siblings (Archive/ArchiveIfActive/Delete), so an unarchive racing a reorder could leave a task's stale position colliding with the reorder's freshly-renumbered range. Both are now transactional and take the same lock. DeleteWorkspaceCascade also inverted this package's lock order: it locked task rows before ever touching workflow_steps, while ReorderStepTasks locks the step first — reproduced as a real Postgres deadlock (40P01) under a concurrent reorder. It now locks the workspace's step rows first, matching every other writer. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…op path AC.7 (the insertion-point indicator) had zero behavioral coverage: only the pure computeInsertionEdge/computeKeyboardInsertionEdge comparators were tested, never DroppableTaskRow's actual render of the border classes and data-testid. Export DroppableTaskRow and render it directly (bypassing the virtualizer and KanbanCard tree, neither of which the indicator depends on) to assert the real DOM output for both the pointer (isOver) and keyboard (forceShowIndicator) entry paths. AC.9's drop-outside-any-band path was only exercised via handleDragCancel (Escape); the real path is handleDragEnd's own `if (!over) return`, never invoked by any test. Add a test that calls handleDragEnd with over: null directly. Relabel drop-classification.test.ts's mislabeled "AC.9" case, which passes overId: null straight to classifyDrop — a branch production code never reaches, since classifyDrop is only called after handleDragEnd's guard. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
An earlier i18n:zh-hant re-derivation (211ce11) regressed sidebarWeeks_one/_other from the traditional 週 to the simplified 周 in both zh-hk and zh-tw, silently breaking formats.test.ts's compact-unit assertions. Found while proving this round's backend test failures pre-existing against the merge-base (a full-suite run this card had not previously triggered) — the merge-base carries the correct 週, so this predates today's diff but not the branch, and is a real broken translation rather than an environmental flake. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
lockTaskStepForWrite's confirm-and-retry loop could end up holding two workflow_steps FOR UPDATE locks at once, in discovery order (stale step first, then the task's real step) instead of this package's established ascending-sorted order. Postgres holds a row lock until end of transaction with no in-place unlock, so a retry never released the first attempt's lock before acquiring the second — a real AB-BA deadlock against a concurrent updateTaskWithWorkflowStepAdmission cross-step move over the same two steps (which always locks ascending). Each attempt now runs inside its own savepoint: a mismatch rolls back to it before retrying, so the function never holds more than one step lock at a time. Two new Postgres tests reproduce the exact scenario: one proves the stale lock is released (FOR UPDATE NOWAIT probe from a second connection), the other runs a genuine cross-step move concurrently with the retry and proves neither call hangs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-lock lockTaskStepForWrite's confirming re-read handled a mismatch (task moved to a different step) but not the third outcome: the task left its step entirely (workflow_step_id cleared, e.g. by a concurrent RemoveTaskFromWorkflow, or the row itself gone) while the lock was held. That path returned success without rolling back the just-opened savepoint, leaving the stale step's FOR UPDATE lock held for the rest of the caller's transaction with nothing left to protect — unwarranted contention against any other transaction locking that now-irrelevant step. Fixed by rolling back to the savepoint on that outcome too, mirroring the existing mismatch-retry cleanup. Extracted the three savepoint begin/release/rollback call sites into small helpers to keep the function's cognitive complexity within this package's lint limit. Also corrected a stale doc comment claiming a reused savepoint name "re-establishes" on each attempt — Postgres nests same-named savepoints instead of replacing them, which is harmless here (bounded by maxAttempts, discarded at transaction end) but the comment overstated the model. Renamed and rewrote the deadlock regression test added for the prior fix: it never actually creates lock contention (the paused goroutine holds zero locks, and the test waits for the other side to fully finish before releasing), so it can't observe a real Postgres deadlock — it was redundant with its sibling NOWAIT-probe test. Rewrote its name and comments to claim only what it actually proves. New Postgres regression test proves the fix via the same NOWAIT-probe pattern as the existing sibling test: reverted, it fails with a real SQLSTATE 55P03 (stale lock still held); restored, it passes, stable across repeated -race runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ROLLBACK TO SAVEPOINT releases every lock taken since the savepoint, not only the stale step lock it was meant to drop. When lockTaskStepForWrite found a task had left its step entirely, releasing that step's savepoint also released the confirming read's FOR UPDATE lock on the task's own row, so the function returned success with nothing locked at all. A concurrent transaction could then reattach the task to a new step before the caller's own mutation (archive, delete, unarchive) ran, letting a concurrent reorder of that new step proceed unserialized against it. lockTaskRowIfStepless re-verifies under its own savepoint, locking only the task's row, and either keeps that lock (still no step: caller is protected) or releases it and retries the normal step-lock sequence against the task's new step. Applied to both the confirming-read path and the older top-of-loop early return for a task with no step on its very first read. Extracted resolveTaskStepIDForLock and releaseStaleStepAndRecheckStepless to keep lockTaskStepForWrite's cognitive complexity under the repo's limit.
KanbanMultiState gained orderRevisionByStepId, pendingReorderBandKeys, and withheldReorderByBandKey; these fixtures predate that and now fail typecheck against the widened type.
VirtualizedKanbanCard and useStableExternalLinkAvailability became unreachable once VirtualizedTaskRow started rendering KanbanCard directly; ESLint's no-unused-vars caught the leftover code and its now-unused memo/useMemo imports.
…rd reorder KanbanColumn's memo comparator never compared activeTaskId, keyboardDraft, or onCardKeyDown, so React skipped re-rendering the column when only those props changed. Pressing Space correctly set useKeyboardReorder's internal state, but the card's aria-grabbed attribute never updated because the column's memo silently blocked the re-render. Fixing the comparator exposed a second regression: onCardKeyDown's identity churned on every board-wide task update (pickUp depended on the tasks array by reference), which busted render isolation for every column. Stabilize it with a ref, matching the pattern already used elsewhere in this codebase.
The AC.7 insertion-indicator refactor replaced the memoized VirtualizedKanbanCard with a plain VirtualizedTaskRow rendered directly inside the column's map(), and stopped stabilizing externalLinkAvailability's reference. Both regressions defeated per-card isolation: any task update anywhere in the column re-rendered every row in it. Restore the memo boundary with a comparator that ignores measureElement (a virtualizer ref-callback implementation detail, not a visual signal), and reintroduce the stable externalLinkAvailability reference. Also compute isDeleting/isArchiving per row in the parent instead of forwarding the raw deletingTaskId/archivingTaskId strings, so an unrelated card's busy state doesn't change every row's props.
…d-flight setWithheldReorder was only cleared inside reconcileAndApply, which never runs when the workflow snapshot is gone by the time the request settles (e.g. the user navigated away). The stale withheld entry would otherwise be consumed by that band's next reorder as if it were fresh. Clear it unconditionally in the finally block alongside the pending flag.
useCrossStepMove's failure path restored the whole pre-move tasks array, discarding any other task's concurrent update that landed while the move was in flight. Revert only the moved task's workflowStepId.
The server computes arrival position itself (AC.28); the field's top-of-step meaning it once documented no longer applies.
settledBoundingBox scrolls and polls; reading the drop target's box while the mouse button was already held could scroll mid-drag and stale the captured source coordinates.
…ture kanban.tasks and the workflow snapshot shared one array and its task objects, so a handler that updated only one side could still pass a synchronization assertion that reads both.
Contributor
Author
Fixup round 1Reviewed and replied to all 16 threads from the automated reviewers ( Fixed and pushed (8 threads): withheld-reorder cleanup on snapshot removal, Routed back to Build (3 threads — real findings, but Build-level design/locking work rather than a same-round fixup):
Resolved without a code change (2 threads): a workflow-switch arrival-allocation bypass that predates this PR's merge-base and is already tracked as an out-of-scope pre-existing residual, and a same-step-promotion finding that contradicts the frozen spec's AC.28 (which explicitly treats WIP promotion into a band as an arrival). Deferred as test-rigor residual (3 threads): two Postgres concurrency tests whose synchronization is scheduling-dependent rather than deterministic, and a real-but-heavy-lift gap where the keyboard-reorder picked-up draft isn't rebased against live band membership changes mid-pickup. All three are now tracked in the task plan. Moving this card back to Build for the 3 routed findings above. |
Ordinary task updates no longer stomp a concurrently reordered/arrived position: UpdateTask now preserves the persisted position by re-reading it inside the write transaction, while UpdateTaskWithExplicitPosition keeps the one legitimate literal-position write path. The Go boot payload, live workflow snapshot HTTP endpoint, and workflow_step.* WS events now all carry each step's order_revision, so the frontend seeds orderRevisionByStepId from hydration instead of trusting whichever task.reordered event happens to arrive first after page load. Bulk move now re-reads its batch's tasks under the lock it just acquired and retries with the corrected step set on a mismatch, instead of trusting a pre-lock read that a concurrent move could have invalidated, closing an AB-BA deadlock window against a single MoveTask on the drifted-to step.
acquireBulkMoveStepLocks re-reads a batch's tasks under its just-acquired locks and retries on step-set drift, but discarded the corrected list instead of returning it. BulkMoveSelectedTasks's dispatch loop kept using the pre-lock snapshot for its already-at-target skip check, so a task that left the target step during lock acquisition was silently never moved back, with no error surfaced. BulkMoveTasks had the same stale-snapshot exposure for its REQ-TASKS-KANBAN-TASK-REORDERING-001.29 dispatch order. acquireBulkMoveStepLocks now returns the lock-corrected task list, and both callers re-derive dispatch order from it via orderTasksForBulkMove instead of their pre-lock read.
TestService_BulkMoveTasksReordersAfterSourceStepDrift closes the gap the 6be6411 fix left uncovered: BulkMoveSelectedTasks already had a drift regression test, but BulkMoveTasks's sibling re-derivation (dispatch order from the lock-corrected task list, not the stale pre-lock read) had none. Asserts final position ordering after a mid-batch drift changes which source step ranks first; RED against the pre-fix dispatch order, GREEN against the current code.
…nd-drop-478 # Conflicts: # apps/web/components/kanban-card-content.tsx # apps/web/components/kanban/graph2-task-pipeline.test.tsx # docs/specs/tasks/README.md
healthRoutePath/readyRoutePath/websocketRoutePath replace the repeated "/health", "/ready", "/ws" literals that golangci-lint's goconst rule flags on the merged tree.
…nd-drop-478 # Conflicts: # docs/specs/tasks/README.md
carlosflorencio
self-requested a review
September 13, 2026 02:55
…nd-drop-478 # Conflicts: # apps/web/components/kanban/swimlane-kanban-content.tsx # apps/web/components/kanban/virtualized-column-task-list.tsx
carlosflorencio
deployed
to
opencode-review-trusted
September 13, 2026 03:42 — with 
GitHub Actions
Active
GitHub Actions
Active
Member
|
Pushed a fixup commit that hardens task reordering: it enforces task-write authorization, rechecks the persisted source step before arrival locking, reconciles whole-step HTTP responses across both Kanban projections, and keeps the newest buffered WebSocket revision. I also merged the latest main to clear the PR conflict. Thanks for the contribution! |
carlosflorencio
deployed
to
opencode-review-trusted
September 13, 2026 03:57 — with
GitHub Actions
Active
GitHub Actions
Active
carlosflorencio
had a problem deploying
to
opencode-review-trusted
September 13, 2026 05:07 — with
GitHub Actions
Error
GitHub Actions
Error
AC-TASKS-KANBAN-TASK-REORDERING-001.28 makes an arriving task sort last in its step (highest position) instead of first, so the pre-existing "oldest" task this test used as its scroll target now renders at the top instead of the bottom, and toBeInViewport() no longer finds it at the bottom of the dense column.
…into feature/enable-drag-and-drop-478 Reconciles a race: both sides fixed the same swimlane-height E2E test against AC-TASKS-KANBAN-TASK-REORDERING-001.28's new arrival ordering independently. Kept the upstream fix (denseTaskCount lookup via listTasks) and dropped the duplicate local fix.
Member
|
Follow-up: I also aligned the compact-lane E2E reachability check with the new persisted position ordering. The conflict resolution is in the branch, and the latest head now has 56 passing checks with no failures. |
…nd-drop-478 Resolves a conflict in apps/backend/internal/task/repository/sqlite/task.go between this branch's preservePosition parameter on updateTaskTx (kanban reorder's arrival/reorder position-preservation contract) and main's markerEntryID return value (kdlbs#3533's repeat-task-reassignment fix). Both are kept: updateTaskTx now takes preservePosition and returns markerEntryID. Also fixes ~23 call sites across apps/backend/internal/office/service/*_test.go that still called Service.QueueRun expecting its pre-kdlbs#3613 single-value (error) return; main's tip commit (bd7974d) changed QueueRun to return (QueueOutcome, error) without updating every caller, breaking `go vet`/ `go test -c` for the whole package. Confirmed pre-existing and unrelated to this branch by reproducing byte-identical on a bare origin/main checkout, whose own CI (Run Backend Tests, run 34749445225) is failing for exactly this reason. Fixed here only so this branch's own tree compiles and tests after picking up main.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tip
PR walkthrough: Open the visual walkthrough
Today: Cards inside a kanban column render in whatever order the server happens to return; there's no way to manually rearrange them within a step.
After this: Users can drag a card with the pointer, or pick it up with the keyboard (Space/Enter, Arrow Up/Down, Escape), to reorder cards within a single column band. The new order is persisted, synced live to other viewers over WebSocket, and survives concurrent arrivals/moves without corrupting order.
Who hits this: Anyone using the Kanban board to triage or prioritize tasks within a workflow step.
Scope: Web frontend (dnd-kit pointer drag, keyboard reorder, live insertion indicator, board-ordering comparator) and backend (reorder endpoint, per-step order revision, arrival-position locking,
task.reorderedWS event, bulk-move ordering fix).Not here: Cross-step drag semantics beyond the existing move action; a WIP-queue redesign; the unreachable legacy
swimlane-graph-content.tsxview (left untouched, not in the view registry).Kanban columns had no way to manually reorder cards within a step; this adds pointer and keyboard drag reordering with server-persisted, revision-gated ordering that stays consistent under concurrent arrivals, moves, and multi-viewer WebSocket sync.
Important Changes
PUT /api/v1/workflow-steps/:id/tasks/reorderendpoint with per-steporder_revision(apply-only-if-greater) so a stale reorder response can't clobber a newer live update.compareStepOrder/StepOrderTaskis now the single AC.1 total-order comparator (position, priority rank, queuedAt/createdAt, id), replacing three previously-divergent sort implementations across the desktop/tablet board, mobile board, and Pipeline view.lockTaskStepForWritegained savepoint-based retry-and-reconfirm locking to close several races between a task's step lock and its own row lock (reattach-during-gap, stepless-on-first-read, archive/unarchive not taking the lock) — see the task plan history for the review rounds that found each one.KanbanColumn's memo comparator was missing three reorder-specific props (activeTaskId,keyboardDraft,onCardKeyDown), which silently blocked re-renders and broke the keyboard pick-up indicator; fixed with a regression test.origin/mainin (two rounds, latest at the Pipeline-row/kanban-card-status-strip refactor and the docs-catalog-discovery restructuring); resolved several conflicts where both branches independently extracted overlapping kanban-card sub-components, and extracted three backend route-path constants (healthRoutePath/readyRoutePath/websocketRoutePath) to keepgolangci-lint'sgoconstrule clean on the merged tree.Screenshots
Keyboard pick-up ("Fix flaky checkout test" picked up via Space, shown with the ring highlight and
aria-grabbed="true"):Pointer drag in progress ("Investigate memory leak in worker" being dragged above "Fix flaky checkout test"):
Validation
make fmt,make typecheck test lint,make lint-format— clean on the merged tree.cd apps/web && pnpm run i18n:ratchet— clean.pnpm run test(full web vitest, 17k+ tests) — clean exceptlib/http-git-server.test.ts, a pre-existing failure caused by no Docker daemon in this environment (zero diff fromorigin/main).make test-e2efull run, plus a focusede2e/tests/kanban/rerun against a freshly rebuilt frontend bundle (99/101 passed) — clean except two pre-existing/environmental failures, both traced to the same flaky e2e-reset teardown ("unsafe worktree path ... not a directory") and confirmed to reproduce identically onorigin/mainalone and intermittently in isolation on this branch's merged tree; unrelated to this branch's code.postgres:16-alpinecontainer, RED/GREEN proven for every locking fix via revert/restore,-raceclean.kanban-column.render-stability.test.tsx(memo comparator),virtualized-column-task-list.render-stability.test.tsx(per-card isolation), plus the existing reorder Playwright spec (kanban-reorder.spec.ts) covering pointer and keyboard reorder end to end.Possible Improvements
Low risk — changes are isolated to the kanban board and its task-step locking; a few test-rigor-only residuals (Postgres coverage gaps, an indirect Pipeline-view assertion) are tracked as non-blocking follow-ups rather than fixed here.
Checklist
apps/web/), I have added or updated Playwright e2e tests inapps/web/e2e/and verified them withmake test-e2e.docs/public/**and updated them or noted why no docs change is needed.Design docs