address pr-swarm finding: ignoring a caller supplied index on the create path
also stripped it from the vikunja-file restore, which is not a request path. A
restore renumbered every task, so PROJ-17 in links and comments came back as
PROJ-4 - while the importer deliberately preserves the identifier those
references are built from.
The other importers never set an index, so the opt-in is a no-op for them.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
address pr-swarm finding: it was a bare errors.New, so a project already holding
an index past the ceiling - reachable on every shipped version, which honours a
client supplied index - answered every later create with an opaque 500. 409 with
the project named says what is wrong.
Healing such a project is left alone on purpose: which index the tasks should
get instead is a migration decision.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
updateSingleTask rebuilt the task from a plain row read, where created_by is not
a column, so the response only ever carried a creator when the client had sent
one back itself. Now that the client stopped sending a field it is not supposed
to write, the task detail view showed "created by" with no name until reload.
Looked up from the id the row already holds, so every consumer gets it, not
just the one that used to echo it.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
address pr-swarm finding: grouping the conflicts by view left them in map order,
and the order is observable - resolving a lower group moves its tasks toward the
next one, so which runs first decides whether the view gets a local respace or a
full recalculation. Same data, different result per run.
Also restores the count == 0 early return dropped when that guard was split, and
lets the placement helper own the batch state so a caller cannot hand it tasks
the state was not built from.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
address pr-swarm finding: the previous guard only clamped the value a client
sent. It still honoured anything up to the ceiling, so one request could pin a
project's counter at a billion and every later task got a ten digit identifier,
and calculateNextTaskIndex could still overflow on a project already holding a
poisoned row.
The request path now ignores the field entirely, matching what it is documented
as. Project duplication is the one caller that needs a picked index, and says so
through createTaskOpts. calculateNextTaskIndex refuses to hand out an index past
the ceiling instead of wrapping, which covers the cross project move too.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
address pr-swarm finding: resolvePositionConflictsAfterInsert ran one select per
inserted position - roughly 800 of them inside one transaction for a 200 task
batch across four views, while holding the view locks. Same check, grouped per
view and chunked like the inserts.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
address pr-swarm finding: BulkTaskCreate.Create carried 50 lines of position
work while every other position rule lives here. The helper takes the insert as
a callback so "make room, then insert, then spread" cannot be called in the
wrong order - making room can trigger a recalculation, and a recalculation
orders by position, which tasks inserted too early do not have yet.
It also clones the view slice before sorting it by id for the lock walk. The
sort used to hit the slice held in the shared state, so the bulk path processed
views in id order while everything else uses their position order.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
address pr-swarm finding: index is documented read only but huma does not strip
read only fields from a request body, so a caller could post index MaxInt64.
calculateNextTaskIndex then wrapped to MinInt64 and every later create in that
project collided on the unique (project, index) constraint - one request, task
creation in a shared project broken for everyone, permanently.
Internal callers legitimately pass an index (duplication keeps task numbering),
so an implausible one now falls back to the server assigned next index instead
of being rejected outright.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
address pr-swarm finding: the type carried a "always go through the constructor,
the maps have to exist" warning, which CLAUDE.md calls a redesign signal. Each
accessor lazy-inits its own map now, so the zero value works and the warning
goes away. Renamed off "cache" as well - it hands out task indexes and mutates
tasks, it is state, not a lookup table.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
Pasting a long list is the workflow this endpoint exists for, and 100 lines is
not a lot of list. The message naming the limit stays for anything above it.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
createTask redid the same work for every task of a batch: fetching the project,
the doer, the project's views, each kanban view's default bucket, the highest
task index, and updating the project's last-updated timestamp. A 20 task batch
measured 207 statements against 22 for a single create, and 100 sequential
updates of one projects row serialize every other writer touching that project.
createTasks now does the n tasks case and createTask is the n=1 call into it, so
the contract of a single create is unchanged. Project duplication, the other
caller creating tasks in a loop, goes through it as well.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
createPositionsForTasksInView placed tasks by the same rule as
makeRoomAtTopOfView plus spreadTasksAtTopOfView, making three implementations of
it in this file. It also returned right after a recalculation, leaving the tasks
it was called for unpositioned until the next heal; going through the helpers
places them straight away.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
Position is documented read only and set through its own endpoint, so honouring
one passed to the bulk create contradicted the rest of the api - and nothing
called for it. The batch is now spread over the gap by slice order alone.
Counting the tasks off by their slice position rather than their task index also
keeps them apart in a view spanning several projects, where indexes repeat.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
setBucket together with skipPositions is a new combination and nothing asserted
tasks still land in a bucket.
Address pr-swarm finding: untested setBucket and skipPositions combination.
CanUpdate materializes every requested task before any permission check runs,
so an unbounded list let anyone force a giant IN query on ids they cannot see.
Cap it at the schema layer like the bulk create, and keep an empty-list guard
for the v1 route, which binds through Echo and never sees the Huma tags.
Address pr-swarm finding: unbounded bulk update input.
A task created with the done bucket of a view flips to done and went through
moveTaskToDoneBuckets, which wrote a position row. The batch's own slot was
then dropped on conflict and the task fell out of the entered order.
Address pr-swarm finding: skipPositions violated by the done bucket path.
Two concurrent batches read the same lowest position and computed
byte-identical slots, which is the arbitrary ordering this branch set out to
remove. Take the same lock the single-task path takes, walk projects and views
in a stable id order so two batches cannot deadlock, and run the conflict
repair after inserting.
Address pr-swarm finding: unserialized position read in the batch path.
Creates the tasks in one transaction and places them above everything the
views already contain, spread out so they keep the order they were passed in.
Doing that one task at a time puts them all in the same spot and leaves the
final order to the position conflict repair.
Claude-Session: https://claude.ai/code/session_01GR9u1EFvq994jqGoeJqYSX
Nested subtasks were returned in the order their relations were created, so
they ignored the view's own order - reordering a subtask by dragging it, or
sorting the view, left the nesting untouched.
Positions only exist per view, and not every caller reads tasks through one:
the task detail page, CalDAV and exports pass no view, and a subtask in another
project has no row for this view either. Those keep relation id order, which
stays stable, and sort after the ones that do have a position.
Route-level tests on both api versions, since the pseudo-projects only get
their parent through the read path. The v2 read body embeds models.Project and
Huma's $schema wrapper copies its fields into a generated struct, so a custom
MarshalJSON on the model would not have applied there — assert the raw JSON
instead of the struct.
781ffac19 (GHSA-44v6-7fxq-vgf4) turned ParentProjectID into a *int64 with
`omitempty` so a write can tell an omitted field from an explicit 0 (detach,
admin only). That also dropped the key from every response where the pointer is
nil: the Favorites and saved-filter pseudo-projects, which are built in Go and
never set it, plus rows whose nullable parent_project_id column was never
backfilled. Clients parse the field as a plain int, so the whole project list
fails to decode (go-vikunja/app#295).
Keep the pointer and its three-way write semantics, but never hand out a nil:
normalize DB rows in AfterLoad and set the field where the pseudo-projects are
built.
Closesgo-vikunja/app#295
Both the listener and the cron bailed out completely as soon as a single saved
filter errored, so no filter view in the instance got updated. Skip the failing
filter instead. Genuine db errors still bubble up so the retry middleware can
do its job.
Evaluating a saved filter needs the list of projects its owner has access to,
which getRawProjectsForUser refuses to return for disabled or locked accounts.
Since the listener walks every manual kanban filter view in the instance, a
single filter of a disabled user made the whole handler fail with "Account is
disabled" - for every task update, for everyone. The recalculation which would
un-crowd the positions is the very thing that failed, so the state never healed.
Every task creation looks for rows with the same position in the view to
resolve conflicts. Without an index on position that scans all positions of
the view, so creating n tasks in one project costs O(n²) row reads - the
second half of the slow import in #3297.
Tasks imported without a position are inserted in front of the lowest one by
halving it. That reaches the minimum spacing every few dozen inserts, which
recalculates every position in the view and makes a large import O(n²) - a
10k task TickTick export with an empty Order column never finished.
Numbering the tasks up front skips the halving entirely and keeps the order
the export was written in.
Closes#3297
Registers the full v2 API and fails on Huma's 'unable to create schema link' stderr warning, catching any future response body type Huma cannot wrap. Fixes#3272.
The unshadowed promoted method made LinkSharing's value method set non-empty, which breaks Huma's $schema wrapper for response bodies embedding it (reflect.StructOf, go#15924).
kolaente
Frederick [Bot]