From 4ac89741e3abea67265837f1f78857da5ed65483 Mon Sep 17 00:00:00 2001 From: Tink bot Date: Tue, 26 May 2026 22:46:38 +0200 Subject: [PATCH] feat(veans): reuse owned bot or prompt for fresh name on collision --- veans/go.mod | 1 + veans/go.sum | 2 + veans/internal/bootstrap/bootstrap.go | 7 +- veans/internal/bootstrap/botuser.go | 154 +++++++++++++++++++++++ veans/internal/bootstrap/botuser_test.go | 139 ++++++++++++++++++++ veans/internal/client/users.go | 17 +++ 6 files changed, 317 insertions(+), 3 deletions(-) create mode 100644 veans/internal/bootstrap/botuser.go create mode 100644 veans/internal/bootstrap/botuser_test.go diff --git a/veans/go.mod b/veans/go.mod index 6b3bcd58e..cf110a333 100644 --- a/veans/go.mod +++ b/veans/go.mod @@ -12,6 +12,7 @@ require ( require ( github.com/danieljoos/wincred v1.2.3 // indirect + github.com/dustinkirkland/golang-petname v0.0.0-20260215035315-f0c533e9ce9b // indirect github.com/godbus/dbus/v5 v5.2.2 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/spf13/pflag v1.0.9 // indirect diff --git a/veans/go.sum b/veans/go.sum index 286500900..cfb90c787 100644 --- a/veans/go.sum +++ b/veans/go.sum @@ -3,6 +3,8 @@ github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMF github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7MRLQK4X0bs= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dustinkirkland/golang-petname v0.0.0-20260215035315-f0c533e9ce9b h1:qZ21OofI7zneC9dOEqul4FmIWz/YjJJMrf6fL7jrFYQ= +github.com/dustinkirkland/golang-petname v0.0.0-20260215035315-f0c533e9ce9b/go.mod h1:8AuBTZBRSFqEYBPYULd+NN474/zZBLP+6WeT5S9xlAc= github.com/godbus/dbus/v5 v5.2.2 h1:TUR3TgtSVDmjiXOgAAyaZbYmIeP3DPkld3jgKGV8mXQ= github.com/godbus/dbus/v5 v5.2.2/go.mod h1:3AAv2+hPq5rdnr5txxxRwiGjPXamgoIHgz9FPBfOp3c= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= diff --git a/veans/internal/bootstrap/bootstrap.go b/veans/internal/bootstrap/bootstrap.go index 9ab291069..7432f53b4 100644 --- a/veans/internal/bootstrap/bootstrap.go +++ b/veans/internal/bootstrap/bootstrap.go @@ -191,12 +191,13 @@ func Init(ctx context.Context, opts *Options) (*Result, error) { return nil, err } - // 8. Create the bot user. - bot, err := human.CreateBotUser(ctx, botUsername, "veans bot for "+project.Title) + // 8. Resolve the bot user: reuse one we already own if the name is + // taken by us, prompt for a fresh name (with a petname suggestion) + // if the name is taken by someone else, otherwise create new. + bot, err := resolveBotUser(ctx, human, botUsername, project.Title, prompter, opts.Out) if err != nil { return nil, err } - progress(opts.Out, "Created bot user %q (id=%d)", bot.Username, bot.ID) // 9. Share the project with the bot. if _, err := human.ShareProjectWithUser(ctx, project.ID, &client.ProjectUser{ diff --git a/veans/internal/bootstrap/botuser.go b/veans/internal/bootstrap/botuser.go new file mode 100644 index 000000000..22650e800 --- /dev/null +++ b/veans/internal/bootstrap/botuser.go @@ -0,0 +1,154 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package bootstrap + +import ( + "context" + "errors" + "fmt" + "io" + "strings" + + petname "github.com/dustinkirkland/golang-petname" + + "code.vikunja.io/veans/internal/auth" + "code.vikunja.io/veans/internal/client" + "code.vikunja.io/veans/internal/output" +) + +// resolveBotUser settles the bot identity for `veans init`: +// +// 1. If a bot we already own with this username exists, ask whether to +// reuse it. Reuse skips creation; the rest of init continues with +// the existing bot's ID. +// 2. If the username is taken by someone else, propose a petname-based +// alternative (e.g. "bot-clever-otter") and loop on rejection. +// 3. Otherwise, create the bot fresh. +// +// The flow is best-effort transparent: in non-interactive contexts +// (--bot-username collides with someone else's bot and no TTY), we +// surface a clear CONFLICT error pointing at --bot-username. +func resolveBotUser(ctx context.Context, c *client.Client, username, projectTitle string, p auth.Prompter, w io.Writer) (*client.BotUser, error) { + for { + // Step 1 + 2: see if anyone is using this name. + mine, err := c.FindMyBotByUsername(ctx, username) + if err != nil { + return nil, output.Wrap(output.CodeUnknown, err, "look up existing bots: %v", err) + } + if mine != nil { + ok, err := confirmReuse(p, w, mine.Username) + if err != nil { + return nil, err + } + if ok { + progress(w, "Reusing existing bot user %q (id=%d)", mine.Username, mine.ID) + return mine, nil + } + // User declined; fall through to prompt for a new name. + username, err = promptForReplacementName(p, w, username, false) + if err != nil { + return nil, err + } + continue + } + + // Step 3: try creating. + bot, err := c.CreateBotUser(ctx, username, "veans bot for "+projectTitle) + if err == nil { + progress(w, "Created bot user %q (id=%d)", bot.Username, bot.ID) + return bot, nil + } + + // On "username already exists" we know it's owned by someone + // other than us (we just checked FindMyBotByUsername). Anything + // else is a real failure — surface it. + var oe *output.Error + if !errors.As(err, &oe) || !isUsernameTakenErr(oe) { + return nil, err + } + username, err = promptForReplacementName(p, w, username, true) + if err != nil { + return nil, err + } + } +} + +// confirmReuse asks whether to reuse a bot user this caller already owns. +// Default is yes — re-running init in a worktree that's already onboarded +// is the common path. +func confirmReuse(p auth.Prompter, w io.Writer, username string) (bool, error) { + fmt.Fprintf(w, "Bot user %q already exists and is owned by you.\n", username) + ans, err := p.ReadLine("Reuse it? [Y/n]: ") + if err != nil { + return false, output.Wrap(output.CodeUnknown, err, "read confirmation: %v", err) + } + switch strings.ToLower(strings.TrimSpace(ans)) { + case "", "y", "yes": + return true, nil + } + return false, nil +} + +// promptForReplacementName asks for an alternate bot username, suggesting +// a petname-based default. ownedByOther=true means the previous name +// collided with someone else's bot; we phrase the prompt accordingly. +func promptForReplacementName(p auth.Prompter, w io.Writer, previous string, ownedByOther bool) (string, error) { + suggested := suggestPetname() + if ownedByOther { + fmt.Fprintf(w, "Bot username %q is taken by another user.\n", previous) + } else { + fmt.Fprintln(w, "Pick a different bot username.") + } + fmt.Fprintf(w, "Suggestion: %s\n", suggested) + ans, err := p.ReadLine(fmt.Sprintf("New bot username [%s]: ", suggested)) + if err != nil { + return "", output.Wrap(output.CodeUnknown, err, "read username: %v", err) + } + name := strings.TrimSpace(ans) + if name == "" { + name = suggested + } + if !strings.HasPrefix(name, "bot-") { + name = "bot-" + name + } + if name == previous { + return "", output.New(output.CodeValidation, "new bot username must differ from %q", previous) + } + return name, nil +} + +// suggestPetname proposes a memorable bot- name like "bot-clever-otter". +// Two words keeps the username short enough for Vikunja's 250-char limit +// while still giving plenty of namespace. +func suggestPetname() string { + return "bot-" + petname.Generate(2, "-") +} + +// isUsernameTakenErr returns true when the wrapped HTTP error from +// CreateBotUser indicates a username collision. Vikunja replies 400 with +// the canonical "user with this username already exists" message. +func isUsernameTakenErr(e *output.Error) bool { + if e == nil { + return false + } + if e.Code != output.CodeValidation { + return false + } + msg := strings.ToLower(e.Message) + return strings.Contains(msg, "username already exists") || + strings.Contains(msg, "user with this username") +} diff --git a/veans/internal/bootstrap/botuser_test.go b/veans/internal/bootstrap/botuser_test.go new file mode 100644 index 000000000..78abbd709 --- /dev/null +++ b/veans/internal/bootstrap/botuser_test.go @@ -0,0 +1,139 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package bootstrap + +import ( + "bytes" + "strings" + "testing" + + "code.vikunja.io/veans/internal/output" +) + +func TestSuggestPetname_ShapeAndPrefix(t *testing.T) { + got := suggestPetname() + if !strings.HasPrefix(got, "bot-") { + t.Fatalf("petname missing bot- prefix: %q", got) + } + // Two adjective-animal words separated by hyphens means at least + // three hyphen-delimited segments (bot, word1, word2). + if parts := strings.Split(got, "-"); len(parts) < 3 { + t.Fatalf("petname looks malformed: %q", got) + } +} + +func TestIsUsernameTakenErr(t *testing.T) { + cases := []struct { + err *output.Error + want bool + }{ + {nil, false}, + {output.New(output.CodeNotFound, "Not found"), false}, + {output.New(output.CodeValidation, "Some other validation issue"), false}, + {output.New(output.CodeValidation, "A user with this username already exists."), true}, + {output.New(output.CodeValidation, "username already exists"), true}, + {output.New(output.CodeValidation, "USERNAME ALREADY EXISTS"), true}, // case-insensitive + } + for _, c := range cases { + if got := isUsernameTakenErr(c.err); got != c.want { + msg := "" + if c.err != nil { + msg = c.err.Message + } + t.Errorf("isUsernameTakenErr(%q) = %v, want %v", msg, got, c.want) + } + } +} + +// scriptedPrompter returns canned answers to ReadLine, in order. +type scriptedPrompter struct { + answers []string + pos int +} + +func (s *scriptedPrompter) ReadLine(_ string) (string, error) { + if s.pos >= len(s.answers) { + return "", nil + } + a := s.answers[s.pos] + s.pos++ + return a, nil +} +func (s *scriptedPrompter) ReadPassword(_ string) (string, error) { return "", nil } + +func TestConfirmReuse(t *testing.T) { + yes := []string{"", "y", "Y", "yes", "Yes", "YES", " yes "} + no := []string{"n", "no", "N", "nope", "anything else"} + var buf bytes.Buffer + + for _, ans := range yes { + p := &scriptedPrompter{answers: []string{ans}} + ok, err := confirmReuse(p, &buf, "bot-x") + if err != nil { + t.Fatal(err) + } + if !ok { + t.Errorf("answer %q should be treated as yes", ans) + } + } + for _, ans := range no { + p := &scriptedPrompter{answers: []string{ans}} + ok, err := confirmReuse(p, &buf, "bot-x") + if err != nil { + t.Fatal(err) + } + if ok { + t.Errorf("answer %q should be treated as no", ans) + } + } +} + +func TestPromptForReplacementName_AcceptsDefault(t *testing.T) { + var buf bytes.Buffer + p := &scriptedPrompter{answers: []string{""}} // accept default + name, err := promptForReplacementName(p, &buf, "bot-old", true) + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(name, "bot-") { + t.Errorf("default name missing bot- prefix: %q", name) + } + if name == "bot-old" { + t.Errorf("default name shouldn't equal previous") + } +} + +func TestPromptForReplacementName_AddsPrefix(t *testing.T) { + var buf bytes.Buffer + p := &scriptedPrompter{answers: []string{"my-choice"}} + name, err := promptForReplacementName(p, &buf, "bot-old", true) + if err != nil { + t.Fatal(err) + } + if name != "bot-my-choice" { + t.Errorf("got %q, want bot-my-choice", name) + } +} + +func TestPromptForReplacementName_RejectsSameAsPrevious(t *testing.T) { + var buf bytes.Buffer + p := &scriptedPrompter{answers: []string{"bot-old"}} + _, err := promptForReplacementName(p, &buf, "bot-old", true) + if err == nil { + t.Fatal("expected error when new name equals previous") + } +} diff --git a/veans/internal/client/users.go b/veans/internal/client/users.go index 95e32ea0d..68b6a6ab3 100644 --- a/veans/internal/client/users.go +++ b/veans/internal/client/users.go @@ -55,6 +55,23 @@ func (c *Client) ListBotUsers(ctx context.Context) ([]*BotUser, error) { return out, nil } +// FindMyBotByUsername scans the caller's owned bots for one with the given +// username and returns it, or nil if no match. Useful for distinguishing +// "name is taken by someone else" from "name is taken by me" before +// attempting creation. +func (c *Client) FindMyBotByUsername(ctx context.Context, username string) (*BotUser, error) { + bots, err := c.ListBotUsers(ctx) + if err != nil { + return nil, err + } + for _, b := range bots { + if b != nil && b.Username == username { + return b, nil + } + } + return nil, nil +} + // statusCheck pulls the HTTP status off an error for callers that need to // distinguish 404-on-/bots from other failures. Currently unused outside this // file, but kept for symmetry.