|
6 | 6 | "os" |
7 | 7 | "os/exec" |
8 | 8 | "path/filepath" |
| 9 | + "strings" |
9 | 10 | "testing" |
10 | 11 |
|
11 | 12 | "github.com/nemirovsky/sluice/internal/mcp" |
@@ -374,6 +375,56 @@ func TestHandleMCPAddInvalidName(t *testing.T) { |
374 | 375 | } |
375 | 376 | } |
376 | 377 |
|
| 378 | +// TestHandleMCPAddInvalidTimeout verifies that --timeout <= 0 is rejected |
| 379 | +// rather than being silently persisted. Previously the CLI accepted any |
| 380 | +// integer (including 0 and negatives), but the runtime constructors fall |
| 381 | +// back to DefaultTimeoutSec when TimeoutSec <= 0, so the persisted store |
| 382 | +// value diverged from the actual runtime behavior. Telegram /mcp add has |
| 383 | +// always rejected these values, this test locks in symmetry. |
| 384 | +func TestHandleMCPAddInvalidTimeout(t *testing.T) { |
| 385 | + cases := []struct { |
| 386 | + name string |
| 387 | + timeout string |
| 388 | + }{ |
| 389 | + {"zero", "0"}, |
| 390 | + {"negative", "-5"}, |
| 391 | + } |
| 392 | + |
| 393 | + for _, tc := range cases { |
| 394 | + t.Run(tc.name, func(t *testing.T) { |
| 395 | + dir := t.TempDir() |
| 396 | + dbPath := filepath.Join(dir, "test.db") |
| 397 | + |
| 398 | + err := handleMCPAdd([]string{ |
| 399 | + "--db", dbPath, |
| 400 | + "--command", "server", |
| 401 | + "--timeout", tc.timeout, |
| 402 | + "ups", |
| 403 | + }) |
| 404 | + if err == nil { |
| 405 | + t.Fatalf("expected error for --timeout %s", tc.timeout) |
| 406 | + } |
| 407 | + if !strings.Contains(err.Error(), "must be a positive integer") { |
| 408 | + t.Errorf("error %q should mention 'must be a positive integer'", err) |
| 409 | + } |
| 410 | + |
| 411 | + // Nothing should be persisted when validation fails. |
| 412 | + db, err := store.New(dbPath) |
| 413 | + if err != nil { |
| 414 | + t.Fatal(err) |
| 415 | + } |
| 416 | + defer func() { _ = db.Close() }() |
| 417 | + upstreams, err := db.ListMCPUpstreams() |
| 418 | + if err != nil { |
| 419 | + t.Fatal(err) |
| 420 | + } |
| 421 | + if len(upstreams) != 0 { |
| 422 | + t.Errorf("expected 0 upstreams after invalid --timeout, got %d", len(upstreams)) |
| 423 | + } |
| 424 | + }) |
| 425 | + } |
| 426 | +} |
| 427 | + |
377 | 428 | // TestHandleMCPRemove verifies removing an upstream by name. |
378 | 429 | func TestHandleMCPRemove(t *testing.T) { |
379 | 430 | dir := t.TempDir() |
@@ -648,18 +699,41 @@ func TestHandleMCPGatewayInvalidChatID(t *testing.T) { |
648 | 699 | } |
649 | 700 |
|
650 | 701 | // TestHandleMCPAddEnvInvalidFormat verifies that bad env format is rejected. |
| 702 | +// Covers both missing "=" (BADFORMAT) and empty-key (=VALUE) cases. The CLI |
| 703 | +// rejects empty keys to match the Telegram handler's behavior so both entry |
| 704 | +// points reject the same malformed inputs. |
651 | 705 | func TestHandleMCPAddEnvInvalidFormat(t *testing.T) { |
652 | 706 | dir := t.TempDir() |
653 | | - dbPath := filepath.Join(dir, "test.db") |
654 | 707 |
|
655 | | - err := handleMCPAdd([]string{ |
656 | | - "--db", dbPath, |
657 | | - "--command", "server", |
658 | | - "--env", "BADFORMAT", |
659 | | - "myserver", |
660 | | - }) |
661 | | - if err == nil { |
662 | | - t.Fatal("expected error for invalid env format") |
| 708 | + cases := []struct { |
| 709 | + label string |
| 710 | + flag string |
| 711 | + value string |
| 712 | + }{ |
| 713 | + {"env missing equals", "--env", "BADFORMAT"}, |
| 714 | + {"env empty key", "--env", "=VALUE"}, |
| 715 | + {"header missing equals", "--header", "BADFORMAT"}, |
| 716 | + {"header empty key", "--header", "=VALUE"}, |
| 717 | + } |
| 718 | + |
| 719 | + for _, tc := range cases { |
| 720 | + t.Run(tc.label, func(t *testing.T) { |
| 721 | + dbPath := filepath.Join(dir, tc.label+".db") |
| 722 | + args := []string{ |
| 723 | + "--db", dbPath, |
| 724 | + "--command", "https://example.com/mcp", |
| 725 | + "--transport", "http", |
| 726 | + tc.flag, tc.value, |
| 727 | + "myserver", |
| 728 | + } |
| 729 | + // The "http" transport is set so that --header is accepted for |
| 730 | + // the header cases. The env cases do not require it, but sharing |
| 731 | + // one arg set keeps the table loop uniform. |
| 732 | + err := handleMCPAdd(args) |
| 733 | + if err == nil { |
| 734 | + t.Fatalf("expected error for %s %q", tc.flag, tc.value) |
| 735 | + } |
| 736 | + }) |
663 | 737 | } |
664 | 738 | } |
665 | 739 |
|
|
0 commit comments