From 8152ae3f9f7c287f140e032cd7617aa01b10331f Mon Sep 17 00:00:00 2001 From: John Westerlund Date: Wed, 4 Mar 2026 11:04:32 +0100 Subject: [PATCH 1/2] feat(keep): add create and delete commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two new subcommands to `gog keep`: - `keep create --title --text ` – create a text note - `keep create --title --item --item ` – create a checklist note - `keep delete ` – delete a note (requires --force or interactive confirmation) Both commands support --dry-run, --json/--plain output, and the full --service-account / --impersonate flow used by the rest of the Keep command group. Eight unit tests cover the happy paths, JSON output, input validation, and the notes/ prefix normalization for delete. Co-Authored-By: Claude Sonnet 4.6 --- internal/cmd/keep.go | 101 ++++++++++++++ internal/cmd/keep_test.go | 287 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 388 insertions(+) diff --git a/internal/cmd/keep.go b/internal/cmd/keep.go index ec67360d..ff6d0c4a 100644 --- a/internal/cmd/keep.go +++ b/internal/cmd/keep.go @@ -25,6 +25,8 @@ type KeepCmd struct { List KeepListCmd `cmd:"" default:"withargs" help:"List notes"` Get KeepGetCmd `cmd:"" name:"get" help:"Get a note"` Search KeepSearchCmd `cmd:"" name:"search" help:"Search notes by text (client-side)"` + Create KeepCreateCmd `cmd:"" name:"create" help:"Create a new note"` + Delete KeepDeleteCmd `cmd:"" name:"delete" help:"Delete a note"` Attachment KeepAttachmentCmd `cmd:"" name:"attachment" help:"Download an attachment"` } @@ -320,6 +322,105 @@ func (c *KeepAttachmentCmd) Run(ctx context.Context, flags *RootFlags, keep *Kee return nil } +type KeepCreateCmd struct { + Title string `name:"title" help:"Note title"` + Text string `name:"text" help:"Note body text"` + Item []string `name:"item" help:"List item text (repeatable; creates a checklist note)"` +} + +func (c *KeepCreateCmd) Run(ctx context.Context, flags *RootFlags, keep *KeepCmd) error { + u := ui.FromContext(ctx) + + title := strings.TrimSpace(c.Title) + text := strings.TrimSpace(c.Text) + + if text == "" && len(c.Item) == 0 { + return usage("provide --text or at least one --item") + } + if text != "" && len(c.Item) > 0 { + return usage("--text and --item are mutually exclusive") + } + + if dryRunErr := dryRunExit(ctx, flags, "keep.create", map[string]any{ + "title": title, + "text": text, + "items": c.Item, + }); dryRunErr != nil { + return dryRunErr + } + + svc, err := getKeepService(ctx, flags, keep) + if err != nil { + return err + } + + note := &keepapi.Note{Title: title} + + if text != "" { + note.Body = &keepapi.Section{ + Text: &keepapi.TextContent{Text: text}, + } + } else { + items := make([]*keepapi.ListItem, 0, len(c.Item)) + for _, it := range c.Item { + items = append(items, &keepapi.ListItem{ + Text: &keepapi.TextContent{Text: strings.TrimSpace(it)}, + }) + } + note.Body = &keepapi.Section{ + List: &keepapi.ListContent{ListItems: items}, + } + } + + created, err := svc.Notes.Create(note).Context(ctx).Do() + if err != nil { + return err + } + + if outfmt.IsJSON(ctx) { + return outfmt.WriteJSON(ctx, os.Stdout, map[string]any{"note": created}) + } + + u.Out().Printf("name\t%s", created.Name) + u.Out().Printf("title\t%s", created.Title) + u.Out().Printf("created\t%s", created.CreateTime) + return nil +} + +type KeepDeleteCmd struct { + NoteID string `arg:"" name:"noteId" help:"Note ID or name (e.g. notes/abc123)"` +} + +func (c *KeepDeleteCmd) Run(ctx context.Context, flags *RootFlags, keep *KeepCmd) error { + u := ui.FromContext(ctx) + + name := strings.TrimSpace(c.NoteID) + if name == "" { + return usage("empty noteId") + } + if !strings.HasPrefix(name, "notes/") { + name = "notes/" + name + } + + if confirmErr := confirmDestructive(ctx, flags, fmt.Sprintf("delete note %s", name)); confirmErr != nil { + return confirmErr + } + + svc, err := getKeepService(ctx, flags, keep) + if err != nil { + return err + } + + if _, err := svc.Notes.Delete(name).Context(ctx).Do(); err != nil { + return err + } + + return writeResult(ctx, u, + kv("deleted", true), + kv("name", name), + ) +} + func getKeepService(ctx context.Context, flags *RootFlags, keepCmd *KeepCmd) (*keepapi.Service, error) { if keepCmd.ServiceAccount != "" { if keepCmd.Impersonate == "" { diff --git a/internal/cmd/keep_test.go b/internal/cmd/keep_test.go index 423b5ff8..e4c00c68 100644 --- a/internal/cmd/keep_test.go +++ b/internal/cmd/keep_test.go @@ -635,3 +635,290 @@ func TestGetKeepService_UsesLegacyPath(t *testing.T) { t.Fatalf("unexpected path: %q", gotPath) } } + +// ---- keep create ---- + +func TestKeepCreate_TextPlain(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, "xdg-config")) + + account := "a@b.com" + _ = writeKeepSA(t, account) + + var gotBody []byte + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.URL.Path == "/v1/notes" { + gotBody, _ = io.ReadAll(r.Body) + _, _ = io.WriteString(w, `{"name":"notes/new1","title":"My note","createTime":"2026-01-01T00:00:00Z","updateTime":"2026-01-01T00:00:00Z"}`) + return + } + http.NotFound(w, r) + })) + t.Cleanup(srv.Close) + + orig := newKeepServiceWithSA + t.Cleanup(func() { newKeepServiceWithSA = orig }) + newKeepServiceWithSA = func(ctx context.Context, _, _ string) (*keepapi.Service, error) { + return keepapi.NewService(ctx, + option.WithEndpoint(srv.URL+"/"), + option.WithHTTPClient(srv.Client()), + option.WithoutAuthentication(), + ) + } + + out := captureStdout(t, func() { + _ = captureStderr(t, func() { + if err := Execute([]string{"keep", "create", "--title", "My note", "--text", "Hello world", "--plain", "--account", account}); err != nil { + t.Fatalf("Execute: %v", err) + } + }) + }) + + if !strings.Contains(out, "notes/new1") { + t.Fatalf("unexpected output: %q", out) + } + if !strings.Contains(string(gotBody), "Hello world") { + t.Fatalf("expected body text in request, got: %q", string(gotBody)) + } +} + +func TestKeepCreate_ListItems(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, "xdg-config")) + + account := "a@b.com" + _ = writeKeepSA(t, account) + + var gotBody []byte + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.URL.Path == "/v1/notes" { + gotBody, _ = io.ReadAll(r.Body) + _, _ = io.WriteString(w, `{"name":"notes/new2","title":"Checklist","createTime":"2026-01-01T00:00:00Z","updateTime":"2026-01-01T00:00:00Z"}`) + return + } + http.NotFound(w, r) + })) + t.Cleanup(srv.Close) + + orig := newKeepServiceWithSA + t.Cleanup(func() { newKeepServiceWithSA = orig }) + newKeepServiceWithSA = func(ctx context.Context, _, _ string) (*keepapi.Service, error) { + return keepapi.NewService(ctx, + option.WithEndpoint(srv.URL+"/"), + option.WithHTTPClient(srv.Client()), + option.WithoutAuthentication(), + ) + } + + out := captureStdout(t, func() { + _ = captureStderr(t, func() { + if err := Execute([]string{"keep", "create", "--title", "Checklist", "--item", "Milk", "--item", "Eggs", "--plain", "--account", account}); err != nil { + t.Fatalf("Execute: %v", err) + } + }) + }) + + if !strings.Contains(out, "notes/new2") { + t.Fatalf("unexpected output: %q", out) + } + body := string(gotBody) + if !strings.Contains(body, "Milk") || !strings.Contains(body, "Eggs") { + t.Fatalf("expected list items in request, got: %q", body) + } +} + +func TestKeepCreate_JSON(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, "xdg-config")) + + account := "a@b.com" + _ = writeKeepSA(t, account) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.URL.Path == "/v1/notes" { + _, _ = io.WriteString(w, `{"name":"notes/new3","title":"T","createTime":"2026-01-01T00:00:00Z","updateTime":"2026-01-01T00:00:00Z"}`) + return + } + http.NotFound(w, r) + })) + t.Cleanup(srv.Close) + + orig := newKeepServiceWithSA + t.Cleanup(func() { newKeepServiceWithSA = orig }) + newKeepServiceWithSA = func(ctx context.Context, _, _ string) (*keepapi.Service, error) { + return keepapi.NewService(ctx, + option.WithEndpoint(srv.URL+"/"), + option.WithHTTPClient(srv.Client()), + option.WithoutAuthentication(), + ) + } + + out := captureStdout(t, func() { + _ = captureStderr(t, func() { + if err := Execute([]string{"--json", "keep", "create", "--text", "body", "--account", account}); err != nil { + t.Fatalf("Execute: %v", err) + } + }) + }) + + var payload struct { + Note map[string]any `json:"note"` + } + if err := json.Unmarshal([]byte(out), &payload); err != nil { + t.Fatalf("json parse: %v\nout=%q", err, out) + } + if payload.Note["name"] != "notes/new3" { + t.Fatalf("unexpected note: %#v", payload.Note) + } +} + +func TestKeepCreate_MissingBody(t *testing.T) { + err := (&KeepCreateCmd{}).Run(context.Background(), nil, nil) + if err == nil { + t.Fatalf("expected error for missing body") + } +} + +func TestKeepCreate_TextAndItemMutuallyExclusive(t *testing.T) { + err := (&KeepCreateCmd{Text: "hi", Item: []string{"x"}}).Run(context.Background(), nil, nil) + if err == nil { + t.Fatalf("expected error for conflicting flags") + } +} + +// ---- keep delete ---- + +func TestKeepDelete_Plain(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, "xdg-config")) + + account := "a@b.com" + _ = writeKeepSA(t, account) + + deleted := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete && r.URL.Path == "/v1/notes/abc" { + deleted = true + _, _ = io.WriteString(w, `{}`) + return + } + http.NotFound(w, r) + })) + t.Cleanup(srv.Close) + + orig := newKeepServiceWithSA + t.Cleanup(func() { newKeepServiceWithSA = orig }) + newKeepServiceWithSA = func(ctx context.Context, _, _ string) (*keepapi.Service, error) { + return keepapi.NewService(ctx, + option.WithEndpoint(srv.URL+"/"), + option.WithHTTPClient(srv.Client()), + option.WithoutAuthentication(), + ) + } + + out := captureStdout(t, func() { + _ = captureStderr(t, func() { + if err := Execute([]string{"keep", "delete", "abc", "--plain", "--account", account, "--force"}); err != nil { + t.Fatalf("Execute: %v", err) + } + }) + }) + + if !deleted { + t.Fatalf("expected DELETE request to be made") + } + if !strings.Contains(out, "deleted") { + t.Fatalf("unexpected output: %q", out) + } +} + +func TestKeepDelete_WithNotesPrefix(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, "xdg-config")) + + account := "a@b.com" + _ = writeKeepSA(t, account) + + deleted := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete && r.URL.Path == "/v1/notes/xyz" { + deleted = true + _, _ = io.WriteString(w, `{}`) + return + } + http.NotFound(w, r) + })) + t.Cleanup(srv.Close) + + orig := newKeepServiceWithSA + t.Cleanup(func() { newKeepServiceWithSA = orig }) + newKeepServiceWithSA = func(ctx context.Context, _, _ string) (*keepapi.Service, error) { + return keepapi.NewService(ctx, + option.WithEndpoint(srv.URL+"/"), + option.WithHTTPClient(srv.Client()), + option.WithoutAuthentication(), + ) + } + + _ = captureStdout(t, func() { + _ = captureStderr(t, func() { + // pass full "notes/xyz" prefix — should not double-prefix + if err := Execute([]string{"keep", "delete", "notes/xyz", "--plain", "--account", account, "--force"}); err != nil { + t.Fatalf("Execute: %v", err) + } + }) + }) + + if !deleted { + t.Fatalf("expected DELETE request for notes/xyz") + } +} + +func TestKeepDelete_JSON(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, "xdg-config")) + + account := "a@b.com" + _ = writeKeepSA(t, account) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + _, _ = io.WriteString(w, `{}`) + return + } + http.NotFound(w, r) + })) + t.Cleanup(srv.Close) + + orig := newKeepServiceWithSA + t.Cleanup(func() { newKeepServiceWithSA = orig }) + newKeepServiceWithSA = func(ctx context.Context, _, _ string) (*keepapi.Service, error) { + return keepapi.NewService(ctx, + option.WithEndpoint(srv.URL+"/"), + option.WithHTTPClient(srv.Client()), + option.WithoutAuthentication(), + ) + } + + out := captureStdout(t, func() { + _ = captureStderr(t, func() { + if err := Execute([]string{"--json", "keep", "delete", "abc", "--account", account, "--force"}); err != nil { + t.Fatalf("Execute: %v", err) + } + }) + }) + + var payload map[string]any + if err := json.Unmarshal([]byte(out), &payload); err != nil { + t.Fatalf("json parse: %v\nout=%q", err, out) + } + if payload["deleted"] != true { + t.Fatalf("expected deleted=true, got: %#v", payload) + } +} From 30394a2573165131d3a5fe16a87ea2a233f9e1b7 Mon Sep 17 00:00:00 2001 From: John Westerlund Date: Wed, 4 Mar 2026 11:23:22 +0100 Subject: [PATCH 2/2] fix(keep): upgrade scope from keep.readonly to keep The keep.readonly scope only allows reads. Now that we have create and delete commands, the service must request the full keep scope so that domain-wide delegation grants write access. Co-Authored-By: Claude Sonnet 4.6 --- internal/googleauth/service.go | 2 +- internal/googleauth/service_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/googleauth/service.go b/internal/googleauth/service.go index b3c3afe8..75635754 100644 --- a/internal/googleauth/service.go +++ b/internal/googleauth/service.go @@ -207,7 +207,7 @@ var serviceInfoByService = map[Service]serviceInfo{ note: "Workspace only", }, ServiceKeep: { - scopes: []string{"https://www.googleapis.com/auth/keep.readonly"}, + scopes: []string{"https://www.googleapis.com/auth/keep"}, user: false, apis: []string{"Keep API"}, note: "Workspace only; service account (domain-wide delegation)", diff --git a/internal/googleauth/service_test.go b/internal/googleauth/service_test.go index 94e25823..70322beb 100644 --- a/internal/googleauth/service_test.go +++ b/internal/googleauth/service_test.go @@ -312,7 +312,7 @@ func TestScopes_ServiceKeep_DefaultIsReadonly(t *testing.T) { t.Fatalf("Scopes: %v", err) } - if len(scopes) != 1 || scopes[0] != "https://www.googleapis.com/auth/keep.readonly" { + if len(scopes) != 1 || scopes[0] != "https://www.googleapis.com/auth/keep" { t.Fatalf("unexpected keep scopes: %#v", scopes) } } @@ -323,7 +323,7 @@ func TestScopesForServiceWithOptions_ServiceKeep_Readonly(t *testing.T) { t.Fatalf("scopesForServiceWithOptions: %v", err) } - if len(scopes) != 1 || scopes[0] != "https://www.googleapis.com/auth/keep.readonly" { + if len(scopes) != 1 || scopes[0] != "https://www.googleapis.com/auth/keep" { t.Fatalf("unexpected keep readonly scopes: %#v", scopes) } }