feat(veans): reuse owned bot or prompt for fresh name on collision
This commit is contained in:
Tink bot
kolaente
parent
cd7cc113a1
commit
4ac89741e3
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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=
|
||||
|
|
|
|||
|
|
@ -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{
|
||||
|
|
|
|||
|
|
@ -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 <https://www.gnu.org/licenses/>.
|
||||
|
||||
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")
|
||||
}
|
||||
|
|
@ -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 <https://www.gnu.org/licenses/>.
|
||||
|
||||
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 := "<nil>"
|
||||
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")
|
||||
}
|
||||
}
|
||||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Reference in New Issue