From 0a3cd7a423772cf42400937970144dcb5e6c6b5c Mon Sep 17 00:00:00 2001 From: Jared Palmer Date: Thu, 19 Feb 2026 10:52:36 -0500 Subject: [PATCH 1/5] Implement native keychain backend resolution and headless password semantics --- internal/secrets/keychain_darwin.go | 172 +++++++++++++++++++++-- internal/secrets/keychain_other.go | 18 +++ internal/secrets/store.go | 154 +++++++++++++++++++-- internal/secrets/store_test.go | 202 +++++++++++++++++++++++++++- 4 files changed, 524 insertions(+), 22 deletions(-) diff --git a/internal/secrets/keychain_darwin.go b/internal/secrets/keychain_darwin.go index 4ac1273..70c732f 100644 --- a/internal/secrets/keychain_darwin.go +++ b/internal/secrets/keychain_darwin.go @@ -2,14 +2,170 @@ package secrets -// IsKeychainLockedError returns false for the file-backed secret store. -func IsKeychainLockedError(_ string) bool { return false } +import ( + "bufio" + "context" + "encoding/base64" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" -// CheckKeychainLocked returns false for the file-backed secret store. -func CheckKeychainLocked() bool { return false } + "github.com/jaredpalmer/mogcli/internal/config" +) -// UnlockKeychain is a no-op for the file-backed secret store. -func UnlockKeychain() error { return nil } +const ( + // errSecInteractionNotAllowed is macOS Security framework error -25308. + errSecInteractionNotAllowed = "-25308" +) -// EnsureKeychainAccess is a no-op for the file-backed secret store. -func EnsureKeychainAccess() error { return nil } +var ( + errKeychainPathUnknown = errors.New("cannot determine login keychain path") + errKeychainNoTTY = errors.New("keychain is locked and no TTY available for password prompt") + errKeychainUnlock = errors.New("unlock keychain: incorrect password or keychain error") + + execLookPathFunc = exec.LookPath + execCommandContextFunc = exec.CommandContext +) + +// IsKeychainLockedError returns true if the error string indicates a locked keychain. +func IsKeychainLockedError(errStr string) bool { + return strings.Contains(errStr, errSecInteractionNotAllowed) +} + +func nativeKeychainAvailable() bool { + _, err := execLookPathFunc("security") + return err == nil +} + +func setNativeKeychainSecret(key string, value []byte) error { + account := nativeKeychainAccount(key) + output, err := runSecurity("add-generic-password", "-a", account, "-s", config.AppName, "-U", "-w", string(value)) + if err != nil { + return formatSecurityError("store keychain secret", err, output) + } + return nil +} + +func getNativeKeychainSecret(key string) ([]byte, error) { + account := nativeKeychainAccount(key) + output, err := runSecurity("find-generic-password", "-a", account, "-s", config.AppName, "-w") + if err != nil { + if isKeychainItemNotFound(output) { + return nil, os.ErrNotExist + } + return nil, formatSecurityError("read keychain secret", err, output) + } + return output, nil +} + +func deleteNativeKeychainSecret(key string) error { + account := nativeKeychainAccount(key) + output, err := runSecurity("delete-generic-password", "-a", account, "-s", config.AppName) + if err != nil && !isKeychainItemNotFound(output) { + return formatSecurityError("delete keychain secret", err, output) + } + return nil +} + +func nativeKeychainAccount(key string) string { + return config.AppName + ":" + base64.RawURLEncoding.EncodeToString([]byte(strings.TrimSpace(key))) +} + +func runSecurity(args ...string) ([]byte, error) { + cmd := execCommandContextFunc(context.Background(), "security", args...) + return cmd.CombinedOutput() +} + +func isKeychainItemNotFound(output []byte) bool { + msg := strings.ToLower(strings.TrimSpace(string(output))) + return strings.Contains(msg, "could not be found") || + strings.Contains(msg, "item not found") +} + +func formatSecurityError(action string, runErr error, output []byte) error { + message := strings.TrimSpace(string(output)) + if message == "" { + return fmt.Errorf("%s: %w", action, runErr) + } + return fmt.Errorf("%s: %w (%s)", action, runErr, message) +} + +// loginKeychainPath returns the path to the user's login keychain. +func loginKeychainPath() string { + home, err := os.UserHomeDir() + if err != nil { + return "" + } + + return filepath.Join(home, "Library", "Keychains", "login.keychain-db") +} + +// CheckKeychainLocked checks if the login keychain is locked. +func CheckKeychainLocked() bool { + path := loginKeychainPath() + if path == "" { + return false + } + + cmd := execCommandContextFunc(context.Background(), "security", "show-keychain-info", path) //nolint:gosec // path is from os.UserHomeDir + return cmd.Run() != nil +} + +// UnlockKeychain prompts for password and unlocks the login keychain. +func UnlockKeychain() error { + path := loginKeychainPath() + if path == "" { + return errKeychainPathUnknown + } + + if password, passwordSet := keyringPassword(); passwordSet { + return unlockKeychainWithPassword(path, password) + } + + if !isTTY(os.Stdin) { + return fmt.Errorf("%w\n\nTo unlock manually, run:\n security unlock-keychain ~/Library/Keychains/login.keychain-db", errKeychainNoTTY) + } + + fmt.Fprint(os.Stderr, "Keychain is locked. Enter your macOS login password to unlock: ") + + line, err := bufio.NewReader(os.Stdin).ReadString('\n') + if err != nil { + return fmt.Errorf("read password: %w", err) + } + + return unlockKeychainWithPassword(path, strings.TrimRight(line, "\r\n")) +} + +func unlockKeychainWithPassword(path string, password string) error { + cmd := execCommandContextFunc(context.Background(), "security", "unlock-keychain", path) //nolint:gosec // path is from os.UserHomeDir + cmd.Stdin = strings.NewReader(password + "\n") + + if err := cmd.Run(); err != nil { + return errKeychainUnlock + } + return nil +} + +// EnsureKeychainAccess checks if the keychain is accessible and unlocks if needed. +func EnsureKeychainAccess() error { + if !CheckKeychainLocked() { + return nil + } + return UnlockKeychain() +} + +func isTTY(file *os.File) bool { + if file == nil { + return false + } + + info, err := file.Stat() + if err != nil { + return false + } + + return (info.Mode() & os.ModeCharDevice) != 0 +} diff --git a/internal/secrets/keychain_other.go b/internal/secrets/keychain_other.go index 314b56a..ff675a5 100644 --- a/internal/secrets/keychain_other.go +++ b/internal/secrets/keychain_other.go @@ -2,6 +2,24 @@ package secrets +import "errors" + +var errNativeKeychainUnsupported = errors.New("native keychain backend unsupported on this platform") + +func nativeKeychainAvailable() bool { return false } + +func setNativeKeychainSecret(_ string, _ []byte) error { + return errNativeKeychainUnsupported +} + +func getNativeKeychainSecret(_ string) ([]byte, error) { + return nil, errNativeKeychainUnsupported +} + +func deleteNativeKeychainSecret(_ string) error { + return errNativeKeychainUnsupported +} + func IsKeychainLockedError(_ string) bool { return false } func CheckKeychainLocked() bool { return false } func UnlockKeychain() error { return nil } diff --git a/internal/secrets/store.go b/internal/secrets/store.go index 3c48a9f..daf61e4 100644 --- a/internal/secrets/store.go +++ b/internal/secrets/store.go @@ -16,7 +16,17 @@ const ( keyringBackendEnv = "MOG_KEYRING_BACKEND" //nolint:gosec // env var name, not a credential ) -var errMissingSecretKey = errors.New("missing secret key") +var ( + errMissingSecretKey = errors.New("missing secret key") + errInvalidKeyringBackend = errors.New("invalid keyring backend") + errNativeKeychainUnavailable = errors.New("native keychain backend unavailable") + + nativeKeychainAvailableFunc = nativeKeychainAvailable + ensureKeychainAccessFunc = EnsureKeychainAccess + setNativeKeychainSecretFunc = setNativeKeychainSecret + getNativeKeychainSecretFunc = getNativeKeychainSecret + deleteNativeKeychainSecretFunc = deleteNativeKeychainSecret +) type KeyringBackendInfo struct { Value string @@ -28,6 +38,8 @@ const ( keyringBackendSourceConfig = "config" keyringBackendSourceDefault = "default" keyringBackendAuto = "auto" + keyringBackendKeychain = "keychain" + keyringBackendFile = "file" ) func ResolveKeyringBackendInfo() (KeyringBackendInfo, error) { @@ -53,21 +65,138 @@ func normalizeKeyringBackend(value string) string { return strings.ToLower(strings.TrimSpace(value)) } +func keyringPassword() (string, bool) { + // Treat empty strings as intentional (for headless/non-interactive use). + return os.LookupEnv(keyringPasswordEnv) +} + func SetSecret(key string, value []byte) error { - path, err := secretPath(key) + normalized, err := normalizeSecretKey(key) + if err != nil { + return err + } + + backend, err := resolveSecretBackend() + if err != nil { + return err + } + + switch backend { + case keyringBackendKeychain: + if err := ensureKeychainAccessFunc(); err != nil { + return fmt.Errorf("access keychain: %w", err) + } + if err := setNativeKeychainSecretFunc(normalized, value); err != nil { + return fmt.Errorf("store secret: %w", err) + } + return nil + case keyringBackendFile: + return setFileSecret(normalized, value) + default: + return fmt.Errorf("%w: %q", errInvalidKeyringBackend, backend) + } +} + +func GetSecret(key string) ([]byte, error) { + normalized, err := normalizeSecretKey(key) + if err != nil { + return nil, err + } + + backend, err := resolveSecretBackend() + if err != nil { + return nil, err + } + + switch backend { + case keyringBackendKeychain: + if err := ensureKeychainAccessFunc(); err != nil { + return nil, fmt.Errorf("access keychain: %w", err) + } + secret, err := getNativeKeychainSecretFunc(normalized) + if err != nil { + return nil, fmt.Errorf("read secret: %w", err) + } + return secret, nil + case keyringBackendFile: + return getFileSecret(normalized) + default: + return nil, fmt.Errorf("%w: %q", errInvalidKeyringBackend, backend) + } +} + +func DeleteSecret(key string) error { + normalized, err := normalizeSecretKey(key) if err != nil { return err } + backend, err := resolveSecretBackend() + if err != nil { + return err + } + + switch backend { + case keyringBackendKeychain: + if err := ensureKeychainAccessFunc(); err != nil { + return fmt.Errorf("access keychain: %w", err) + } + if err := deleteNativeKeychainSecretFunc(normalized); err != nil { + return fmt.Errorf("delete secret: %w", err) + } + return nil + case keyringBackendFile: + return deleteFileSecret(normalized) + default: + return fmt.Errorf("%w: %q", errInvalidKeyringBackend, backend) + } +} + +func resolveSecretBackend() (string, error) { + info, err := ResolveKeyringBackendInfo() + if err != nil { + return "", err + } + + switch info.Value { + case "", keyringBackendAuto: + if nativeKeychainAvailableFunc() { + return keyringBackendKeychain, nil + } + return keyringBackendFile, nil + case keyringBackendKeychain: + if !nativeKeychainAvailableFunc() { + return "", fmt.Errorf("%w: %q", errNativeKeychainUnavailable, keyringBackendKeychain) + } + return keyringBackendKeychain, nil + case keyringBackendFile: + return keyringBackendFile, nil + default: + return "", fmt.Errorf("%w: %q (expected auto, keychain, or file)", errInvalidKeyringBackend, info.Value) + } +} + +func normalizeSecretKey(key string) (string, error) { + key = strings.TrimSpace(key) + if key == "" { + return "", errMissingSecretKey + } + return key, nil +} + +func setFileSecret(key string, value []byte) error { + path, err := secretPathFromNormalized(key) + if err != nil { + return err + } if err := os.WriteFile(path, value, 0o600); err != nil { return fmt.Errorf("store secret: %w", err) } - return nil } -func GetSecret(key string) ([]byte, error) { - path, err := secretPath(key) +func getFileSecret(key string) ([]byte, error) { + path, err := secretPathFromNormalized(key) if err != nil { return nil, err } @@ -76,12 +205,11 @@ func GetSecret(key string) ([]byte, error) { if err != nil { return nil, fmt.Errorf("read secret: %w", err) } - return item, nil } -func DeleteSecret(key string) error { - path, err := secretPath(key) +func deleteFileSecret(key string) error { + path, err := secretPathFromNormalized(key) if err != nil { return err } @@ -89,16 +217,18 @@ func DeleteSecret(key string) error { if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) { return fmt.Errorf("delete secret: %w", err) } - return nil } func secretPath(key string) (string, error) { - key = strings.TrimSpace(key) - if key == "" { - return "", errMissingSecretKey + normalized, err := normalizeSecretKey(key) + if err != nil { + return "", err } + return secretPathFromNormalized(normalized) +} +func secretPathFromNormalized(key string) (string, error) { dir, err := config.EnsureKeyringDir() if err != nil { return "", fmt.Errorf("ensure secret dir: %w", err) diff --git a/internal/secrets/store_test.go b/internal/secrets/store_test.go index 2045120..6dad9de 100644 --- a/internal/secrets/store_test.go +++ b/internal/secrets/store_test.go @@ -5,9 +5,11 @@ import ( "os" "path/filepath" "testing" + + "github.com/jaredpalmer/mogcli/internal/config" ) -func TestSetGetDeleteSecret(t *testing.T) { +func TestSetGetDeleteSecretFileBackend(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) @@ -36,7 +38,6 @@ func TestSetGetDeleteSecret(t *testing.T) { t.Fatal("expected error after deleting secret") } - // Verify path is inside keyring dir. dir, err := os.UserConfigDir() if err != nil { t.Fatalf("UserConfigDir failed: %v", err) @@ -45,3 +46,200 @@ func TestSetGetDeleteSecret(t *testing.T) { t.Fatalf("stat keyring dir failed: %v", statErr) } } + +func TestSetGetDeleteSecretKeychainBackendDispatch(t *testing.T) { + t.Setenv("MOG_KEYRING_BACKEND", "keychain") + restore := stubNativeKeychainFuncs(t) + + nativeKeychainAvailableFunc = func() bool { return true } + + ensureCalls := 0 + ensureKeychainAccessFunc = func() error { + ensureCalls++ + return nil + } + + var setKey string + var setValue []byte + setNativeKeychainSecretFunc = func(key string, value []byte) error { + setKey = key + setValue = append([]byte(nil), value...) + return nil + } + + getNativeKeychainSecretFunc = func(key string) ([]byte, error) { + if key != "mog:test:key" { + t.Fatalf("unexpected get key %q", key) + } + return []byte("value-from-keychain"), nil + } + + deleteKey := "" + deleteNativeKeychainSecretFunc = func(key string) error { + deleteKey = key + return nil + } + + t.Cleanup(restore) + + if err := SetSecret("mog:test:key", []byte("hello-world")); err != nil { + t.Fatalf("SetSecret failed: %v", err) + } + if setKey != "mog:test:key" { + t.Fatalf("unexpected set key: %q", setKey) + } + if string(setValue) != "hello-world" { + t.Fatalf("unexpected set value: %q", string(setValue)) + } + + got, err := GetSecret("mog:test:key") + if err != nil { + t.Fatalf("GetSecret failed: %v", err) + } + if string(got) != "value-from-keychain" { + t.Fatalf("unexpected get value: %q", string(got)) + } + + if err := DeleteSecret("mog:test:key"); err != nil { + t.Fatalf("DeleteSecret failed: %v", err) + } + if deleteKey != "mog:test:key" { + t.Fatalf("unexpected delete key: %q", deleteKey) + } + + if ensureCalls != 3 { + t.Fatalf("expected EnsureKeychainAccess to be called 3 times, got %d", ensureCalls) + } +} + +func TestAutoBackendFallsBackToFileWhenNativeUnavailable(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + t.Setenv("MOG_KEYRING_BACKEND", "auto") + + restore := stubNativeKeychainFuncs(t) + nativeKeychainAvailableFunc = func() bool { return false } + setNativeKeychainSecretFunc = func(_ string, _ []byte) error { + t.Fatal("unexpected native keychain set call") + return nil + } + getNativeKeychainSecretFunc = func(_ string) ([]byte, error) { + t.Fatal("unexpected native keychain get call") + return nil, nil + } + deleteNativeKeychainSecretFunc = func(_ string) error { + t.Fatal("unexpected native keychain delete call") + return nil + } + t.Cleanup(restore) + + key := "mog:test:auto-file" + value := []byte("auto-file-value") + if err := SetSecret(key, value); err != nil { + t.Fatalf("SetSecret failed: %v", err) + } + + got, err := GetSecret(key) + if err != nil { + t.Fatalf("GetSecret failed: %v", err) + } + if string(got) != string(value) { + t.Fatalf("unexpected secret value: %q", string(got)) + } + + if err := DeleteSecret(key); err != nil { + t.Fatalf("DeleteSecret failed: %v", err) + } +} + +func TestResolveKeyringBackendInfoPrecedence(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) + t.Setenv("MOG_KEYRING_BACKEND", "") + + info, err := ResolveKeyringBackendInfo() + if err != nil { + t.Fatalf("ResolveKeyringBackendInfo default failed: %v", err) + } + if info.Value != keyringBackendAuto || info.Source != keyringBackendSourceDefault { + t.Fatalf("unexpected default backend info: %#v", info) + } + + if err := config.WriteConfig(config.File{KeyringBackend: "file"}); err != nil { + t.Fatalf("WriteConfig failed: %v", err) + } + + info, err = ResolveKeyringBackendInfo() + if err != nil { + t.Fatalf("ResolveKeyringBackendInfo config failed: %v", err) + } + if info.Value != keyringBackendFile || info.Source != keyringBackendSourceConfig { + t.Fatalf("unexpected config backend info: %#v", info) + } + + t.Setenv("MOG_KEYRING_BACKEND", "keychain") + info, err = ResolveKeyringBackendInfo() + if err != nil { + t.Fatalf("ResolveKeyringBackendInfo env failed: %v", err) + } + if info.Value != keyringBackendKeychain || info.Source != keyringBackendSourceEnv { + t.Fatalf("unexpected env backend info: %#v", info) + } +} + +func TestSetSecretRejectsInvalidBackend(t *testing.T) { + t.Setenv("MOG_KEYRING_BACKEND", "bogus") + err := SetSecret("mog:test:key", []byte("value")) + if err == nil { + t.Fatal("expected SetSecret to fail for invalid backend") + } + if !errors.Is(err, errInvalidKeyringBackend) { + t.Fatalf("expected errInvalidKeyringBackend, got %v", err) + } +} + +func TestSetSecretRejectsUnavailableNativeKeychain(t *testing.T) { + t.Setenv("MOG_KEYRING_BACKEND", "keychain") + restore := stubNativeKeychainFuncs(t) + nativeKeychainAvailableFunc = func() bool { return false } + t.Cleanup(restore) + + err := SetSecret("mog:test:key", []byte("value")) + if err == nil { + t.Fatal("expected SetSecret to fail for unavailable native keychain") + } + if !errors.Is(err, errNativeKeychainUnavailable) { + t.Fatalf("expected errNativeKeychainUnavailable, got %v", err) + } +} + +func TestKeyringPasswordTreatsEmptyAsIntentional(t *testing.T) { + t.Setenv("MOG_KEYRING_PASSWORD", "") + password, ok := keyringPassword() + if !ok { + t.Fatal("expected empty keyring password to be treated as set") + } + if password != "" { + t.Fatalf("expected empty password, got %q", password) + } +} + +func stubNativeKeychainFuncs(t *testing.T) func() { + t.Helper() + + originalAvailable := nativeKeychainAvailableFunc + originalEnsure := ensureKeychainAccessFunc + originalSet := setNativeKeychainSecretFunc + originalGet := getNativeKeychainSecretFunc + originalDelete := deleteNativeKeychainSecretFunc + + return func() { + nativeKeychainAvailableFunc = originalAvailable + ensureKeychainAccessFunc = originalEnsure + setNativeKeychainSecretFunc = originalSet + getNativeKeychainSecretFunc = originalGet + deleteNativeKeychainSecretFunc = originalDelete + } +} From 2c304767cda4c8bdebc98f58d824e672c255da63 Mon Sep 17 00:00:00 2001 From: Jared Palmer Date: Thu, 19 Feb 2026 10:53:21 -0500 Subject: [PATCH 2/5] Split calendar attendee and reminder update patches for Graph safety --- internal/services/calendar/service.go | 47 +++++++++- internal/services/calendar/service_test.go | 103 +++++++++++++++++++++ 2 files changed, 148 insertions(+), 2 deletions(-) diff --git a/internal/services/calendar/service.go b/internal/services/calendar/service.go index cce1ada..fccc253 100644 --- a/internal/services/calendar/service.go +++ b/internal/services/calendar/service.go @@ -69,8 +69,22 @@ func (s *Service) Create(ctx context.Context, payload map[string]any) (map[strin } func (s *Service) Update(ctx context.Context, id string, payload map[string]any) error { - _, _, err := s.client.Do(ctx, http.MethodPatch, "/me/events/"+url.PathEscape(strings.TrimSpace(id)), nil, payload, updateCalendarScopes, nil) - return err + endpoint := "/me/events/" + url.PathEscape(strings.TrimSpace(id)) + primary, attendeesOnly := splitUpdatePayload(payload) + + if len(primary) > 0 { + if _, _, err := s.client.Do(ctx, http.MethodPatch, endpoint, nil, primary, updateCalendarScopes, nil); err != nil { + return err + } + } + + if len(attendeesOnly) > 0 { + if _, _, err := s.client.Do(ctx, http.MethodPatch, endpoint, nil, attendeesOnly, updateCalendarScopes, nil); err != nil { + return err + } + } + + return nil } func (s *Service) Delete(ctx context.Context, id string) error { @@ -84,3 +98,32 @@ func trimPage(items []map[string]any, next string, max int) ([]map[string]any, s } return items[:max], next } + +func splitUpdatePayload(payload map[string]any) (map[string]any, map[string]any) { + if len(payload) == 0 { + return map[string]any{}, nil + } + + attendees, hasAttendees := payload["attendees"] + if !hasAttendees || !hasReminderField(payload) { + return clonePayload(payload), nil + } + + primary := clonePayload(payload) + delete(primary, "attendees") + return primary, map[string]any{"attendees": attendees} +} + +func hasReminderField(payload map[string]any) bool { + _, hasReminderOn := payload["isReminderOn"] + _, hasReminderMins := payload["reminderMinutesBeforeStart"] + return hasReminderOn || hasReminderMins +} + +func clonePayload(payload map[string]any) map[string]any { + cloned := make(map[string]any, len(payload)) + for key, value := range payload { + cloned[key] = value + } + return cloned +} diff --git a/internal/services/calendar/service_test.go b/internal/services/calendar/service_test.go index 13ef79b..bb9c7e3 100644 --- a/internal/services/calendar/service_test.go +++ b/internal/services/calendar/service_test.go @@ -2,6 +2,7 @@ package calendar import ( "context" + "encoding/json" "fmt" "net/http" "net/http/httptest" @@ -47,3 +48,105 @@ func TestListUsesPageTokenURL(t *testing.T) { t.Fatalf("unexpected next link: %s", next) } } + +func TestUpdateSplitsAttendeesFromReminderPatch(t *testing.T) { + var payloads []map[string]any + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPatch { + t.Fatalf("unexpected method %s", r.Method) + } + if r.URL.Path != "/me/events/event-id" { + t.Fatalf("unexpected path %s", r.URL.Path) + } + + var body map[string]any + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Fatalf("decode body failed: %v", err) + } + payloads = append(payloads, body) + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + client := graph.NewClient(func(context.Context, []string) (string, error) { return "token", nil }) + client.BaseURL = server.URL + client.HTTPClient = server.Client() + + payload := map[string]any{ + "subject": "Updated", + "isReminderOn": true, + "reminderMinutesBeforeStart": 15, + "attendees": []map[string]any{ + { + "emailAddress": map[string]any{"address": "dev@example.com"}, + "type": "required", + }, + }, + } + + if err := New(client).Update(context.Background(), "event-id", payload); err != nil { + t.Fatalf("Update failed: %v", err) + } + + if len(payloads) != 2 { + t.Fatalf("expected two patch requests, got %d", len(payloads)) + } + + if _, ok := payloads[0]["attendees"]; ok { + t.Fatalf("expected first patch payload to exclude attendees: %#v", payloads[0]) + } + if _, ok := payloads[0]["isReminderOn"]; !ok { + t.Fatalf("expected first payload to include reminder field: %#v", payloads[0]) + } + + if len(payloads[1]) != 1 { + t.Fatalf("expected second payload to include only attendees, got %#v", payloads[1]) + } + if _, ok := payloads[1]["attendees"]; !ok { + t.Fatalf("expected second payload to include attendees: %#v", payloads[1]) + } +} + +func TestUpdateSendsSinglePatchWhenNoReminderAttendeeConflict(t *testing.T) { + requests := 0 + var received map[string]any + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + if r.Method != http.MethodPatch { + t.Fatalf("unexpected method %s", r.Method) + } + if r.URL.Path != "/me/events/event-id" { + t.Fatalf("unexpected path %s", r.URL.Path) + } + if err := json.NewDecoder(r.Body).Decode(&received); err != nil { + t.Fatalf("decode body failed: %v", err) + } + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + client := graph.NewClient(func(context.Context, []string) (string, error) { return "token", nil }) + client.BaseURL = server.URL + client.HTTPClient = server.Client() + + payload := map[string]any{ + "subject": "Updated", + "attendees": []map[string]any{ + { + "emailAddress": map[string]any{"address": "dev@example.com"}, + "type": "required", + }, + }, + } + + if err := New(client).Update(context.Background(), "event-id", payload); err != nil { + t.Fatalf("Update failed: %v", err) + } + + if requests != 1 { + t.Fatalf("expected one patch request, got %d", requests) + } + if _, ok := received["attendees"]; !ok { + t.Fatalf("expected attendees field in patch payload: %#v", received) + } +} From 8be9aaacd49e01c5a37c3e3126ac6571bce70cd9 Mon Sep 17 00:00:00 2001 From: Jared Palmer Date: Thu, 19 Feb 2026 10:55:32 -0500 Subject: [PATCH 3/5] Expand contacts fields and add deterministic custom field handling --- internal/cmd/contacts.go | 79 ++++++-- internal/cmd/contacts_flags_test.go | 80 ++++++++ internal/services/contacts/custom_fields.go | 184 ++++++++++++++++++ .../services/contacts/custom_fields_test.go | 69 +++++++ internal/services/contacts/service.go | 7 +- internal/services/contacts/service_test.go | 42 ++++ 6 files changed, 448 insertions(+), 13 deletions(-) create mode 100644 internal/cmd/contacts_flags_test.go create mode 100644 internal/services/contacts/custom_fields.go create mode 100644 internal/services/contacts/custom_fields_test.go diff --git a/internal/cmd/contacts.go b/internal/cmd/contacts.go index 4c12089..a851267 100644 --- a/internal/cmd/contacts.go +++ b/internal/cmd/contacts.go @@ -80,10 +80,15 @@ func (c *ContactsGetCmd) Run(ctx context.Context) error { } type ContactsCreateCmd struct { - DisplayName string `name:"display-name" required:"" help:"Contact display name"` - Email string `name:"email" help:"Primary email"` - MobilePhone string `name:"mobile-phone" help:"Mobile phone"` - User string `name:"user" help:"App-only target user override (UPN or user ID)"` + DisplayName string `name:"display-name" required:"" help:"Contact display name"` + Email string `name:"email" help:"Primary email"` + MobilePhone string `name:"mobile-phone" help:"Mobile phone"` + Org string `name:"org" help:"Organization/company name"` + Title string `name:"title" help:"Job title"` + URL string `name:"url" help:"Business homepage URL"` + Note string `name:"note" help:"Personal note"` + Custom []string `name:"custom" help:"Custom field key=value (repeat or comma-separate)"` + User string `name:"user" help:"App-only target user override (UPN or user ID)"` } func (c *ContactsCreateCmd) Run(ctx context.Context) error { @@ -103,6 +108,26 @@ func (c *ContactsCreateCmd) Run(ctx context.Context) error { if strings.TrimSpace(c.MobilePhone) != "" { payload["mobilePhone"] = c.MobilePhone } + if strings.TrimSpace(c.Org) != "" { + payload["companyName"] = strings.TrimSpace(c.Org) + } + if strings.TrimSpace(c.Title) != "" { + payload["jobTitle"] = strings.TrimSpace(c.Title) + } + if strings.TrimSpace(c.URL) != "" { + payload["businessHomePage"] = strings.TrimSpace(c.URL) + } + if strings.TrimSpace(c.Note) != "" { + payload["personalNotes"] = strings.TrimSpace(c.Note) + } + + customFields, err := contacts.ParseCustomFields(c.Custom) + if err != nil { + return usage(err.Error()) + } + if len(customFields) > 0 { + payload["categories"] = contacts.EncodeCustomFieldCategories(customFields) + } item, err := contacts.New(rt.Graph, targetUser).Create(ctx, payload) if err != nil { @@ -117,11 +142,16 @@ func (c *ContactsCreateCmd) Run(ctx context.Context) error { } type ContactsUpdateCmd struct { - ID string `arg:"" required:"" help:"Contact ID"` - DisplayName string `name:"display-name" help:"Contact display name"` - Email string `name:"email" help:"Primary email"` - MobilePhone string `name:"mobile-phone" help:"Mobile phone"` - User string `name:"user" help:"App-only target user override (UPN or user ID)"` + ID string `arg:"" required:"" help:"Contact ID"` + DisplayName string `name:"display-name" help:"Contact display name"` + Email string `name:"email" help:"Primary email"` + MobilePhone string `name:"mobile-phone" help:"Mobile phone"` + Org string `name:"org" help:"Organization/company name"` + Title string `name:"title" help:"Job title"` + URL string `name:"url" help:"Business homepage URL"` + Note string `name:"note" help:"Personal note"` + Custom []string `name:"custom" help:"Custom field key=value (repeat or comma-separate)"` + User string `name:"user" help:"App-only target user override (UPN or user ID)"` } func (c *ContactsUpdateCmd) Run(ctx context.Context) error { @@ -139,8 +169,22 @@ func (c *ContactsUpdateCmd) Run(ctx context.Context) error { if strings.TrimSpace(c.MobilePhone) != "" { payload["mobilePhone"] = c.MobilePhone } - if len(payload) == 0 { - return usage("provide at least one field to update") + if strings.TrimSpace(c.Org) != "" { + payload["companyName"] = strings.TrimSpace(c.Org) + } + if strings.TrimSpace(c.Title) != "" { + payload["jobTitle"] = strings.TrimSpace(c.Title) + } + if strings.TrimSpace(c.URL) != "" { + payload["businessHomePage"] = strings.TrimSpace(c.URL) + } + if strings.TrimSpace(c.Note) != "" { + payload["personalNotes"] = strings.TrimSpace(c.Note) + } + + customFields, err := contacts.ParseCustomFields(c.Custom) + if err != nil { + return usage(err.Error()) } rt, err := resolveRuntime(ctx, capContactsUpdate) @@ -151,7 +195,18 @@ func (c *ContactsUpdateCmd) Run(ctx context.Context) error { if err != nil { return err } - if err := contacts.New(rt.Graph, targetUser).Update(ctx, c.ID, payload); err != nil { + svc := contacts.New(rt.Graph, targetUser) + if len(customFields) > 0 { + existing, getErr := svc.Get(ctx, c.ID) + if getErr != nil { + return getErr + } + payload["categories"] = contacts.MergeCustomFieldCategories(existing["categories"], customFields) + } + if len(payload) == 0 { + return usage("provide at least one field to update") + } + if err := svc.Update(ctx, c.ID, payload); err != nil { return err } diff --git a/internal/cmd/contacts_flags_test.go b/internal/cmd/contacts_flags_test.go new file mode 100644 index 0000000..0ebbc42 --- /dev/null +++ b/internal/cmd/contacts_flags_test.go @@ -0,0 +1,80 @@ +package cmd + +import ( + "reflect" + "testing" +) + +func TestContactsCreateExtendedFlagsParse(t *testing.T) { + parser, cli, err := newParser("test") + if err != nil { + t.Fatalf("newParser failed: %v", err) + } + + args := []string{ + "contacts", "create", + "--display-name", "Jane Doe", + "--email", "jane@example.com", + "--mobile-phone", "+1-555-0100", + "--org", "Contoso", + "--title", "Engineer", + "--url", "https://example.com", + "--note", "Team lead", + "--custom", "team=platform", + "--custom", "region=NA", + } + if _, err := parser.Parse(args); err != nil { + t.Fatalf("parse failed: %v", err) + } + + if cli.Contacts.Create.Org != "Contoso" { + t.Fatalf("unexpected org: %q", cli.Contacts.Create.Org) + } + if cli.Contacts.Create.Title != "Engineer" { + t.Fatalf("unexpected title: %q", cli.Contacts.Create.Title) + } + if cli.Contacts.Create.URL != "https://example.com" { + t.Fatalf("unexpected url: %q", cli.Contacts.Create.URL) + } + if cli.Contacts.Create.Note != "Team lead" { + t.Fatalf("unexpected note: %q", cli.Contacts.Create.Note) + } + if !reflect.DeepEqual(cli.Contacts.Create.Custom, []string{"team=platform", "region=NA"}) { + t.Fatalf("unexpected custom flags: %#v", cli.Contacts.Create.Custom) + } +} + +func TestContactsUpdateExtendedFlagsParse(t *testing.T) { + parser, cli, err := newParser("test") + if err != nil { + t.Fatalf("newParser failed: %v", err) + } + + args := []string{ + "contacts", "update", "contact-id", + "--org", "Contoso", + "--title", "Manager", + "--url", "https://example.com/profile", + "--note", "Updated notes", + "--custom", "tier=gold", + } + if _, err := parser.Parse(args); err != nil { + t.Fatalf("parse failed: %v", err) + } + + if cli.Contacts.Update.Org != "Contoso" { + t.Fatalf("unexpected org: %q", cli.Contacts.Update.Org) + } + if cli.Contacts.Update.Title != "Manager" { + t.Fatalf("unexpected title: %q", cli.Contacts.Update.Title) + } + if cli.Contacts.Update.URL != "https://example.com/profile" { + t.Fatalf("unexpected url: %q", cli.Contacts.Update.URL) + } + if cli.Contacts.Update.Note != "Updated notes" { + t.Fatalf("unexpected note: %q", cli.Contacts.Update.Note) + } + if !reflect.DeepEqual(cli.Contacts.Update.Custom, []string{"tier=gold"}) { + t.Fatalf("unexpected custom flags: %#v", cli.Contacts.Update.Custom) + } +} diff --git a/internal/services/contacts/custom_fields.go b/internal/services/contacts/custom_fields.go new file mode 100644 index 0000000..030575d --- /dev/null +++ b/internal/services/contacts/custom_fields.go @@ -0,0 +1,184 @@ +package contacts + +import ( + "fmt" + "net/url" + "sort" + "strings" +) + +const customCategoryPrefix = "mog.custom." + +type CustomField struct { + Key string `json:"key"` + Value string `json:"value"` +} + +func ParseCustomFields(values []string) ([]CustomField, error) { + dedup := map[string]string{} + + for _, raw := range values { + parts := strings.Split(raw, ",") + for _, part := range parts { + token := strings.TrimSpace(part) + if token == "" { + continue + } + + key, value, ok := strings.Cut(token, "=") + if !ok { + return nil, fmt.Errorf("invalid custom field %q (expected key=value)", token) + } + + key = strings.TrimSpace(key) + if key == "" { + return nil, fmt.Errorf("invalid custom field %q (key must not be empty)", token) + } + + dedup[key] = strings.TrimSpace(value) + } + } + + fields := make([]CustomField, 0, len(dedup)) + for key, value := range dedup { + fields = append(fields, CustomField{Key: key, Value: value}) + } + sortCustomFields(fields) + + return fields, nil +} + +func EncodeCustomFieldCategories(fields []CustomField) []string { + if len(fields) == 0 { + return nil + } + + normalized := make([]CustomField, len(fields)) + copy(normalized, fields) + sortCustomFields(normalized) + + out := make([]string, 0, len(normalized)) + for _, field := range normalized { + key := strings.TrimSpace(field.Key) + if key == "" { + continue + } + out = append(out, customCategoryPrefix+url.QueryEscape(key)+"="+url.QueryEscape(strings.TrimSpace(field.Value))) + } + + if len(out) == 0 { + return nil + } + return out +} + +func MergeCustomFieldCategories(existing any, fields []CustomField) []string { + keep := make([]string, 0) + for _, category := range parseCategories(existing) { + if _, ok := decodeCustomFieldCategory(category); ok { + continue + } + keep = append(keep, category) + } + + custom := EncodeCustomFieldCategories(fields) + if len(keep) == 0 && len(custom) == 0 { + return nil + } + + merged := make([]string, 0, len(keep)+len(custom)) + merged = append(merged, keep...) + merged = append(merged, custom...) + return merged +} + +func decodeCustomFields(categories any) []CustomField { + decoded := make([]CustomField, 0) + for _, category := range parseCategories(categories) { + field, ok := decodeCustomFieldCategory(category) + if ok { + decoded = append(decoded, field) + } + } + sortCustomFields(decoded) + return decoded +} + +func parseCategories(value any) []string { + if value == nil { + return nil + } + + switch typed := value.(type) { + case []string: + out := make([]string, 0, len(typed)) + for _, category := range typed { + category = strings.TrimSpace(category) + if category != "" { + out = append(out, category) + } + } + return out + case []any: + out := make([]string, 0, len(typed)) + for _, raw := range typed { + if category, ok := raw.(string); ok { + category = strings.TrimSpace(category) + if category != "" { + out = append(out, category) + } + } + } + return out + default: + return nil + } +} + +func decodeCustomFieldCategory(category string) (CustomField, bool) { + category = strings.TrimSpace(category) + if !strings.HasPrefix(category, customCategoryPrefix) { + return CustomField{}, false + } + + raw := strings.TrimPrefix(category, customCategoryPrefix) + rawKey, rawValue, ok := strings.Cut(raw, "=") + if !ok { + return CustomField{}, false + } + + key, keyErr := url.QueryUnescape(strings.TrimSpace(rawKey)) + value, valueErr := url.QueryUnescape(strings.TrimSpace(rawValue)) + if keyErr != nil || valueErr != nil { + return CustomField{}, false + } + + key = strings.TrimSpace(key) + if key == "" { + return CustomField{}, false + } + + return CustomField{Key: key, Value: strings.TrimSpace(value)}, true +} + +func addNormalizedCustomFields(item map[string]any) { + if item == nil { + return + } + + fields := decodeCustomFields(item["categories"]) + if len(fields) == 0 { + return + } + + item["custom"] = fields +} + +func sortCustomFields(fields []CustomField) { + sort.Slice(fields, func(i, j int) bool { + if fields[i].Key == fields[j].Key { + return fields[i].Value < fields[j].Value + } + return fields[i].Key < fields[j].Key + }) +} diff --git a/internal/services/contacts/custom_fields_test.go b/internal/services/contacts/custom_fields_test.go new file mode 100644 index 0000000..e8541be --- /dev/null +++ b/internal/services/contacts/custom_fields_test.go @@ -0,0 +1,69 @@ +package contacts + +import ( + "reflect" + "testing" +) + +func TestParseCustomFieldsSortsAndDeduplicates(t *testing.T) { + fields, err := ParseCustomFields([]string{ + "tier=gold", + "region=NA,team=platform", + "tier=platinum", + }) + if err != nil { + t.Fatalf("ParseCustomFields failed: %v", err) + } + + want := []CustomField{ + {Key: "region", Value: "NA"}, + {Key: "team", Value: "platform"}, + {Key: "tier", Value: "platinum"}, + } + if !reflect.DeepEqual(fields, want) { + t.Fatalf("unexpected fields: got %#v want %#v", fields, want) + } +} + +func TestParseCustomFieldsRejectsInvalidInput(t *testing.T) { + _, err := ParseCustomFields([]string{"not-a-pair"}) + if err == nil { + t.Fatal("expected ParseCustomFields to fail for invalid input") + } +} + +func TestMergeCustomFieldCategoriesPreservesNonCustomValues(t *testing.T) { + existing := append([]string{"Friends"}, EncodeCustomFieldCategories([]CustomField{{Key: "old", Value: "1"}})...) + merged := MergeCustomFieldCategories(existing, []CustomField{{Key: "new", Value: "2"}}) + + want := append([]string{"Friends"}, EncodeCustomFieldCategories([]CustomField{{Key: "new", Value: "2"}})...) + if !reflect.DeepEqual(merged, want) { + t.Fatalf("unexpected merged categories: got %#v want %#v", merged, want) + } +} + +func TestAddNormalizedCustomFieldsAddsSortedCustomSlice(t *testing.T) { + item := map[string]any{ + "id": "contact-id", + "categories": []any{ + "Friends", + "mog.custom.team=platform", + "mog.custom.region=NA", + }, + } + + addNormalizedCustomFields(item) + + custom, ok := item["custom"].([]CustomField) + if !ok { + t.Fatalf("expected custom field slice, got %#v", item["custom"]) + } + + want := []CustomField{ + {Key: "region", Value: "NA"}, + {Key: "team", Value: "platform"}, + } + if !reflect.DeepEqual(custom, want) { + t.Fatalf("unexpected custom fields: got %#v want %#v", custom, want) + } +} diff --git a/internal/services/contacts/service.go b/internal/services/contacts/service.go index a56cb0d..694c804 100644 --- a/internal/services/contacts/service.go +++ b/internal/services/contacts/service.go @@ -28,7 +28,7 @@ func New(client *graph.Client, appOnlyUser string) *Service { func (s *Service) List(ctx context.Context, max int, page string) ([]map[string]any, string, error) { query := url.Values{} - query.Set("$select", "id,displayName,emailAddresses,mobilePhone") + query.Set("$select", "id,displayName,emailAddresses,mobilePhone,companyName,jobTitle,businessHomePage,personalNotes,categories") if max > 0 { query.Set("$top", fmt.Sprintf("%d", max)) } @@ -50,18 +50,23 @@ func (s *Service) List(ctx context.Context, max int, page string) ([]map[string] } trimmed, trimmedNext := trimPage(items, next, max) + for _, item := range trimmed { + addNormalizedCustomFields(item) + } return trimmed, trimmedNext, nil } func (s *Service) Get(ctx context.Context, id string) (map[string]any, error) { var payload map[string]any err := s.client.DoJSON(ctx, http.MethodGet, s.contactsEndpoint()+"/"+url.PathEscape(strings.TrimSpace(id)), nil, nil, getContactsScopes, &payload) + addNormalizedCustomFields(payload) return payload, err } func (s *Service) Create(ctx context.Context, payload map[string]any) (map[string]any, error) { var created map[string]any err := s.client.DoJSON(ctx, http.MethodPost, s.contactsEndpoint(), nil, payload, createContactsScopes, &created) + addNormalizedCustomFields(created) return created, err } diff --git a/internal/services/contacts/service_test.go b/internal/services/contacts/service_test.go index e0d66c6..83ea721 100644 --- a/internal/services/contacts/service_test.go +++ b/internal/services/contacts/service_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strings" "testing" "github.com/jaredpalmer/mogcli/internal/graph" @@ -49,6 +50,47 @@ func TestListUsesPageTokenURL(t *testing.T) { } } +func TestListSelectIncludesExtendedFieldsAndNormalizesCustom(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/me/contacts" { + t.Fatalf("unexpected path %s", r.URL.Path) + } + selectValue := r.URL.Query().Get("$select") + for _, required := range []string{"companyName", "jobTitle", "businessHomePage", "personalNotes", "categories"} { + if !strings.Contains(selectValue, required) { + t.Fatalf("expected $select to include %s, got %q", required, selectValue) + } + } + _, _ = fmt.Fprint(w, `{"value":[{"id":"1","categories":["mog.custom.team=platform","mog.custom.region=NA"]}]}`) + })) + defer server.Close() + + client := graph.NewClient(func(context.Context, []string) (string, error) { return "token", nil }) + client.BaseURL = server.URL + client.HTTPClient = server.Client() + + items, _, err := New(client, "").List(context.Background(), 10, "") + if err != nil { + t.Fatalf("List failed: %v", err) + } + if len(items) != 1 { + t.Fatalf("expected one contact, got %d", len(items)) + } + + custom, ok := items[0]["custom"].([]CustomField) + if !ok { + t.Fatalf("expected normalized custom fields, got %#v", items[0]["custom"]) + } + if len(custom) != 2 { + t.Fatalf("expected two custom fields, got %d", len(custom)) + } + if custom[0].Key != "region" || custom[1].Key != "team" { + t.Fatalf("expected deterministic custom order, got %#v", custom) + } +} + func TestEndpointsRouteByAuthMode(t *testing.T) { t.Parallel() From c7b8ea03be5ae17909f27ba47165664c2baaa3f4 Mon Sep 17 00:00:00 2001 From: Jared Palmer Date: Thu, 19 Feb 2026 10:56:39 -0500 Subject: [PATCH 4/5] Add quoted reply support to mail send command --- internal/cmd/mail.go | 114 +++++++++++++++++++++++++++++++++++++- internal/cmd/mail_test.go | 72 ++++++++++++++++++++++++ 2 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 internal/cmd/mail_test.go diff --git a/internal/cmd/mail.go b/internal/cmd/mail.go index a9b7418..fbd78a3 100644 --- a/internal/cmd/mail.go +++ b/internal/cmd/mail.go @@ -84,7 +84,8 @@ func (c *MailGetCmd) Run(ctx context.Context) error { type MailSendCmd struct { To []string `name:"to" required:"" help:"Recipient email (repeat or comma-separate)"` Subject string `name:"subject" required:"" help:"Email subject"` - Body string `name:"body" required:"" help:"Plain text body"` + Body string `name:"body" help:"Plain text body"` + Quote string `name:"quote" help:"Message ID to quote in reply body"` User string `name:"user" help:"App-only target user override (UPN or user ID)"` DryRun bool `name:"dry-run" help:"Preview send without sending the message"` } @@ -103,6 +104,9 @@ func (c *MailSendCmd) Run(ctx context.Context) error { if len(recipients) == 0 { return usage("at least one --to recipient is required") } + if strings.TrimSpace(c.Body) == "" && strings.TrimSpace(c.Quote) == "" { + return usage("--body is required unless --quote is provided") + } if c.DryRun { if outfmt.IsJSON(ctx) { return outfmt.WriteJSON(os.Stdout, map[string]any{ @@ -111,14 +115,28 @@ func (c *MailSendCmd) Run(ctx context.Context) error { "to": recipients, "subject": c.Subject, "body_len": len(c.Body), + "quote": strings.TrimSpace(c.Quote), }) } + if strings.TrimSpace(c.Quote) != "" { + fmt.Fprintf(os.Stdout, "Dry run: would send quoted reply to %s with subject %q (quote id: %s)\n", strings.Join(recipients, ", "), c.Subject, strings.TrimSpace(c.Quote)) + return nil + } fmt.Fprintf(os.Stdout, "Dry run: would send message to %s with subject %q\n", strings.Join(recipients, ", "), c.Subject) return nil } svc := mail.New(rt.Graph, targetUser) - if err := svc.Send(ctx, recipients, c.Subject, c.Body); err != nil { + body := strings.TrimSpace(c.Body) + quoteID := strings.TrimSpace(c.Quote) + if quoteID != "" { + source, getErr := svc.Get(ctx, quoteID) + if getErr != nil { + return getErr + } + body = composeQuotedReplyBody(body, source) + } + if err := svc.Send(ctx, recipients, c.Subject, body); err != nil { return err } @@ -149,3 +167,95 @@ func splitCSV(values []string) []string { return out } + +func composeQuotedReplyBody(body string, source map[string]any) string { + quoteBlock := buildQuoteBlock(source) + body = strings.TrimSpace(body) + if body == "" { + return quoteBlock + } + return body + "\n\n" + quoteBlock +} + +func buildQuoteBlock(source map[string]any) string { + sender := messageSender(source) + sent := strings.TrimSpace(stringValue(source["sentDateTime"])) + content := messageQuoteContent(source) + + header := "Quoted message:" + switch { + case sent != "" && sender != "": + header = fmt.Sprintf("On %s, %s wrote:", sent, sender) + case sender != "": + header = fmt.Sprintf("%s wrote:", sender) + case sent != "": + header = fmt.Sprintf("On %s:", sent) + } + + return header + "\n" + quoteLines(content) +} + +func messageSender(source map[string]any) string { + from, ok := source["from"].(map[string]any) + if !ok { + return "sender" + } + + emailAddress, ok := from["emailAddress"].(map[string]any) + if !ok { + return "sender" + } + + if address := strings.TrimSpace(stringValue(emailAddress["address"])); address != "" { + return address + } + if name := strings.TrimSpace(stringValue(emailAddress["name"])); name != "" { + return name + } + return "sender" +} + +func messageQuoteContent(source map[string]any) string { + if preview := strings.TrimSpace(stringValue(source["bodyPreview"])); preview != "" { + return preview + } + + body, ok := source["body"].(map[string]any) + if !ok { + return "(no message body)" + } + + content := strings.TrimSpace(stringValue(body["content"])) + if content == "" { + return "(no message body)" + } + + return content +} + +func quoteLines(content string) string { + content = strings.ReplaceAll(content, "\r\n", "\n") + lines := strings.Split(content, "\n") + if len(lines) == 0 { + return "> (no message body)" + } + + quoted := make([]string, 0, len(lines)) + for _, line := range lines { + if strings.TrimSpace(line) == "" { + quoted = append(quoted, ">") + continue + } + quoted = append(quoted, "> "+line) + } + return strings.Join(quoted, "\n") +} + +func stringValue(v any) string { + switch typed := v.(type) { + case string: + return typed + default: + return "" + } +} diff --git a/internal/cmd/mail_test.go b/internal/cmd/mail_test.go new file mode 100644 index 0000000..e299861 --- /dev/null +++ b/internal/cmd/mail_test.go @@ -0,0 +1,72 @@ +package cmd + +import ( + "strings" + "testing" +) + +func TestMailSendQuoteFlagParsesWithoutBody(t *testing.T) { + parser, cli, err := newParser("test") + if err != nil { + t.Fatalf("newParser failed: %v", err) + } + + args := []string{ + "mail", "send", + "--to", "dev@example.com", + "--subject", "Re: Hello", + "--quote", "message-id-123", + } + if _, err := parser.Parse(args); err != nil { + t.Fatalf("parse failed: %v", err) + } + + if cli.Mail.Send.Body != "" { + t.Fatalf("expected empty body, got %q", cli.Mail.Send.Body) + } + if cli.Mail.Send.Quote != "message-id-123" { + t.Fatalf("unexpected quote id: %q", cli.Mail.Send.Quote) + } +} + +func TestComposeQuotedReplyBodyAppendsQuoteBlock(t *testing.T) { + source := map[string]any{ + "sentDateTime": "2026-02-18T13:00:00Z", + "from": map[string]any{ + "emailAddress": map[string]any{ + "address": "sender@example.com", + }, + }, + "bodyPreview": "Hello there\nHow are you?", + } + + got := composeQuotedReplyBody("Thanks for the update.", source) + wantContains := []string{ + "Thanks for the update.", + "On 2026-02-18T13:00:00Z, sender@example.com wrote:", + "> Hello there", + "> How are you?", + } + for _, fragment := range wantContains { + if !strings.Contains(got, fragment) { + t.Fatalf("expected reply body to contain %q, got:\n%s", fragment, got) + } + } +} + +func TestComposeQuotedReplyBodyAllowsQuoteOnly(t *testing.T) { + source := map[string]any{ + "bodyPreview": "Only quoted text", + } + + got := composeQuotedReplyBody("", source) + if strings.Contains(got, "\n\n") { + t.Fatalf("expected quote-only body without leading empty section, got %q", got) + } + if !strings.Contains(got, "sender wrote:") { + t.Fatalf("expected quote header, got %q", got) + } + if !strings.Contains(got, "> Only quoted text") { + t.Fatalf("expected quoted message content, got %q", got) + } +} From 452af5cecd9e920e24975c55d750d88a5ea6a16e Mon Sep 17 00:00:00 2001 From: Jared Palmer Date: Thu, 19 Feb 2026 10:57:24 -0500 Subject: [PATCH 5/5] Document keyring, contacts, calendar, and mail quote enhancements --- README.md | 19 +++++++++++++++++++ docs/microsoft-port-plan.md | 8 ++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 904d214..0fff6e3 100644 --- a/README.md +++ b/README.md @@ -175,6 +175,7 @@ Mail: mog mail list --max 50 --query "from:alerts@example.com" mog mail get mog mail send --to dev@contoso.com --subject "Deploy complete" --body "Finished." +mog mail send --to dev@contoso.com --subject "Re: Deploy complete" --quote mog mail send --to dev@contoso.com --subject "Deploy complete" --body "Finished." --dry-run ``` @@ -195,6 +196,16 @@ Contacts: ```bash mog contacts list --max 100 mog contacts create --display-name "Jane Doe" --email "jane@contoso.com" +mog contacts create \ + --display-name "Jane Doe" \ + --email "jane@contoso.com" \ + --org "Contoso" \ + --title "Program Manager" \ + --url "https://contoso.example/jane" \ + --note "Customer success lead" \ + --custom region=NA \ + --custom team=platform +mog contacts update --title "Director" --custom region=EMEA ``` Groups: @@ -277,6 +288,14 @@ Current keys: Profile metadata is stored in config. Tokens and secrets are stored via keychain/keyring backends. +`keyring_backend` supports `auto`, `keychain`, and `file`. + +- `auto`: use native OS keychain when available, otherwise file backend. +- `keychain`: require native OS keychain support. +- `file`: store under the local mogcli keyring directory. + +`MOG_KEYRING_PASSWORD` is treated as explicitly configured even when empty, so headless runs do not implicitly prompt for keyring passwords. + ## Troubleshooting No active profile: diff --git a/docs/microsoft-port-plan.md b/docs/microsoft-port-plan.md index 967bfc1..1991545 100644 --- a/docs/microsoft-port-plan.md +++ b/docs/microsoft-port-plan.md @@ -1,10 +1,10 @@ # mogcli Microsoft Port Plan (Revision 2) -Date: 2026-02-12 +Date: 2026-02-19 Status: implementation in progress (auth wizard, progressive delegated consent, and selective app-only routing implemented) Audience: Codex agents implementing `mogcli` -## Implementation status update (2026-02-12) +## Implementation status update (2026-02-19) Implemented in codebase: @@ -20,6 +20,10 @@ Implemented in codebase: 10. Task mutation mutability error normalization in `internal/services/tasks/service.go` for built-in/well-known Microsoft To Do list constraints. 11. `--dry-run` previews for write operations in mail, calendar, and onedrive command surfaces. 12. OneDrive local path hardening (`SafeExpandPath`), metadata-rich delete confirmation, and progress output for local file transfers. +13. Keyring backend hardening: `auto|keychain|file` resolution, native macOS keychain support, and explicit empty `MOG_KEYRING_PASSWORD` handling for headless contexts. +14. Calendar update patch safety: attendee updates are split from reminder-field patches to avoid Graph validation conflicts. +15. Contacts create/update now supports richer fields (`org`, `title`, `url`, `note`, `custom`) with deterministic custom-field ordering in command output. +16. Mail send supports quoted replies via `--quote ` to include source message context automatically. ## 0. Critique of the prior plan