From 3ec862f7ca3fd6ae295b05806f855126dd8e9150 Mon Sep 17 00:00:00 2001 From: akarineren Date: Mon, 27 Jul 2026 19:19:30 +0900 Subject: [PATCH 1/2] Refactor Sync and Ingest methods to clarify API key requirements; enhance tests for offline scanning --- internal/cli/cli.go | 21 ++++++++++----------- internal/cli/cli_test.go | 25 ++++++++++++++++++++++--- pkg/agentlib/agentlib.go | 11 ++++++++++- pkg/agentlib/agentlib_test.go | 13 +++++++++---- 4 files changed, 51 insertions(+), 19 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 4f35843..7b0561f 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -57,26 +57,25 @@ func (a *App) Sync(ctx context.Context) error { return a.Upload(ctx) } -// Ingest scans the selected providers into the shared local queue. It writes -// the database, so the caller holds the data lock. +// Ingest scans the selected providers into the shared local queue. Scanning +// is pure local work and needs no API key: events queue in the database until +// a key exists to upload them. It writes the database, so the caller holds +// the data lock. func (a *App) Ingest() error { - settings, err := a.Agent.Settings() - if err != nil { - return err - } - if settings.APIKey == "" { - return errors.New("API key is required in ~/.tokitoki/api_key") - } - _, err = a.Scanner.Scan(a.ProviderDirs) + _, err := a.Scanner.Scan(a.ProviderDirs) return err } -// Upload drains queued events to the server. +// Upload drains queued events to the server — the half of a sync that needs +// the API key. func (a *App) Upload(ctx context.Context) error { settings, err := a.Agent.Settings() if err != nil { return err } + if settings.APIKey == "" { + return errors.New("API key is required in ~/.tokitoki/api_key") + } if err := usageupload.SyncPending(ctx, settings, a.UsageDB); err != nil { return err } diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index d2b5eb8..ccaf50c 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -10,8 +10,12 @@ import ( "github.com/tokitoki-dev/tokitoki-cli/internal/agent" "github.com/tokitoki-dev/tokitoki-cli/internal/store" + "github.com/tokitoki-dev/tokitoki-cli/internal/usagedb" + "github.com/tokitoki-dev/tokitoki-cli/internal/usagescan" ) +// Scanning is offline work, so a missing API key only stops the upload half: +// Sync still ingests, then fails with the key requirement. func TestSyncRequiresAPIKey(t *testing.T) { app := newApp(t) err := app.Sync(context.Background()) @@ -20,6 +24,13 @@ func TestSyncRequiresAPIKey(t *testing.T) { } } +func TestIngestWorksWithoutAPIKey(t *testing.T) { + app := newApp(t) + if err := app.Ingest(); err != nil { + t.Fatalf("Ingest() without API key = %v, want offline scan to succeed", err) + } +} + func TestSetAPIKeyPersistsSettings(t *testing.T) { app := newApp(t) if err := app.SetAPIKey("tokitoki_test_key"); err != nil { @@ -61,12 +72,20 @@ func TestGetAPIKeyRequiresConfiguredKey(t *testing.T) { func newApp(t *testing.T) *App { t.Helper() - fileStore, err := store.Open(t.TempDir()) + dataDir := t.TempDir() + fileStore, err := store.Open(dataDir) + if err != nil { + t.Fatal(err) + } + usageDB, err := usagedb.Open(store.UsageDBPath(dataDir)) if err != nil { t.Fatal(err) } + t.Cleanup(func() { _ = usageDB.Close() }) return &App{ - Agent: agent.New(fileStore, slog.New(slog.NewTextHandler(io.Discard, nil))), - Out: &bytes.Buffer{}, + Agent: agent.New(fileStore, slog.New(slog.NewTextHandler(io.Discard, nil))), + UsageDB: usageDB, + Scanner: usagescan.New(usageDB), + Out: &bytes.Buffer{}, } } diff --git a/pkg/agentlib/agentlib.go b/pkg/agentlib/agentlib.go index fa318c1..aae1112 100644 --- a/pkg/agentlib/agentlib.go +++ b/pkg/agentlib/agentlib.go @@ -239,7 +239,9 @@ func (c *Client) VerifyAPIKey(ctx context.Context) (bool, error) { return deviceauth.VerifyKey(ctx, usageupload.BaseURL(), apiKey) } -// Sync scans selected provider directories and uploads newly discovered events. +// Sync scans selected provider directories and uploads newly discovered +// events. Scanning is local and always runs; without a configured API key the +// events simply stay queued and upload resumes once a key is saved. func (c *Client) Sync(ctx context.Context, options SyncOptions) error { providerDirs := normalizeProviderDirs(options.ProviderDirs) if len(providerDirs) == 0 { @@ -275,6 +277,13 @@ func (c *Client) Sync(ctx context.Context, options SyncOptions) error { if err := c.withDataLock(app.Ingest); err != nil { return err } + if _, err := c.GetAPIKey(); err != nil { + if errors.Is(err, ErrMissingAPIKey) { + c.logger.Debug("skip upload; API key is not configured") + return nil + } + return err + } return c.withUploadLock(func() error { return app.Upload(ctx) }) } diff --git a/pkg/agentlib/agentlib_test.go b/pkg/agentlib/agentlib_test.go index 335ad03..8ece8fc 100644 --- a/pkg/agentlib/agentlib_test.go +++ b/pkg/agentlib/agentlib_test.go @@ -5,10 +5,10 @@ import ( "errors" "os" "path/filepath" - "strings" "testing" "github.com/tokitoki-dev/tokitoki-cli/internal/config" + "github.com/tokitoki-dev/tokitoki-cli/internal/store" ) func TestNewUsesDefaultDataDir(t *testing.T) { @@ -115,15 +115,20 @@ func TestDefaultProviderDirsIncludesBuiltInProviders(t *testing.T) { } } -func TestSyncRequiresAPIKey(t *testing.T) { +// Scanning is offline; a missing API key only means the upload half is +// skipped, so Sync succeeds and events queue locally for later. +func TestSyncWithoutAPIKeyScansOffline(t *testing.T) { client := newTestClient(t) claudeDir := t.TempDir() err := client.Sync(context.Background(), SyncOptions{ ProviderDirs: map[Provider][]string{ProviderClaude: {claudeDir}}, }) - if err == nil || !strings.Contains(err.Error(), "API key is required") { - t.Fatalf("Sync() error = %v, want API key requirement", err) + if err != nil { + t.Fatalf("Sync() without API key = %v, want offline scan to succeed", err) + } + if _, err := os.Stat(store.UsageDBPath(client.DataDir())); err != nil { + t.Fatalf("usage database missing after offline sync: %v", err) } } From bd6f83952076977563f83f3deab49048e3f77cfc Mon Sep 17 00:00:00 2001 From: akarineren Date: Mon, 27 Jul 2026 20:35:12 +0900 Subject: [PATCH 2/2] Queue heartbeats without an API key; align Sync semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SendHeartbeat checked the API key before InsertEvents, so an editor that started sending heartbeats before the user signed in dropped every event instead of queueing it. Insert first, then skip only the upload half when no key exists — the same rule Ingest/Upload already follow. cli.App.Sync returned an error without a key while agentlib.Client.Sync returned nil; both now skip the upload and succeed. --- internal/cli/cli.go | 11 +++++++++++ internal/cli/cli_test.go | 20 ++++++++++++++------ pkg/agentlib/agentlib.go | 22 ++++++++++++++-------- pkg/agentlib/agentlib_test.go | 30 ++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 14 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 7b0561f..a1f61b1 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -47,6 +47,10 @@ func (a *App) GetAPIKey() error { // emitting counts or summaries: success only means the local files were // processed and the server accepted the request. // +// Without a configured API key the scan still runs and its events stay queued; +// only the upload half is skipped, so a user who has not signed in yet keeps +// accumulating history instead of losing it. +// // Callers that coordinate multiple processes call the two phases separately — // Ingest under the data lock, Upload under the upload lock — so a slow drain // never blocks another process's ingestion. @@ -54,6 +58,13 @@ func (a *App) Sync(ctx context.Context) error { if err := a.Ingest(); err != nil { return err } + settings, err := a.Agent.Settings() + if err != nil { + return err + } + if settings.APIKey == "" { + return nil + } return a.Upload(ctx) } diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index ccaf50c..dd8481b 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -14,13 +14,12 @@ import ( "github.com/tokitoki-dev/tokitoki-cli/internal/usagescan" ) -// Scanning is offline work, so a missing API key only stops the upload half: -// Sync still ingests, then fails with the key requirement. -func TestSyncRequiresAPIKey(t *testing.T) { +// Scanning is offline work, so a missing API key only skips the upload half: +// Sync ingests and returns cleanly, leaving events queued for a later run. +func TestSyncWithoutAPIKeyScansOffline(t *testing.T) { app := newApp(t) - err := app.Sync(context.Background()) - if err == nil || !strings.Contains(err.Error(), "API key is required") { - t.Fatalf("Sync() error = %v, want API key requirement", err) + if err := app.Sync(context.Background()); err != nil { + t.Fatalf("Sync() without API key = %v, want offline scan to succeed", err) } } @@ -31,6 +30,15 @@ func TestIngestWorksWithoutAPIKey(t *testing.T) { } } +// Upload is the half that needs the key, so calling it directly still fails. +func TestUploadRequiresAPIKey(t *testing.T) { + app := newApp(t) + err := app.Upload(context.Background()) + if err == nil || !strings.Contains(err.Error(), "API key is required") { + t.Fatalf("Upload() error = %v, want API key requirement", err) + } +} + func TestSetAPIKeyPersistsSettings(t *testing.T) { app := newApp(t) if err := app.SetAPIKey("tokitoki_test_key"); err != nil { diff --git a/pkg/agentlib/agentlib.go b/pkg/agentlib/agentlib.go index aae1112..58958ff 100644 --- a/pkg/agentlib/agentlib.go +++ b/pkg/agentlib/agentlib.go @@ -277,13 +277,14 @@ func (c *Client) Sync(ctx context.Context, options SyncOptions) error { if err := c.withDataLock(app.Ingest); err != nil { return err } - if _, err := c.GetAPIKey(); err != nil { - if errors.Is(err, ErrMissingAPIKey) { - c.logger.Debug("skip upload; API key is not configured") - return nil - } + settings, err := agent.New(fileStore, c.logger).Settings() + if err != nil { return err } + if settings.APIKey == "" { + c.logger.Debug("skip upload; API key is not configured") + return nil + } return c.withUploadLock(func() error { return app.Upload(ctx) }) } @@ -363,6 +364,10 @@ func (c *Client) SendHeartbeat(ctx context.Context, heartbeat Heartbeat) error { // Queue the event under the data lock, then drain under the upload lock. // The drain can take the whole network timeout; heartbeats from other // editors must be able to enqueue while it runs, not wait behind it. + // + // Queueing is local work and never depends on the API key: an editor that + // starts sending heartbeats before the user configures one must not drop + // them. The key only gates the upload half below. var settings agent.Settings if err := c.withDataLock(func() error { fileStore, err := store.Open(c.dataDir) @@ -373,15 +378,16 @@ func (c *Client) SendHeartbeat(ctx context.Context, heartbeat Heartbeat) error { if err != nil { return err } - if settings.APIKey == "" { - return ErrMissingAPIKey - } _, err = usageDB.InsertEvents([]usage.Entry{entry}) return err }); err != nil { return err } + if settings.APIKey == "" { + c.logger.Debug("skip upload; API key is not configured") + return nil + } return c.withUploadLock(func() error { return usageupload.SyncPending(ctx, settings, usageDB) }) diff --git a/pkg/agentlib/agentlib_test.go b/pkg/agentlib/agentlib_test.go index 8ece8fc..cc3dfd3 100644 --- a/pkg/agentlib/agentlib_test.go +++ b/pkg/agentlib/agentlib_test.go @@ -6,9 +6,11 @@ import ( "os" "path/filepath" "testing" + "time" "github.com/tokitoki-dev/tokitoki-cli/internal/config" "github.com/tokitoki-dev/tokitoki-cli/internal/store" + "github.com/tokitoki-dev/tokitoki-cli/internal/usagedb" ) func TestNewUsesDefaultDataDir(t *testing.T) { @@ -132,6 +134,34 @@ func TestSyncWithoutAPIKeyScansOffline(t *testing.T) { } } +// An editor may start sending heartbeats before the user signs in. The event +// must still be queued locally; only the upload is skipped. +func TestSendHeartbeatWithoutAPIKeyQueuesEvent(t *testing.T) { + client := newTestClient(t) + + err := client.SendHeartbeat(context.Background(), Heartbeat{ + Entity: filepath.Join(t.TempDir(), "main.go"), + Editor: "vscode", + }) + if err != nil { + t.Fatalf("SendHeartbeat() without API key = %v, want queued event", err) + } + + usageDB, err := usagedb.Open(store.UsageDBPath(client.DataDir())) + if err != nil { + t.Fatal(err) + } + defer usageDB.Close() + + pending, err := usageDB.PendingEvents(time.Now(), 0) + if err != nil { + t.Fatal(err) + } + if len(pending) != 1 { + t.Fatalf("pending events = %d, want 1 queued heartbeat", len(pending)) + } +} + func TestApplyProjectFileOverridesHeartbeatIdentity(t *testing.T) { projectDir := filepath.Join(t.TempDir(), "local-checkout") entity := filepath.Join(projectDir, "src", "main.go")