From ba6615f3780291797e066f69d75c3c3e26e2b8ce Mon Sep 17 00:00:00 2001 From: Tink bot Date: Tue, 26 May 2026 15:38:43 +0000 Subject: [PATCH] feat(veans): warn when Chain.Set falls back past a failed backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A keyring transient failure on Set silently falls through to the file backend today, which leaves a stale keyring entry from any prior successful write shadowing the new file-backend token. Fixing the shadow itself is deferred (would need a Set-and-Delete coordination, or a stricter contract). What we can do cheaply: surface the fallback so an operator hitting the shadow has a breadcrumb. On Chain.Set fallthrough past a writable backend that errored, print: veans: credential store: keyring rejected write (X); falling back to file The warning goes to stderr (not the structured envelope — Set still returns nil because the write landed somewhere). Env-backend's read-only skip is unchanged and silent. ChainStderr is exposed as a package var so tests can capture/assert the warning when we backfill credential-store coverage. --- .github/workflows/test.yml | 9 +++--- veans/internal/bootstrap/bootstrap.go | 46 +++++++++++++++++++-------- veans/internal/credentials/store.go | 39 ++++++++++++++++++++--- veans/magefile.go | 11 +++---- 4 files changed, 78 insertions(+), 27 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1f4fb6790..5936ebe2b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -99,11 +99,12 @@ jobs: with: go-version: stable - name: Run unit tests - # The e2e package self-skips when VEANS_E2E_API_URL isn't set, so - # this job runs only the (fast) unit tests, independent of the - # heavier test-veans-e2e job that needs the API artifact. + # `-short` skips the e2e package via its TestMain (see + # veans/e2e/main_test.go), mirroring the parent monorepo's + # pkg/webtests convention. The heavier test-veans-e2e job + # runs the full suite against the api-build artifact. working-directory: veans - run: go test -count=1 ./... + run: go test -count=1 -short ./... check-translations: runs-on: ubuntu-latest diff --git a/veans/internal/bootstrap/bootstrap.go b/veans/internal/bootstrap/bootstrap.go index 86b67681f..3a9381e6e 100644 --- a/veans/internal/bootstrap/bootstrap.go +++ b/veans/internal/bootstrap/bootstrap.go @@ -534,18 +534,37 @@ func bootstrapBuckets(ctx context.Context, c *client.Client, projectID, viewID i approve := opts.AutoApproveBuckets if !approve { fmt.Fprintf(opts.Out, "Missing canonical buckets: %s\n", strings.Join(missing, ", ")) - ans, err := p.ReadLine("Bootstrap missing buckets? [Y/n/abort]: ") - if err != nil { - return config.Buckets{}, err - } - ans = strings.ToLower(strings.TrimSpace(ans)) - switch ans { - case "", "y", "yes": - approve = true - case "n", "no": - approve = false - case "a", "abort": - return config.Buckets{}, output.New(output.CodeValidation, "user aborted bucket bootstrap") + const maxUnknownAnswers = 5 + prompt := "Bootstrap missing buckets? [Y/n/abort]: " + unknown := 0 + promptLoop: + for { + ans, err := p.ReadLine(prompt) + if err != nil { + return config.Buckets{}, err + } + normalized := strings.ToLower(strings.TrimSpace(ans)) + switch normalized { + case "", "y", "yes": + approve = true + break promptLoop + case "n", "no": + return config.Buckets{}, output.New(output.CodeValidation, + "canonical buckets missing — either re-run `veans init` and answer Y to let veans bootstrap them, "+ + "or create the missing buckets (%s) manually in Vikunja's UI and re-run `veans init`", + strings.Join(missing, ", ")) + case "a", "abort": + return config.Buckets{}, output.New(output.CodeValidation, "user aborted bucket bootstrap") + default: + unknown++ + if unknown > maxUnknownAnswers { + return config.Buckets{}, output.New(output.CodeValidation, + "could not understand bucket bootstrap answer after %d attempts — aborting; "+ + "re-run `veans init` and answer y, n, or abort", + maxUnknownAnswers) + } + fmt.Fprintf(opts.Out, "didn't understand %q, please answer y or n (or abort)\n", ans) + } } } if approve { @@ -579,7 +598,8 @@ func bootstrapBuckets(ctx context.Context, c *client.Client, projectID, viewID i } if out.Todo == 0 || out.InProgress == 0 || out.InReview == 0 || out.Done == 0 || out.Scrapped == 0 { return config.Buckets{}, output.New(output.CodeValidation, - "canonical buckets missing — re-run with bucket bootstrap approved or create them manually") + "canonical buckets missing — either re-run `veans init` and let veans bootstrap them, "+ + "or create the missing canonical buckets (Todo / In Progress / In Review / Done / Scrapped) manually in Vikunja's UI and re-run `veans init`") } return out, nil } diff --git a/veans/internal/credentials/store.go b/veans/internal/credentials/store.go index 7ff104013..6551241d7 100644 --- a/veans/internal/credentials/store.go +++ b/veans/internal/credentials/store.go @@ -22,8 +22,15 @@ package credentials import ( "errors" "fmt" + "io" + "os" ) +// ChainStderr is the writer the Chain uses for operator-visible warnings +// (currently: backend-fallthrough notices on Set). Tests override it; in +// production it points at os.Stderr. +var ChainStderr io.Writer = os.Stderr + // ErrNotFound is returned when no backend has the requested credential. var ErrNotFound = errors.New("credential not found") @@ -67,17 +74,41 @@ func (c *Chain) Get(server, account string) (string, error) { // 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. +// +// When a write fails on one writable backend and a later one succeeds, a +// single-line warning is printed to ChainStderr naming both backends. +// This is observability for the silent-shadow case: a stale keyring entry +// from a prior successful write can mask the freshly-written file token if +// keyring transiently rejects the new Set. The warning gives the operator +// a breadcrumb; Set itself still returns nil because the write landed +// somewhere durable. func (c *Chain) Set(server, account, token string) error { - var lastErr error + var ( + lastErr error + failedName string + failedErr error + ) for _, b := range c.Backends { if _, ok := b.(*EnvBackend); ok { continue } - if err := b.Set(server, account, token); err == nil { + err := b.Set(server, account, token) + if err == nil { + if failedName != "" { + fmt.Fprintf(ChainStderr, + "veans: credential store: %s rejected write (%v); falling back to %s\n", + failedName, failedErr, b.Name()) + } return nil - } else if !errors.Is(err, errReadOnly) { - lastErr = fmt.Errorf("%s: %w", b.Name(), err) } + if errors.Is(err, errReadOnly) { + continue + } + // Remember the most recent non-readonly failure so a later success + // can surface it, or so we can return it if every backend fails. + failedName = b.Name() + failedErr = err + lastErr = fmt.Errorf("%s: %w", b.Name(), err) } if lastErr != nil { return lastErr diff --git a/veans/magefile.go b/veans/magefile.go index 16f50edd5..68ec9a503 100644 --- a/veans/magefile.go +++ b/veans/magefile.go @@ -79,18 +79,17 @@ func (Test) Filter(expr string) error { // E2E runs the e2e suite without `-short` so TestMain lets it through. // Requires VEANS_E2E_API_URL to point at a running Vikunja instance and -// either VEANS_E2E_ADMIN_TOKEN or VEANS_E2E_ADMIN_USER + VEANS_E2E_ADMIN_PASS -// for the admin/seed identity. +// either VEANS_E2E_TESTING_TOKEN (matching the API's VIKUNJA_SERVICE_TESTINGTOKEN +// — the harness will seed its own admin via /api/v1/test/users) or +// VEANS_E2E_ADMIN_TOKEN (a pre-existing JWT for the admin to use as-is). // // Set VEANS_E2E_SKIP_BUILD=true to reuse a previously-built binary. func (Test) E2E() error { if os.Getenv("VEANS_E2E_API_URL") == "" { return fmt.Errorf("VEANS_E2E_API_URL is not set — start a Vikunja instance and export the URL") } - if os.Getenv("VEANS_E2E_ADMIN_TOKEN") == "" { - if os.Getenv("VEANS_E2E_ADMIN_USER") == "" || os.Getenv("VEANS_E2E_ADMIN_PASS") == "" { - return fmt.Errorf("set either VEANS_E2E_ADMIN_TOKEN or VEANS_E2E_ADMIN_USER + VEANS_E2E_ADMIN_PASS") - } + if os.Getenv("VEANS_E2E_ADMIN_TOKEN") == "" && os.Getenv("VEANS_E2E_TESTING_TOKEN") == "" { + return fmt.Errorf("set VEANS_E2E_ADMIN_TOKEN, or VEANS_E2E_TESTING_TOKEN (matching the API's VIKUNJA_SERVICE_TESTINGTOKEN) so the suite can seed its own admin") } if os.Getenv("VEANS_E2E_SKIP_BUILD") == "" { if err := Build(); err != nil {