rickub
rickub/clipublic Fork
⑂ 94ea6bb
Branches
feat/10-repo-rename-transfer feat/7-edit-title-and-body feat/8-run-logs-job-filter feat/staleness-notice main test/create-body-dash-stdin
Tags
v0.1.0 v0.1.0-rc.11 v0.1.0-rc.14 v0.1.0-rc.16 v0.1.0-rc.20 v0.1.0-rc.26 v0.1.0-rc.36 v0.1.0-rc.43 v0.1.0-rc.50 v0.1.0-rc.64 v0.2.0 View all branches → View all tags →
Commits
⬇ Clone ▾
git clone https://git.rickub.com/rickub/cli.git
git clone ssh://git@rickub.com/rickub/cli.git

Host key fingerprint (ed25519): SHA256:iycHnxEyq0Q7uyVpB7JlznP0G7JrTPXLYRcAU5CSLhc — verify it before your first connect.

Download ZIP

review: make the arg-count test real, and confirm before a transferUnverified

Two findings from the review of !3:

- `TestRepoMoveArgCount` came over from 4a5d5d2 asserting only that the command
  errored — with no token configured, `newClient()` failed first, so it passed
  whether or not the `ExactArgs(2)` guard existed. Verified: relaxing `rename` to
  `MinimumNArgs(1)` left it green, while in real use that regression panics on
  `args[1]`. It now runs against the stub server with a token, asserts the error
  actually says "accepts 2 arg(s)", and asserts nothing reached the server — and
  it does fail on that relaxed guard.
- `repo transfer` fired immediately, while `repo delete` right below it makes you
  type the repo name. A transfer is the stronger one-way door of the two: after
  the move the actor may hold no ADMIN in the destination, so transferring back
  answers `forbidden_owner`. It now confirms the same way, with `--yes` for
  scripts. `rename` deliberately stays unprompted — it breaks clone URLs, but the
  actor keeps ADMIN and can rename straight back, so there is no door to close.

The prompt is a third divergence from the ported commit, on top of `clean()` and
the bad-spec test; noted in the MR alongside the other two.

