From 763bf563fa175bab7e3cbc982e573dda8f71ca4d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 7 May 2026 22:28:22 +0000 Subject: [PATCH] fix(veans): wire-format alignment + dedicated bucket-move endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several fixes uncovered by running the e2e suite end-to-end against a real Vikunja: - view_kind / bucket_configuration_mode are serialized as strings via custom MarshalJSON on the parent enums, not ints. Update the client types and ViewKind* constants accordingly. - POST /tasks/{id} doesn't move tasks between buckets (the relation lives in a separate task_buckets table). Add MoveTaskToBucket using the dedicated POST /projects/{p}/views/{v}/buckets/{b}/tasks endpoint and call it from create (when status != todo), update (status transitions), and claim. Also add the corresponding action to the bot's permission grant via PermissionsForBot. - Bot creation lives at PUT /user/bots, not /bots — the route is scoped under the /user subgroup. Update both Create and List. - Task.BucketID is 0 on read (xorm:"-"); the actual bucket surfaces via ?expand=buckets as a Buckets slice. GetTask now requests the expand and CurrentBucketID resolves the per-view entry. - Chain.Set falls through to the next backend on failure (e.g. keyring on a host with no dbus). Previously a single backend error aborted the chain, breaking init in CI/headless environments. - E2e suite: identifier now derived from the high-entropy tail of the base-36 timestamp (the head was the runner's hostname prefix and collided across runs); commit.gpgsign disabled in workspace setup. Local e2e run against the rebuilt API passes all 7 tests. --- veans/e2e/claim_test.go | 5 +++-- veans/e2e/helpers.go | 4 ++++ veans/e2e/init_test.go | 28 ++++++++++++++------------- veans/e2e/shared_test.go | 2 +- veans/internal/client/buckets.go | 17 ++++++++++++++++ veans/internal/client/routes.go | 30 ++++++++++++++++++----------- veans/internal/client/tasks.go | 29 ++++++++++++++++++++++++++-- veans/internal/client/types.go | 22 +++++++++++++++------ veans/internal/client/users.go | 16 +++++++-------- veans/internal/commands/claim.go | 15 ++++++++------- veans/internal/commands/create.go | 16 ++++++--------- veans/internal/commands/list.go | 7 ++++--- veans/internal/commands/show.go | 2 +- veans/internal/commands/update.go | 17 ++++++++++++++-- veans/internal/credentials/store.go | 10 +++++++++- 15 files changed, 153 insertions(+), 67 deletions(-) diff --git a/veans/e2e/claim_test.go b/veans/e2e/claim_test.go index bd26d8e32..a0e7d6695 100644 --- a/veans/e2e/claim_test.go +++ b/veans/e2e/claim_test.go @@ -52,8 +52,9 @@ func TestClaim_AssignsBotMovesToInProgressTagsBranch(t *testing.T) { // Verify bucket transition by reading the workspace's .veans.yml — the // bot's expected In Progress bucket is stored there. cfg := loadConfig(t, ws) - if server.BucketID != cfg.Buckets.InProgress { - t.Fatalf("task not in In Progress bucket: got %d, want %d", server.BucketID, cfg.Buckets.InProgress) + bucket := server.CurrentBucketID(cfg.ViewID) + if bucket != cfg.Buckets.InProgress { + t.Fatalf("task not in In Progress bucket: got %d, want %d", bucket, cfg.Buckets.InProgress) } // Bot assigned. diff --git a/veans/e2e/helpers.go b/veans/e2e/helpers.go index 6dd3c695a..56943cd4c 100644 --- a/veans/e2e/helpers.go +++ b/veans/e2e/helpers.go @@ -121,6 +121,10 @@ func (h *Harness) NewWorkspace(t *testing.T) *Workspace { {"git", "init", "-q", "-b", "main"}, {"git", "config", "user.email", "veans-e2e@example.com"}, {"git", "config", "user.name", "veans-e2e"}, + // Disable any inherited commit signing; the test commit doesn't + // need provenance and signing brokers can fail in dev containers. + {"git", "config", "commit.gpgsign", "false"}, + {"git", "config", "tag.gpgsign", "false"}, } { cmd := exec.CommandContext(t.Context(), c[0], c[1:]...) cmd.Dir = dir diff --git a/veans/e2e/init_test.go b/veans/e2e/init_test.go index 2a62eb97e..61dd8b1e4 100644 --- a/veans/e2e/init_test.go +++ b/veans/e2e/init_test.go @@ -19,7 +19,7 @@ package e2e import ( "context" "fmt" - "os" + "strconv" "strings" "testing" "time" @@ -38,7 +38,7 @@ func TestInit_HappyPath(t *testing.T) { // Use a unique title and identifier per run so parallel jobs don't // collide on the bot username. suffix := uniqueSuffix() - project := h.CreateProject(t, "veans-e2e-"+suffix, "VE"+strings.ToUpper(suffix[:4])) + project := h.CreateProject(t, "veans-e2e-"+suffix, identifier(suffix)) view := h.FindKanbanView(t, project.ID) ws := h.NewWorkspace(t) @@ -155,19 +155,21 @@ func TestInit_NoIdentifierFallsBackToHashNN(t *testing.T) { } } -// uniqueSuffix returns a short random-ish slug for naming test artifacts. -// Time-based is fine here — tests don't need cryptographic uniqueness. +// uniqueSuffix returns a short slug derived from the current nanosecond +// timestamp, base-36-encoded so every character is alphanumeric. Tests +// also use this slug as a project identifier, which Vikunja caps at 10 +// chars, so the encoding has to be compact and free of separators. func uniqueSuffix() string { - hostname, _ := os.Hostname() - if hostname == "" { - hostname = "host" - } - return strings.ToLower(fmt.Sprintf("%s-%d", trunc(hostname, 4), time.Now().UnixNano()))[:18] + return strconv.FormatInt(time.Now().UnixNano(), 36) } -func trunc(s string, n int) string { - if len(s) <= n { - return s +// identifier returns a stable 10-char-or-fewer slug for use as a Vikunja +// project identifier. The base-36 timestamp's most-significant chars +// barely change across consecutive runs, so we use the trailing chars +// (which carry the nanosecond entropy) and uppercase them. +func identifier(suffix string) string { + if len(suffix) > 8 { + suffix = suffix[len(suffix)-8:] } - return s[:n] + return strings.ToUpper(suffix) } diff --git a/veans/e2e/shared_test.go b/veans/e2e/shared_test.go index 6211290d7..8dac7ccab 100644 --- a/veans/e2e/shared_test.go +++ b/veans/e2e/shared_test.go @@ -31,7 +31,7 @@ func provisionWorkspace(t *testing.T) (*Workspace, *Harness) { t.Helper() h := New(t) suffix := uniqueSuffix() - project := h.CreateProject(t, "veans-e2e-"+suffix, "VE"+strings.ToUpper(suffix[:4])) + project := h.CreateProject(t, "veans-e2e-"+suffix, identifier(suffix)) view := h.FindKanbanView(t, project.ID) ws := h.NewWorkspace(t) diff --git a/veans/internal/client/buckets.go b/veans/internal/client/buckets.go index d153357b4..ba4c324e5 100644 --- a/veans/internal/client/buckets.go +++ b/veans/internal/client/buckets.go @@ -44,3 +44,20 @@ func (c *Client) CreateBucket(ctx context.Context, projectID, viewID int64, b *B } return &out, nil } + +// MoveTaskToBucket positions an existing task in `bucketID` on the +// project's view. Vikunja stores task↔bucket relations in a separate +// table (`task_buckets`), so POST /tasks/{id} with bucket_id does not +// reliably move tasks — this dedicated endpoint is the one the Kanban +// UI's drag-and-drop uses. +func (c *Client) MoveTaskToBucket(ctx context.Context, projectID, viewID, bucketID, taskID int64) error { + path := fmt.Sprintf("/projects/%d/views/%d/buckets/%d/tasks", + projectID, viewID, bucketID) + body := map[string]int64{ + "task_id": taskID, + "project_view_id": viewID, + "bucket_id": bucketID, + "project_id": projectID, + } + return c.Do(ctx, "POST", path, nil, body, nil) +} diff --git a/veans/internal/client/routes.go b/veans/internal/client/routes.go index 93de7e1d8..179773f20 100644 --- a/veans/internal/client/routes.go +++ b/veans/internal/client/routes.go @@ -42,23 +42,31 @@ func (c *Client) Routes(ctx context.Context) (map[string]RouteGroup, error) { // the server are silently dropped, so the resulting permission map is // always valid for PUT /tokens regardless of Vikunja version. // -// The set is intentionally tasks-centric — the bot doesn't need to manage -// users, teams, or webhooks. We grant `read_one`/`read_all` on projects so -// the bot can resolve PROJ-NN and #NN identifiers, but no project mutation. +// The action names reflect Vikunja's actual route map (see GET /routes): +// bucket CRUD and the bucket-task move endpoint live under the `projects` +// group as `views_buckets*` and `views_buckets_tasks`, not a separate +// `buckets` group. func PermissionsForBot(routes map[string]RouteGroup) map[string][]string { wanted := map[string][]string{ - "tasks": {"read_one", "read_all", "create", "update", "delete"}, - "projects": {"read_one", "read_all"}, + // Read + write tasks across the project. The bot creates, updates, + // and reads tasks; it doesn't delete (humans/merge hook close). + "tasks": { + "read_one", "read_all", "create", "update", "position", + "read", "update_bulk", + }, + // Project access: read project metadata, manage buckets & move + // tasks between them. tasks_by-index resolves #NN / PROJ-NN. + "projects": { + "read_one", "read_all", "tasks_by-index", + "views_buckets", "views_buckets_put", "views_buckets_post", + "views_buckets_delete", "views_buckets_tasks", + }, "projects_views": {"read_one", "read_all"}, - "buckets": {"read_one", "read_all", "create", "update", "delete"}, "labels": {"read_one", "read_all", "create", "update", "delete"}, - "comments": {"read_one", "read_all", "create", "update", "delete"}, "tasks_comments": {"read_one", "read_all", "create", "update", "delete"}, - "relations": {"create", "delete"}, "tasks_relations": {"create", "delete"}, - "assignees": {"read_all", "create", "delete"}, - "tasks_assignees": {"read_all", "create", "delete"}, - "tasks_labels": {"create", "delete", "read_all"}, + "tasks_assignees": {"read_all", "create", "delete", "update_bulk"}, + "tasks_labels": {"create", "delete", "read_all", "update_bulk"}, } out := map[string][]string{} for group, actions := range wanted { diff --git a/veans/internal/client/tasks.go b/veans/internal/client/tasks.go index fd7f61622..e342fd372 100644 --- a/veans/internal/client/tasks.go +++ b/veans/internal/client/tasks.go @@ -84,15 +84,40 @@ func (c *Client) ListProjectTasks(ctx context.Context, projectID int64, opts *Ta } } -// GetTask fetches a single task by numeric ID. +// GetTask fetches a single task by numeric ID. expand=buckets is requested +// because Vikunja's bare GET returns bucket_id=0 — the per-view bucket +// memberships only surface under the Buckets slice. func (c *Client) GetTask(ctx context.Context, id int64) (*Task, error) { var out Task - if err := c.Do(ctx, "GET", fmt.Sprintf("/tasks/%d", id), nil, nil, &out); err != nil { + q := url.Values{} + q.Add("expand", "buckets") + if err := c.Do(ctx, "GET", fmt.Sprintf("/tasks/%d", id), q, nil, &out); err != nil { return nil, err } return &out, nil } +// CurrentBucketID returns the task's bucket id on the given project view, +// or 0 if no bucket entry is present (which happens when buckets aren't +// expanded, or the task is in no view-bound bucket yet). +func (t *Task) CurrentBucketID(viewID int64) int64 { + if t.BucketID != 0 { + return t.BucketID + } + for _, b := range t.Buckets { + if b == nil { + continue + } + // Buckets returned via expand=buckets are scoped to the requesting + // view; without view scoping the slice can include entries from + // every view this task belongs to. + if viewID == 0 || b.ProjectViewID == viewID || b.ProjectViewID == 0 { + return b.ID + } + } + return 0 +} + // CreateTask inserts a task into a project (PUT /projects/{id}/tasks). func (c *Client) CreateTask(ctx context.Context, projectID int64, t *Task) (*Task, error) { var out Task diff --git a/veans/internal/client/types.go b/veans/internal/client/types.go index 14c490e5d..576799334 100644 --- a/veans/internal/client/types.go +++ b/veans/internal/client/types.go @@ -54,19 +54,24 @@ type Project struct { } // ProjectView is a saved view (Kanban/List/Gantt/Table) on a project. +// view_kind is serialized as a string on the wire ("list" / "gantt" / +// "table" / "kanban"), not an int — Vikunja's ProjectViewKind has a +// custom MarshalJSON. type ProjectView struct { ID int64 `json:"id"` Title string `json:"title"` ProjectID int64 `json:"project_id"` - ViewKind int `json:"view_kind"` - BucketConfMode int `json:"bucket_configuration_mode,omitempty"` + // view_kind / bucket_configuration_mode are serialized as strings on + // the wire (custom MarshalJSON on the parent enums), not ints. + ViewKind string `json:"view_kind"` + BucketConfMode string `json:"bucket_configuration_mode,omitempty"` } const ( - ViewKindList = 0 - ViewKindGantt = 1 - ViewKindTable = 2 - ViewKindKanban = 3 + ViewKindList = "list" + ViewKindGantt = "gantt" + ViewKindTable = "table" + ViewKindKanban = "kanban" ) // Bucket is a kanban bucket bound to a single project view. @@ -93,7 +98,12 @@ type Task struct { Position float64 `json:"position,omitempty"` Created time.Time `json:"created,omitempty"` Updated time.Time `json:"updated,omitempty"` + // BucketID is only set by Vikunja when sending a task to a server- + // side endpoint (e.g. the bucket-move POST); reads return it as 0. + // The current bucket(s) — one per Kanban view — are exposed via + // ?expand=buckets in the Buckets slice. BucketID int64 `json:"bucket_id,omitempty"` + Buckets []*Bucket `json:"buckets,omitempty"` Assignees []*User `json:"assignees,omitempty"` Labels []*Label `json:"labels,omitempty"` StartDate *time.Time `json:"start_date,omitempty"` diff --git a/veans/internal/client/users.go b/veans/internal/client/users.go index 84fc1e721..95e32ea0d 100644 --- a/veans/internal/client/users.go +++ b/veans/internal/client/users.go @@ -24,22 +24,22 @@ import ( "code.vikunja.io/veans/internal/output" ) -// CreateBotUser provisions a bot user via PUT /bots. The username must be -// prefixed `bot-` (Vikunja enforces this). The caller becomes the bot's +// CreateBotUser provisions a bot user via PUT /user/bots. The username must +// be prefixed `bot-` (Vikunja enforces this). The caller becomes the bot's // owner, which is what allows them to mint API tokens for the bot via // PUT /tokens with owner_id. // -// On Vikunja versions that predate the /bots endpoint, the server returns -// 404, which we surface as BOT_USERS_UNAVAILABLE so init can fail fast with -// a clear message. +// On Vikunja versions that predate the /user/bots endpoint, the server +// returns 404, which we surface as BOT_USERS_UNAVAILABLE so init can fail +// fast with a clear message. func (c *Client) CreateBotUser(ctx context.Context, username, name string) (*BotUser, error) { var out BotUser - err := c.Do(ctx, "PUT", "/bots", nil, &BotUserCreate{Username: username, Name: name}, &out) + err := c.Do(ctx, "PUT", "/user/bots", nil, &BotUserCreate{Username: username, Name: name}, &out) if err != nil { var oe *output.Error if errors.As(err, &oe) && oe.Code == output.CodeNotFound { return nil, output.Wrap(output.CodeBotUsersUnavailable, err, - "this Vikunja instance does not expose /bots — upgrade to a newer version") + "this Vikunja instance does not expose /user/bots — upgrade to a newer version") } return nil, err } @@ -49,7 +49,7 @@ func (c *Client) CreateBotUser(ctx context.Context, username, name string) (*Bot // ListBotUsers returns all bot users owned by the authenticated user. func (c *Client) ListBotUsers(ctx context.Context) ([]*BotUser, error) { var out []*BotUser - if err := c.Do(ctx, "GET", "/bots", nil, nil, &out); err != nil { + if err := c.Do(ctx, "GET", "/user/bots", nil, nil, &out); err != nil { return nil, err } return out, nil diff --git a/veans/internal/commands/claim.go b/veans/internal/commands/claim.go index 0f09d3e41..ea6b2bff5 100644 --- a/veans/internal/commands/claim.go +++ b/veans/internal/commands/claim.go @@ -23,7 +23,6 @@ import ( "github.com/spf13/cobra" - "code.vikunja.io/veans/internal/client" "code.vikunja.io/veans/internal/output" "code.vikunja.io/veans/internal/status" ) @@ -43,16 +42,18 @@ func newClaimCmd() *cobra.Command { return err } - // Move to In Progress. + // Move to In Progress. Vikunja's task↔bucket relation lives + // in a separate table; POST /tasks doesn't move buckets, so + // use the dedicated endpoint. bid, err := status.BucketID(status.InProgress, rt.cfg.Buckets) if err != nil { return err } - task, err := rt.client.UpdateTask(cmd.Context(), id, &client.Task{ - ID: id, - BucketID: bid, - Done: false, - }) + if err := rt.client.MoveTaskToBucket(cmd.Context(), + rt.cfg.ProjectID, rt.cfg.ViewID, bid, id); err != nil { + return err + } + task, err := rt.client.GetTask(cmd.Context(), id) if err != nil { return err } diff --git a/veans/internal/commands/create.go b/veans/internal/commands/create.go index ed06ad5af..5a8d6d5fe 100644 --- a/veans/internal/commands/create.go +++ b/veans/internal/commands/create.go @@ -93,18 +93,14 @@ func runCreate(ctx context.Context, rt *runtime, title string, f *createFlags) ( return nil, err } - // If the initial bucket isn't where Vikunja put it (defaults to first - // bucket on the view), nudge it explicitly. - if created.BucketID != bucketID { - updated, err := rt.client.UpdateTask(ctx, created.ID, &client.Task{ - ID: created.ID, - BucketID: bucketID, - Done: st.Done(), - }) - if err != nil { + // Vikunja places newly-created tasks in the view's default bucket + // regardless of bucket_id in the create payload — move it explicitly + // when the requested status isn't Todo. + if st != status.Todo { + if err := rt.client.MoveTaskToBucket(ctx, + rt.cfg.ProjectID, rt.cfg.ViewID, bucketID, created.ID); err != nil { return nil, output.Wrap(output.CodeUnknown, err, "set initial bucket: %v", err) } - created = updated } // Attach labels (lazily creating them under veans: namespace). diff --git a/veans/internal/commands/list.go b/veans/internal/commands/list.go index 7ae42c899..7527eb6ec 100644 --- a/veans/internal/commands/list.go +++ b/veans/internal/commands/list.go @@ -94,8 +94,9 @@ func runList(cmd *cobra.Command, rt *runtime, f *listFlags) ([]*client.Task, err // Apply client-side filters AND-style. var out []*client.Task for _, t := range tasks { + taskBucket := t.CurrentBucketID(rt.cfg.ViewID) if f.ready { - if t.Done || t.BucketID != rt.cfg.Buckets.Todo { + if t.Done || taskBucket != rt.cfg.Buckets.Todo { continue } } @@ -126,7 +127,7 @@ func runList(cmd *cobra.Command, rt *runtime, f *listFlags) ([]*client.Task, err return nil, err } wantBucket, _ := status.BucketID(s, rt.cfg.Buckets) - if t.BucketID == wantBucket { + if taskBucket == wantBucket { ok = true break } @@ -164,7 +165,7 @@ func renderTasksHuman(w fmtWriter, tasks []*client.Task, cfg *config.Config) { return } for _, t := range tasks { - s := status.FromBucketID(t.BucketID, cfg.Buckets) + s := status.FromBucketID(t.CurrentBucketID(cfg.ViewID), cfg.Buckets) stamp := string(s) if stamp == "" { stamp = "-" diff --git a/veans/internal/commands/show.go b/veans/internal/commands/show.go index 5fe8bf5b2..a14661ed1 100644 --- a/veans/internal/commands/show.go +++ b/veans/internal/commands/show.go @@ -56,7 +56,7 @@ func newShowCmd() *cobra.Command { } func renderTaskHuman(w fmtWriter, t *client.Task, cfg *config.Config) { - s := status.FromBucketID(t.BucketID, cfg.Buckets) + s := status.FromBucketID(t.CurrentBucketID(cfg.ViewID), cfg.Buckets) fmt.Fprintf(w, "%s %s [%s]\n", cfg.FormatTaskID(t.Index), t.Title, s) if t.Priority > 0 { fmt.Fprintf(w, "Priority: %d\n", t.Priority) diff --git a/veans/internal/commands/update.go b/veans/internal/commands/update.go index 5d37c5c26..be93f660e 100644 --- a/veans/internal/commands/update.go +++ b/veans/internal/commands/update.go @@ -73,7 +73,7 @@ func newUpdateCmd() *cobra.Command { if globals.JSON { return json.NewEncoder(cmd.OutOrStdout()).Encode(task) } - s := status.FromBucketID(task.BucketID, rt.cfg.Buckets) + s := status.FromBucketID(task.CurrentBucketID(rt.cfg.ViewID), rt.cfg.Buckets) fmt.Fprintf(cmd.OutOrStdout(), "Updated %s [%s] %s\n", rt.cfg.FormatTaskID(task.Index), s, task.Title) return nil @@ -151,12 +151,17 @@ func runUpdate(ctx context.Context, rt *runtime, id int64, f *updateFlags) (*cli dirty = true } + // Status transitions: `done` is set on the task body (Update processes + // it natively), but the bucket move uses the dedicated TaskBucket + // endpoint after the field update so the change is visible on the + // Kanban view. + var bucketTransitionTarget int64 if newStatus != "" { bid, err := status.BucketID(newStatus, rt.cfg.Buckets) if err != nil { return nil, err } - body.BucketID = bid + bucketTransitionTarget = bid body.Done = newStatus.Done() dirty = true } @@ -184,6 +189,14 @@ func runUpdate(ctx context.Context, rt *runtime, id int64, f *updateFlags) (*cli updated = u } + // Move the task between buckets after the field update. + if bucketTransitionTarget != 0 { + if err := rt.client.MoveTaskToBucket(ctx, + rt.cfg.ProjectID, rt.cfg.ViewID, bucketTransitionTarget, id); err != nil { + return nil, err + } + } + // Label add/remove run after the field update so a status transition // can't clobber freshly-attached labels. for _, raw := range f.addLabels { diff --git a/veans/internal/credentials/store.go b/veans/internal/credentials/store.go index e4eaf01f7..619b52280 100644 --- a/veans/internal/credentials/store.go +++ b/veans/internal/credentials/store.go @@ -64,7 +64,12 @@ func (c *Chain) Get(server, account string) (string, error) { } // Set writes to the first backend that accepts a write. Env is read-only. +// Backends that error out (e.g. keyring on a host with no dbus) are skipped +// transparently, falling through to the next — the file backend is the +// reliable last-resort. Only if every writable backend fails do we surface +// the last error. func (c *Chain) Set(server, account, token string) error { + var lastErr error for _, b := range c.Backends { if _, ok := b.(*EnvBackend); ok { continue @@ -72,9 +77,12 @@ func (c *Chain) Set(server, account, token string) error { if err := b.Set(server, account, token); err == nil { return nil } else if !errors.Is(err, errReadOnly) { - return fmt.Errorf("%s: %w", b.Name(), err) + lastErr = fmt.Errorf("%s: %w", b.Name(), err) } } + if lastErr != nil { + return lastErr + } return errors.New("no writable backend available") }