diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d1b403..5f85b73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,39 @@ # Changelog +## 0.5.2 — 2026-05-14 + +### Fixed + +- `vk create` now cleans up the temporary vectoria knowledgebase it + created when the pipeline fails before the backend task takes + ownership (`InitTask` rejection: `insufficient_credits`, + `script_invalid`, network errors, or any earlier upload/poll + failure). Previously every failed `vk create` invocation left an + orphan kb in vectoria forever — testing during 0.5.0 validation + alone accumulated 6+ such orphans, and the user's tenant had + accumulated 424 kbs total. The cleanup is best-effort: errors are + swallowed so they don't mask the real failure the user is about to + see. Once `InitTask` succeeds, the backend task owns the kb's + lifecycle and the CLI no longer interferes. + +### New + +- `vectoria.Client.DeleteKB(ctx, kbID)` — exposes the existing + vectoria `DELETE /v1/knowledgebases/{id}` endpoint, used by the + cleanup above. Available to external callers of the client. + +## 0.5.1 — 2026-05-14 + +### Fixed + +- `vk create` now exits **5** (business failure) when the backend rejects + `POST /v1/tasks/init` with `insufficient_credits` (envelope code 100001), + matching the stream-side path's existing behavior. Previously this case + exited 1 (cobra's generic error code), inconsistent with the documented + exit-code contract and with the same condition surfacing later in the + pipeline. Caught while running real-backend smoke tests during 0.5.0 + validation. + ## 0.5.0 — 2026-05-14 ### New diff --git a/client/vectoria/knowledgebase.go b/client/vectoria/knowledgebase.go index f68ab32..671596b 100644 --- a/client/vectoria/knowledgebase.go +++ b/client/vectoria/knowledgebase.go @@ -56,3 +56,14 @@ func (c *Client) DeleteDoc(ctx context.Context, kbID, docID string) error { } return nil } + +// DeleteKB removes a knowledgebase and all its documents. +// Used by `vk create` to clean up orphan kbs when upload-then-pipeline fails +// before the backend task takes ownership of the kb. +func (c *Client) DeleteKB(ctx context.Context, kbID string) error { + path := fmt.Sprintf("/v1/knowledgebases/%s", kbID) + if err := c.http.Do(ctx, "DELETE", path, nil, nil); err != nil { + return fmt.Errorf("delete knowledgebase: %w", err) + } + return nil +} diff --git a/client/vectoria/knowledgebase_test.go b/client/vectoria/knowledgebase_test.go index 94b4860..f6bf7d0 100644 --- a/client/vectoria/knowledgebase_test.go +++ b/client/vectoria/knowledgebase_test.go @@ -128,6 +128,26 @@ func TestDeleteDoc(t *testing.T) { } } +func TestDeleteKB(t *testing.T) { + var gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + if r.Method != "DELETE" { + t.Fatalf("unexpected method %s", r.Method) + } + w.WriteHeader(204) + })) + defer srv.Close() + + c := vectoria.New(srv.URL, staticToken("test-jwt")) + if err := c.DeleteKB(context.Background(), "kb_1"); err != nil { + t.Fatalf("DeleteKB: %v", err) + } + if gotPath != "/v1/knowledgebases/kb_1" { + t.Fatalf("path = %q, want /v1/knowledgebases/kb_1", gotPath) + } +} + func TestUploadDoc_FileContent(t *testing.T) { var gotContent string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/cmd/create.go b/cmd/create.go index 340a46c..0ea6723 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -101,6 +101,17 @@ var createCmd = &cobra.Command{ return clerr.Validation(i18n.T("create.err.script_needs_doc")) } + // Orphan kb cleanup: drop the kb if InitTask never returns OK + // (task stays nil). os.Exit(5) below skips defers, so an inline + // call is also needed there. + var task *figlens.Task + defer func() { + if kbID == "" || task != nil { + return + } + cleanupOrphanKB(kbID) + }() + // Step 2: optimize prompt (skip if user provided --prompt). _, url, tp, err := cmdutil.Default().Service("figlens") if err != nil { @@ -146,10 +157,16 @@ var createCmd = &cobra.Command{ initParams.KnowledgeID = kbID initParams.DocID = docID } - task, err := fc.InitTask(ctx, initParams) + task, err = fc.InitTask(ctx, initParams) if err != nil { if errs.HasCode(err, "insufficient_credits") { - return fmt.Errorf("%s", i18n.T("credits.insufficient")) + // Mirror the stream-side path's exit code: business failure → 5. + // os.Exit skips defers — clean up the orphan kb inline first. + if kbID != "" { + cleanupOrphanKB(kbID) + } + fmt.Fprintln(os.Stderr, i18n.T("credits.insufficient")) + os.Exit(5) } if errs.HasCode(err, "script_invalid") { // Backend's localized message already lives on the error. @@ -162,6 +179,9 @@ var createCmd = &cobra.Command{ } return err } + // Past this point, `task != nil` and the backend task owns the kb; + // the deferred cleanup above will skip on any later error path + // (task.failed, stream interrupted, --async detach). // Step 4: async or sync. if flagCreateAsync { @@ -388,6 +408,20 @@ func uploadFile(ctx context.Context, filePath string) (string, string, error) { if err != nil { return "", "", err } + // Best-effort cleanup if any subsequent step in this function fails. + // Cleared just before the successful return. + cleanup := func() { + // Fresh context with timeout: parent ctx may already be cancelled, + // and a hung backend must not hold the user hostage. + c, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _ = vc.DeleteKB(c, kbID) + } + defer func() { + if cleanup != nil { + cleanup() + } + }() f, err := os.Open(filePath) if err != nil { @@ -405,6 +439,7 @@ func uploadFile(ctx context.Context, filePath string) (string, string, error) { if err != nil { return "", "", err } + cleanup = nil // ownership transfers to caller from here return kbID, docID, nil } @@ -421,6 +456,18 @@ func uploadURL(ctx context.Context, url string) (string, string, error) { if err != nil { return "", "", err } + cleanup := func() { + // Fresh context with timeout: parent ctx may already be cancelled, + // and a hung backend must not hold the user hostage. + c, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _ = vc.DeleteKB(c, kbID) + } + defer func() { + if cleanup != nil { + cleanup() + } + }() fmt.Fprintln(os.Stderr, i18n.T("create.uploading_url", url)) doc, err := vc.UploadURL(ctx, kbID, url) @@ -432,9 +479,22 @@ func uploadURL(ctx context.Context, url string) (string, string, error) { if err != nil { return "", "", err } + cleanup = nil return kbID, docID, nil } +// cleanupOrphanKB best-effort deletes a kb the CLI created when the +// backend never claimed it. Errors are swallowed: hygiene, not correctness. +func cleanupOrphanKB(kbID string) { + vc, err := cliauth.NewVectoriaClient() + if err != nil { + return + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _ = vc.DeleteKB(ctx, kbID) +} + // pollDocReady polls until the document is completed or fails. func pollDocReady(ctx context.Context, vc *vectoria.Client, kbID, docID string) (string, error) { fmt.Fprintln(os.Stderr, i18n.T("create.doc_polling", docID)) diff --git a/package.json b/package.json index 4b9624b..987fc49 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "vibeknow-cli", - "version": "0.5.0", + "version": "0.5.2", "description": "VibeKnow CLI — turn docs / URLs into videos", "license": "MIT", "bin": { diff --git a/skills/vibeknow-core/SKILL.md b/skills/vibeknow-core/SKILL.md index 587f706..985a870 100644 --- a/skills/vibeknow-core/SKILL.md +++ b/skills/vibeknow-core/SKILL.md @@ -1,6 +1,6 @@ --- name: vibeknow-core -version: 0.5.0 +version: 0.5.2 description: "vibeknow CLI setup, authentication, profile management, and diagnostics. Use when: first-time setup, auth errors, switching environments, diagnosing connection issues." metadata: requires: diff --git a/skills/vibeknow-create/SKILL.md b/skills/vibeknow-create/SKILL.md index 5b1451d..b2d990c 100644 --- a/skills/vibeknow-create/SKILL.md +++ b/skills/vibeknow-create/SKILL.md @@ -1,6 +1,6 @@ --- name: vibeknow-create -version: 0.5.0 +version: 0.5.2 description: "Generate videos from documents/URLs/files, track video task progress, download results, list voice templates. Use when: user wants to create a video, check task status, download video, or browse voices." metadata: requires: diff --git a/skills/vibeknow-doc/SKILL.md b/skills/vibeknow-doc/SKILL.md index b8ee7e3..c65d915 100644 --- a/skills/vibeknow-doc/SKILL.md +++ b/skills/vibeknow-doc/SKILL.md @@ -1,6 +1,6 @@ --- name: vibeknow-doc -version: 0.5.0 +version: 0.5.2 description: "Upload documents to vectoria and check processing status. Use when: user wants to upload a document, check if a document is ready, or get a doc_id for use with vibeknow create." metadata: requires: diff --git a/tests/integration/create_credits_test.go b/tests/integration/create_credits_test.go new file mode 100644 index 0000000..c8af797 --- /dev/null +++ b/tests/integration/create_credits_test.go @@ -0,0 +1,123 @@ +package integration + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "strings" + "sync" + "testing" +) + +// TestCreate_InsufficientCreditsOnInit_Exits5 covers the bug fixed in 0.5.1: +// when the backend rejects InitTask with envelope code 100001 (insufficient +// credits), the CLI must exit 5 (business failure) to match the stream-side +// path's behavior, not exit 1 from cobra's default error handler. +// +// It also covers the orphan-kb cleanup fixed in 0.5.2: when InitTask fails +// after the CLI uploaded a doc and created a kb, the CLI must DELETE the +// just-created kb before exiting so it doesn't leak. +func TestCreate_InsufficientCreditsOnInit_Exits5(t *testing.T) { + if testing.Short() { + t.Skip("integration test") + } + + var mu sync.Mutex + var kbCreated, kbDeleted bool + var deletedID, createdID string + + const fixedKBID = "kb_insufcredits_test" + mux := http.NewServeMux() + // Exact match: vectoria CreateKB (POST /v1/knowledgebases). + mux.HandleFunc("/v1/knowledgebases", func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + kbCreated = true + createdID = fixedKBID + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{"id": createdID}) + }) + // Prefix match: doc upload (POST), doc status (GET), kb delete (DELETE). + mux.HandleFunc("/v1/knowledgebases/", func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case "POST": + _ = r.ParseMultipartForm(32 << 20) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{"id": "doc_x", "status": "completed"}) + case "GET": + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{"id": "doc_x", "status": "completed"}) + case "DELETE": + // Assert the full path to fail loudly if upload or poll ever + // accidentally routes here. + if r.URL.Path != "/v1/knowledgebases/"+fixedKBID { + t.Errorf("unexpected DELETE path %q", r.URL.Path) + } + mu.Lock() + kbDeleted = true + deletedID = strings.TrimPrefix(r.URL.Path, "/v1/knowledgebases/") + mu.Unlock() + w.WriteHeader(204) + } + }) + mux.HandleFunc("/v1/tasks/init", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusPaymentRequired) + _ = json.NewEncoder(w).Encode(map[string]any{ + "code": 100001, + "message": "insufficient credits", + }) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + // Write a tiny local file so the CLI takes the upload path (which creates a kb). + tmpFile := t.TempDir() + "/test.txt" + if err := os.WriteFile(tmpFile, []byte("hello world"), 0644); err != nil { + t.Fatalf("write tmp file: %v", err) + } + + bin := build(t) + configHome := buildProfile(t, map[string]string{ + "figlens": srv.URL, + "vectoria": srv.URL, + }) + + cmd := exec.Command(bin, "create", "--from", tmpFile) + var stdout, stderr strings.Builder + cmd.Stdout = &stdout + cmd.Stderr = &stderr + cmd.Env = append(os.Environ(), + "VIBEKNOW_TOKEN=fake-token", + "VIBEKNOW_CONFIG_HOME="+configHome, + ) + + err := cmd.Run() + code := 0 + if ee, ok := err.(*exec.ExitError); ok { + code = ee.ExitCode() + } else if err != nil { + t.Fatalf("run: %v\nstderr: %s", err, stderr.String()) + } + + if code != 5 { + t.Fatalf("exit code = %d, want 5 (business failure)\nstderr: %s", code, stderr.String()) + } + if !strings.Contains(stderr.String(), "insufficient credits") { + t.Fatalf("stderr missing insufficient-credits message:\n%s", stderr.String()) + } + + mu.Lock() + defer mu.Unlock() + if !kbCreated { + t.Fatalf("expected kb to be created by CLI but no POST /v1/knowledgebases received") + } + if !kbDeleted { + t.Fatalf("expected orphan kb cleanup: DELETE /v1/knowledgebases/ was never called.\nstderr:%s", stderr.String()) + } + if deletedID != createdID { + t.Fatalf("deleted kb = %q, want %q", deletedID, createdID) + } +} diff --git a/tests/integration/video_flow_test.go b/tests/integration/video_flow_test.go index bb5072f..638ff66 100644 --- a/tests/integration/video_flow_test.go +++ b/tests/integration/video_flow_test.go @@ -9,38 +9,54 @@ import ( "os" "os/exec" "path/filepath" + "sort" "strings" "sync/atomic" "testing" ) -// buildVideoProfile writes a minimal profiles.yaml that points only at figlens -// (no vectoria needed for pure video commands) and returns the config-home -// path to pass to the binary via VIBEKNOW_CONFIG_HOME. Using that var (rather -// than XDG_CONFIG_HOME) keeps the test cross-platform — on Windows the CLI -// resolves its config dir from %AppData% and ignores XDG_CONFIG_HOME entirely. -func buildVideoProfile(t *testing.T, figlensURL string) string { +// buildProfile writes a minimal profiles.yaml wiring the given endpoint +// name→URL pairs and returns the config-home path to pass to the binary +// via VIBEKNOW_CONFIG_HOME. Using that var (rather than XDG_CONFIG_HOME) +// keeps tests cross-platform — on Windows the CLI resolves its config +// dir from %AppData% and ignores XDG_CONFIG_HOME entirely. +func buildProfile(t *testing.T, endpoints map[string]string) string { t.Helper() configDir := filepath.Join(t.TempDir(), "vibeknow") if err := os.MkdirAll(configDir, 0755); err != nil { t.Fatalf("mkdir config: %v", err) } + // Sort keys for stable output (helps debugging). + names := make([]string, 0, len(endpoints)) + for k := range endpoints { + names = append(names, k) + } + sort.Strings(names) + var endpointsBlock strings.Builder + for _, name := range names { + fmt.Fprintf(&endpointsBlock, " %s: %s\n", name, endpoints[name]) + } profileYAML := fmt.Sprintf(`schema_version: "2" current: test profiles: - name: test endpoints: - figlens: %s - credential_ref: test +%s credential_ref: test trust: dev is_production: false -`, figlensURL) +`, endpointsBlock.String()) if err := os.WriteFile(filepath.Join(configDir, "profiles.yaml"), []byte(profileYAML), 0644); err != nil { t.Fatalf("write profiles.yaml: %v", err) } return configDir } +// buildVideoProfile is a convenience wrapper around buildProfile for tests +// that only need a figlens endpoint. Existing callers stay unchanged. +func buildVideoProfile(t *testing.T, figlensURL string) string { + return buildProfile(t, map[string]string{"figlens": figlensURL}) +} + // runVideoCmd runs the binary with the given args, capturing stdout and stderr // separately. Returns stdout, combined output, and exit code. func runVideoCmd(t *testing.T, bin, configHome string, args ...string) (string, string, int) {