Verified live: a mistyped confirmation aborts and the repo stays put, the right
answer goes through, `--yes` skips the prompt.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KPPhRdTiUiiSvLjthotkT1
Olivier Girardot committed 2026-09-08T00:57:56+02:00 Browse files
94ea6bb parent: aada7be
modified README.md +3 -1
@@ -147,7 +147,9 @@ rickub repo create <name> [--org H] [--public|--private] [-d desc]
147147 rickub repo view <owner/repo>
148148 rickub repo edit <owner/repo> [--visibility public|private] [-d desc] [--default-branch B]
149149 rickub repo rename <owner/repo> <new-name> # needs ADMIN; breaks every existing clone URL
150-rickub repo transfer <owner/repo> <new-owner> # needs ADMIN here and create rights there
150+rickub repo transfer <owner/repo> <new-owner> [--yes]
151+ # needs ADMIN here and create rights there;
152+ # confirms first — you may not be able to move it back
151153 rickub repo delete <owner/repo> [--yes]
152154 rickub repo clone <owner/repo> [dir] [--protocol ssh|https] [-- git-args…]
153155 rickub repo files <owner/repo> [path] [--ref R]
@@ -147,7 +147,9 @@ rickub repo create <name> [--org H] [--public|--private] [-d desc]
147 rickub repo view <owner/repo>147 rickub repo view <owner/repo>
148 rickub repo edit <owner/repo> [--visibility public|private] [-d desc] [--default-branch B]148 rickub repo edit <owner/repo> [--visibility public|private] [-d desc] [--default-branch B]
149 rickub repo rename <owner/repo> <new-name> # needs ADMIN; breaks every existing clone URL149 rickub repo rename <owner/repo> <new-name> # needs ADMIN; breaks every existing clone URL
150-rickub repo transfer <owner/repo> <new-owner> # needs ADMIN here and create rights there150+rickub repo transfer <owner/repo> <new-owner> [--yes]
151+ # needs ADMIN here and create rights there;
152+ # confirms first — you may not be able to move it back
151 rickub repo delete <owner/repo> [--yes]153 rickub repo delete <owner/repo> [--yes]
152 rickub repo clone <owner/repo> [dir] [--protocol ssh|https] [-- git-args…]154 rickub repo clone <owner/repo> [dir] [--protocol ssh|https] [-- git-args…]
153 rickub repo files <owner/repo> [path] [--ref R]155 rickub repo files <owner/repo> [path] [--ref R]
modified cmd/repo.go +18 -1
@@ -27,6 +27,7 @@ var (
2727 repoEditDesc string
2828 repoEditBranch string
2929 repoDeleteYes bool
30+ repoTransferYes bool
3031 repoContentsRef string
3132 pageFlag int
3233 perPageFlag int
@@ -106,10 +107,15 @@ name instead).`,
106107
107108 Requires ADMIN on the repository, a verified email address, and the right
108109 to create repositories under the destination. A private repository counts
109-against the destination's plan's private-repo limit.`,
110+against the destination's plan's private-repo limit.
111+
112+You are asked to confirm first: a transfer is not reversible by the person
113+who made it, since you may hold no ADMIN in the destination namespace
114+afterwards. Pass --yes to skip the prompt in a script.`,
110115 Args: cobra.ExactArgs(2),
111116 RunE: runRepoTransfer,
112117 }
118+ transferCmd.Flags().BoolVar(&repoTransferYes, "yes", false, "skip the confirmation prompt")
113119
114120 // delete
115121 deleteCmd := &cobra.Command{
@@ -343,6 +349,17 @@ func runRepoTransfer(cmd *cobra.Command, args []string) error {
343349 if err != nil {
344350 return err
345351 }
352+ // A transfer is not reversible by the person who made it: once the repo is in
353+ // the destination namespace the actor may hold no ADMIN there, and transferring
354+ // it back answers forbidden_owner. That is a stronger one-way door than
355+ // `repo delete`, which does prompt — so this one prompts too, with the same
356+ // --yes escape for scripts.
357+ if !repoTransferYes {
358+ ans := prompt(cmd, fmt.Sprintf("Transfer %s/%s to %s? You may not be able to move it back. Type the repo name to confirm: ", owner, repo, args[1]))
359+ if ans != repo {
360+ return fmt.Errorf("confirmation did not match; aborted")
361+ }
362+ }
346363 r, err := client.TransferRepo(cmd.Context(), owner, repo, args[1])
347364 if err != nil {
348365 return err
@@ -27,6 +27,7 @@ var (
27 repoEditDesc string27 repoEditDesc string
28 repoEditBranch string28 repoEditBranch string
29 repoDeleteYes bool29 repoDeleteYes bool
30+ repoTransferYes bool
30 repoContentsRef string31 repoContentsRef string
31 pageFlag int32 pageFlag int
32 perPageFlag int33 perPageFlag int
@@ -106,10 +107,15 @@ name instead).`,
106 107
107 Requires ADMIN on the repository, a verified email address, and the right108 Requires ADMIN on the repository, a verified email address, and the right
108 to create repositories under the destination. A private repository counts109 to create repositories under the destination. A private repository counts
109-against the destination's plan's private-repo limit.`,110+against the destination's plan's private-repo limit.
111+
112+You are asked to confirm first: a transfer is not reversible by the person
113+who made it, since you may hold no ADMIN in the destination namespace
114+afterwards. Pass --yes to skip the prompt in a script.`,
110 Args: cobra.ExactArgs(2),115 Args: cobra.ExactArgs(2),
111 RunE: runRepoTransfer,116 RunE: runRepoTransfer,
112 }117 }
118+ transferCmd.Flags().BoolVar(&repoTransferYes, "yes", false, "skip the confirmation prompt")
113 119
114 // delete120 // delete
115 deleteCmd := &cobra.Command{121 deleteCmd := &cobra.Command{
@@ -343,6 +349,17 @@ func runRepoTransfer(cmd *cobra.Command, args []string) error {
343 if err != nil {349 if err != nil {
344 return err350 return err
345 }351 }
352+ // A transfer is not reversible by the person who made it: once the repo is in
353+ // the destination namespace the actor may hold no ADMIN there, and transferring
354+ // it back answers forbidden_owner. That is a stronger one-way door than
355+ // `repo delete`, which does prompt — so this one prompts too, with the same
356+ // --yes escape for scripts.
357+ if !repoTransferYes {
358+ ans := prompt(cmd, fmt.Sprintf("Transfer %s/%s to %s? You may not be able to move it back. Type the repo name to confirm: ", owner, repo, args[1]))
359+ if ans != repo {
360+ return fmt.Errorf("confirmation did not match; aborted")
361+ }
362+ }
346 r, err := client.TransferRepo(cmd.Context(), owner, repo, args[1])363 r, err := client.TransferRepo(cmd.Context(), owner, repo, args[1])
347 if err != nil {364 if err != nil {
348 return err365 return err
modified cmd/repo_test.go +96 -6
@@ -22,6 +22,7 @@ func execute(t *testing.T, args ...string) (string, string, error) {
2222 flagHost = ""
2323 flagToken = ""
2424 repoListUser = ""
25+ repoTransferYes = false
2526 repoListOrg = ""
2627 pageFlag = 0
2728 perPageFlag = 0
@@ -223,7 +224,9 @@ func TestRepoTransferCallsMoveEndpoint(t *testing.T) {
223224 defer srv.Close()
224225 t.Setenv("RICKUB_HOST", srv.URL)
225226
226- out, _, err := execute(t, "repo", "transfer", "rick/portal", "acme")
227+ // --yes: this test is about the wire call, not the confirmation, which
228+ // TestRepoTransferPromptsBeforeMoving covers.
229+ out, _, err := execute(t, "repo", "transfer", "rick/portal", "acme", "--yes")
227230 if err != nil {
228231 t.Fatalf("execute: %v", err)
229232 }
@@ -240,12 +243,99 @@ func TestRepoTransferCallsMoveEndpoint(t *testing.T) {
240243
241244 func TestRepoMoveArgCount(t *testing.T) {
242245 t.Setenv("XDG_CONFIG_HOME", t.TempDir())
243- // Both commands are argument-strict: no destination, no request.
244- if _, _, err := execute(t, "repo", "rename", "rick/portal"); err == nil {
245- t.Error("rename without a new name should fail arg parsing")
246+ t.Setenv("RICKUB_TOKEN", "rickub_pat_test")
247+ seen := &captured{}
248+ srv := repoMoveServer(t, seen)
249+ defer srv.Close()
250+ t.Setenv("RICKUB_HOST", srv.URL)
251+
252+ // Both commands are argument-strict: no destination, no request. The token and
253+ // host are configured on purpose — without them `newClient()` would fail first
254+ // and this would pass even with the ExactArgs guard removed, which is what the
255+ // version ported from 4a5d5d2 did.
256+ for _, args := range [][]string{
257+ {"repo", "rename", "rick/portal"},
258+ {"repo", "transfer", "rick/portal"},
259+ } {
260+ _, _, err := execute(t, args...)
261+ if err == nil {
262+ t.Errorf("%v: a missing destination should fail arg parsing", args)
263+ continue
264+ }
265+ if !strings.Contains(err.Error(), "accepts 2 arg(s)") {
266+ t.Errorf("%v: want an arg-count error, got %v", args, err)
267+ }
268+ if seen.Path != "" {
269+ t.Errorf("%v: reached the server at %q", args, seen.Path)
270+ }
271+ }
272+}
273+
274+// TestRepoTransferPromptsBeforeMoving: a transfer is a one-way door for the person
275+// making it — after the move the actor may hold no ADMIN in the destination, so
276+// transferring back answers forbidden_owner. It confirms like `repo delete` does.
277+func TestRepoTransferPromptsBeforeMoving(t *testing.T) {
278+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
279+ t.Setenv("RICKUB_TOKEN", "rickub_pat_test")
280+ seen := &captured{}
281+ srv := repoMoveServer(t, seen)
282+ defer srv.Close()
283+ t.Setenv("RICKUB_HOST", srv.URL)
284+
285+ // A wrong answer aborts, and nothing is sent.
286+ rootCmd.SetIn(strings.NewReader("nope\n"))
287+ _, _, err := execute(t, "repo", "transfer", "rick/portal", "acme")
288+ rootCmd.SetIn(nil)
289+ if err == nil {
290+ t.Fatal("a mistyped confirmation was accepted")
291+ }
292+ if seen.Path != "" {
293+ t.Fatalf("an unconfirmed transfer reached the server at %q", seen.Path)
294+ }
295+
296+ // Typing the repo name goes through.
297+ rootCmd.SetIn(strings.NewReader("portal\n"))
298+ out, _, err := execute(t, "repo", "transfer", "rick/portal", "acme")
299+ rootCmd.SetIn(nil)
300+ if err != nil {
301+ t.Fatalf("execute: %v", err)
302+ }
303+ if seen.Path != "/api/v1/repos/rick/portal/transfer" {
304+ t.Errorf("confirmed transfer did not reach the endpoint: %q", seen.Path)
305+ }
306+ if !strings.Contains(out, "Transferred to acme/portal") {
307+ t.Errorf("output missing new full name:\n%s", out)
308+ }
309+
310+ // --yes skips the prompt: scripts keep working without a tty.
311+ seen.Path = ""
312+ if _, _, err := execute(t, "repo", "transfer", "rick/portal", "acme", "--yes"); err != nil {
313+ t.Fatalf("--yes: %v", err)
314+ }
315+ if seen.Path != "/api/v1/repos/rick/portal/transfer" {
316+ t.Errorf("--yes did not reach the endpoint: %q", seen.Path)
317+ }
318+}
319+
320+// TestRepoRenameDoesNotPrompt: rename stays unprompted. It breaks clone URLs, but
321+// the actor keeps ADMIN and can rename straight back — a confirmation on every
322+// rename would be friction without a one-way door behind it.
323+func TestRepoRenameDoesNotPrompt(t *testing.T) {
324+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
325+ t.Setenv("RICKUB_TOKEN", "rickub_pat_test")
326+ seen := &captured{}
327+ srv := repoMoveServer(t, seen)
328+ defer srv.Close()
329+ t.Setenv("RICKUB_HOST", srv.URL)
330+
331+ rootCmd.SetIn(strings.NewReader(""))
332+ _, _, err := execute(t, "repo", "rename", "rick/portal", "portal-gun")
333+ rootCmd.SetIn(nil)
334+ if err != nil {
335+ t.Fatalf("execute: %v", err)
246336 }
247- if _, _, err := execute(t, "repo", "transfer", "rick/portal"); err == nil {
248- t.Error("transfer without a destination owner should fail arg parsing")
337+ if seen.Path != "/api/v1/repos/rick/portal/rename" {
338+ t.Errorf("rename should not need a confirmation: %q", seen.Path)
249339 }
250340 }
251341
@@ -22,6 +22,7 @@ func execute(t *testing.T, args ...string) (string, string, error) {
22 flagHost = ""22 flagHost = ""
23 flagToken = ""23 flagToken = ""
24 repoListUser = ""24 repoListUser = ""
25+ repoTransferYes = false
25 repoListOrg = ""26 repoListOrg = ""
26 pageFlag = 027 pageFlag = 0
27 perPageFlag = 028 perPageFlag = 0
@@ -223,7 +224,9 @@ func TestRepoTransferCallsMoveEndpoint(t *testing.T) {
223 defer srv.Close()224 defer srv.Close()
224 t.Setenv("RICKUB_HOST", srv.URL)225 t.Setenv("RICKUB_HOST", srv.URL)
225 226
226- out, _, err := execute(t, "repo", "transfer", "rick/portal", "acme")227+ // --yes: this test is about the wire call, not the confirmation, which
228+ // TestRepoTransferPromptsBeforeMoving covers.
229+ out, _, err := execute(t, "repo", "transfer", "rick/portal", "acme", "--yes")
227 if err != nil {230 if err != nil {
228 t.Fatalf("execute: %v", err)231 t.Fatalf("execute: %v", err)
229 }232 }
@@ -240,12 +243,99 @@ func TestRepoTransferCallsMoveEndpoint(t *testing.T) {
240 243
241 func TestRepoMoveArgCount(t *testing.T) {244 func TestRepoMoveArgCount(t *testing.T) {
242 t.Setenv("XDG_CONFIG_HOME", t.TempDir())245 t.Setenv("XDG_CONFIG_HOME", t.TempDir())
243- // Both commands are argument-strict: no destination, no request.246+ t.Setenv("RICKUB_TOKEN", "rickub_pat_test")
244- if _, _, err := execute(t, "repo", "rename", "rick/portal"); err == nil {247+ seen := &captured{}
245- t.Error("rename without a new name should fail arg parsing")248+ srv := repoMoveServer(t, seen)
249+ defer srv.Close()
250+ t.Setenv("RICKUB_HOST", srv.URL)
251+
252+ // Both commands are argument-strict: no destination, no request. The token and
253+ // host are configured on purpose — without them `newClient()` would fail first
254+ // and this would pass even with the ExactArgs guard removed, which is what the
255+ // version ported from 4a5d5d2 did.
256+ for _, args := range [][]string{
257+ {"repo", "rename", "rick/portal"},
258+ {"repo", "transfer", "rick/portal"},
259+ } {
260+ _, _, err := execute(t, args...)
261+ if err == nil {
262+ t.Errorf("%v: a missing destination should fail arg parsing", args)
263+ continue
264+ }
265+ if !strings.Contains(err.Error(), "accepts 2 arg(s)") {
266+ t.Errorf("%v: want an arg-count error, got %v", args, err)
267+ }
268+ if seen.Path != "" {
269+ t.Errorf("%v: reached the server at %q", args, seen.Path)
270+ }
271+ }
272+}
273+
274+// TestRepoTransferPromptsBeforeMoving: a transfer is a one-way door for the person
275+// making it — after the move the actor may hold no ADMIN in the destination, so
276+// transferring back answers forbidden_owner. It confirms like `repo delete` does.
277+func TestRepoTransferPromptsBeforeMoving(t *testing.T) {
278+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
279+ t.Setenv("RICKUB_TOKEN", "rickub_pat_test")
280+ seen := &captured{}
281+ srv := repoMoveServer(t, seen)
282+ defer srv.Close()
283+ t.Setenv("RICKUB_HOST", srv.URL)
284+
285+ // A wrong answer aborts, and nothing is sent.
286+ rootCmd.SetIn(strings.NewReader("nope\n"))
287+ _, _, err := execute(t, "repo", "transfer", "rick/portal", "acme")
288+ rootCmd.SetIn(nil)
289+ if err == nil {
290+ t.Fatal("a mistyped confirmation was accepted")
291+ }
292+ if seen.Path != "" {
293+ t.Fatalf("an unconfirmed transfer reached the server at %q", seen.Path)
294+ }
295+
296+ // Typing the repo name goes through.
297+ rootCmd.SetIn(strings.NewReader("portal\n"))
298+ out, _, err := execute(t, "repo", "transfer", "rick/portal", "acme")
299+ rootCmd.SetIn(nil)
300+ if err != nil {
301+ t.Fatalf("execute: %v", err)
302+ }
303+ if seen.Path != "/api/v1/repos/rick/portal/transfer" {
304+ t.Errorf("confirmed transfer did not reach the endpoint: %q", seen.Path)
305+ }
306+ if !strings.Contains(out, "Transferred to acme/portal") {
307+ t.Errorf("output missing new full name:\n%s", out)
308+ }
309+
310+ // --yes skips the prompt: scripts keep working without a tty.
311+ seen.Path = ""
312+ if _, _, err := execute(t, "repo", "transfer", "rick/portal", "acme", "--yes"); err != nil {
313+ t.Fatalf("--yes: %v", err)
314+ }
315+ if seen.Path != "/api/v1/repos/rick/portal/transfer" {
316+ t.Errorf("--yes did not reach the endpoint: %q", seen.Path)
317+ }
318+}
319+
320+// TestRepoRenameDoesNotPrompt: rename stays unprompted. It breaks clone URLs, but
321+// the actor keeps ADMIN and can rename straight back — a confirmation on every
322+// rename would be friction without a one-way door behind it.
323+func TestRepoRenameDoesNotPrompt(t *testing.T) {
324+ t.Setenv("XDG_CONFIG_HOME", t.TempDir())
325+ t.Setenv("RICKUB_TOKEN", "rickub_pat_test")
326+ seen := &captured{}
327+ srv := repoMoveServer(t, seen)
328+ defer srv.Close()
329+ t.Setenv("RICKUB_HOST", srv.URL)
330+
331+ rootCmd.SetIn(strings.NewReader(""))
332+ _, _, err := execute(t, "repo", "rename", "rick/portal", "portal-gun")
333+ rootCmd.SetIn(nil)
334+ if err != nil {
335+ t.Fatalf("execute: %v", err)
246 }336 }
247- if _, _, err := execute(t, "repo", "transfer", "rick/portal"); err == nil {337+ if seen.Path != "/api/v1/repos/rick/portal/rename" {
248- t.Error("transfer without a destination owner should fail arg parsing")338+ t.Errorf("rename should not need a confirmation: %q", seen.Path)
249 }339 }
250 }340 }
251 341