Pick the first claude account that is under its limits

README.md

@@ -56,7 +56,7 @@ claude_config_dir = "~/.claude-work"
 claude_fallback_config_dirs = ["~/.claude"]
 ```
 
-When claude refuses a background review because its account is over a usage limit, or cuts one short because the account reached its limit partway through, and the session has posted nothing, docket stops that session and starts the review again under the next account in the list. New reviews then run under that account until it hits a limit too, and docket then moves on to the next account whose limit has reset, coming back round to the first. When every account is over its limit, the review waits under "Did not start" with the time the first limit resets, and `enter` starts it again. A review you run in this terminal shows you claude's refusal itself. docket records the limit when the session ends, so `u` and the next review move to another account. docket keeps the limits claude reported in `~/.docket/limits.jsonl`. claude counts usage per login, so a limit on one directory also holds every other directory signed in to the same login, and docket skips them all. Listing two directories signed in to one login therefore adds nothing to fall back to.
+When claude refuses a background review because its account is over a usage limit, or cuts one short because the account reached its limit partway through, and the session has posted nothing, docket stops that session and starts the review again under the first account in the list that is not over a limit. Each new review runs under the first account in the list that is not over a limit, so docket goes back to an earlier account as soon as its limit resets. When every account is over its limit, the review waits under "Did not start" with the time the first limit resets, and `enter` starts it again. A review you run in this terminal shows you claude's refusal itself. docket records the limit when the session ends, so `u` and the next review move to another account. docket keeps the limits claude reported in `~/.docket/limits.jsonl`. claude counts usage per login, so a limit on one directory also holds every other directory signed in to the same login, and docket skips them all. Listing two directories signed in to one login therefore adds nothing to fall back to.
 
 ## Review requests
 

internal/core/account/account.go

@@ -1,6 +1,6 @@
 // Package account picks the claude account a new session runs under. docket
-// rotates through the accounts config.toml names, and moves past an account once
-// claude refuses one of its sessions for a usage limit. claude counts usage per
+// takes the first account config.toml names, and passes over an account while
+// a usage limit that claude reported for it is in force. claude counts usage per
 // login. A limit therefore applies to every account dir signed in to that login.
 package account
 
@@ -9,7 +9,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
-	"slices"
 	"time"
 )
 
@@ -66,44 +65,16 @@ func (c Choice) Limited() bool {
 	return !c.Until.IsZero()
 }
 
-// Pick chooses the account for a new session. accounts is the rotation order.
+// Pick chooses the account for a new session. accounts is the priority order.
 // logins maps each account to the login it is signed in to. An account that
 // logins leaves out has an unknown login. limits is every limit docket has
-// recorded, in any order. A limit that applies to no account in the rotation
-// counts for nothing.
+// recorded, in any order.
 //
-// The rotation starts after the first account that the most recent limit
-// applies to, passes every account that limit applies to, and takes the first
-// account that is not over a limit. docket therefore stays on an account until
-// that account hits a limit, even after an earlier account's limit resets.
-// Before any account has hit a limit, the pick is the first.
+// The pick is the first account that is not over a limit. docket therefore
+// returns to an earlier account as soon as its limit resets.
 func Pick(accounts []string, logins map[string]Login, limits []Limit, now time.Time) Choice {
-	if len(accounts) == 0 {
-		return Choice{}
-	}
-	start := 0
-	var latest Limit
-	for _, limit := range limits {
-		i := slices.IndexFunc(accounts, func(account string) bool {
-			return limit.appliesTo(account, logins[account])
-		})
-		if i >= 0 && limit.At.After(latest.At) {
-			latest, start = limit, i+1
-		}
-	}
-	// The latest limit also covers every later dir on its login. A start on one of
-	// those would move docket back to that login once the limit resets.
-	for range accounts {
-		candidate := accounts[start%len(accounts)]
-		if latest.At.IsZero() || !latest.appliesTo(candidate, logins[candidate]) {
-			break
-		}
-		start++
-	}
-
 	var soonest Choice
-	for step := range len(accounts) {
-		candidate := accounts[(start+step)%len(accounts)]
+	for _, candidate := range accounts {
 		until := reset(candidate, logins[candidate], limits)
 		if !until.After(now) {
 			return Choice{Account: candidate}

internal/core/account/account_test.go

@@ -24,28 +24,22 @@ func at(minutes int) time.Time { return t0.Add(time.Duration(minutes) * time.Min
 
 // The user's own setup: an automation account first, and the default account
 // as the fallback.
-func TestPickStaysOnAnAccountUntilItHitsALimit(t *testing.T) {
+func TestPickReturnsToAnEarlierAccountOnceItsLimitResets(t *testing.T) {
 	accounts := []string{automation, defaultAccount}
 	t1 := at(0)
-	t2 := t1.Add(8 * 24 * time.Hour)
 	var limits []Limit
 
 	if got := Pick(accounts, nil, limits, t1.Add(-time.Hour)); got != (Choice{Account: automation}) {
 		t.Fatalf("with no limits, pick = %+v, want the automation account", got)
 	}
 
-	limits = append(limits, Limit{Account: automation, At: t1, Until: t1.Add(7 * 24 * time.Hour)})
+	limits = append(limits, Limit{Account: automation, At: t1, Until: t1.Add(5 * time.Hour)})
 	if got := Pick(accounts, nil, limits, t1.Add(time.Minute)); got != (Choice{Account: defaultAccount}) {
 		t.Fatalf("after automation hit its limit, pick = %+v, want the default account", got)
 	}
 
-	if got := Pick(accounts, nil, limits, t1.Add(7*24*time.Hour+time.Hour)); got != (Choice{Account: defaultAccount}) {
-		t.Fatalf("after automation's limit reset, pick = %+v, want docket to stay on the default account", got)
-	}
-
-	limits = append(limits, Limit{Account: defaultAccount, At: t2, Until: t2.Add(5 * time.Hour)})
-	if got := Pick(accounts, nil, limits, t2.Add(time.Minute)); got != (Choice{Account: automation}) {
-		t.Fatalf("after the default account hit its limit, pick = %+v, want the automation account again", got)
+	if got := Pick(accounts, nil, limits, t1.Add(5*time.Hour)); got != (Choice{Account: automation}) {
+		t.Fatalf("after automation's limit reset, pick = %+v, want the automation account again", got)
 	}
 }
 
@@ -121,10 +115,9 @@ func TestPickWithASingleAccount(t *testing.T) {
 	}
 }
 
-// Each case lists the accounts in rotation order. The rotation starts after the
-// account that hit a limit most recently, wraps past the end, and skips every
-// account still over a limit.
-func TestPickRotatesFromTheMostRecentlyLimitedAccount(t *testing.T) {
+// Each case lists the accounts in priority order. The pick is the first account
+// that is under its limits, whichever account hit a limit most recently.
+func TestPickTakesTheFirstAccountUnderItsLimits(t *testing.T) {
 	accounts := []string{automation, defaultAccount, work}
 	for _, tc := range []struct {
 		name   string
@@ -144,51 +137,50 @@ func TestPickRotatesFromTheMostRecentlyLimitedAccount(t *testing.T) {
 				{Account: defaultAccount, At: at(20), Until: at(30)},
 			},
 			now:  at(60),
-			want: work,
+			want: automation,
 		},
 		{
-			name: "the next account is free",
+			name: "the first account is limited",
 			limits: []Limit{
-				{Account: defaultAccount, At: at(0), Until: at(600)},
+				{Account: automation, At: at(0), Until: at(600)},
 			},
 			now:  at(1),
-			want: work,
+			want: defaultAccount,
 		},
 		{
-			name: "the rotation wraps past the last account",
+			name: "a later account is limited",
 			limits: []Limit{
-				{Account: work, At: at(0), Until: at(600)},
+				{Account: defaultAccount, At: at(0), Until: at(600)},
 			},
 			now:  at(1),
 			want: automation,
 		},
 		{
-			name: "the next account is still limited",
+			name: "only the last account is free",
 			limits: []Limit{
-				{Account: work, At: at(0), Until: at(600)},
+				{Account: automation, At: at(0), Until: at(600)},
 				{Account: defaultAccount, At: at(5), Until: at(600)},
 			},
 			now:  at(10),
-			want: automation,
+			want: work,
 		},
 		{
-			name: "the most recently limited account is checked last",
+			name: "the most recently limited account has reset",
 			limits: []Limit{
-				{Account: automation, At: at(0), Until: at(600)},
-				{Account: defaultAccount, At: at(5), Until: at(600)},
-				{Account: work, At: at(10), Until: at(20)},
+				{Account: work, At: at(0), Until: at(600)},
+				{Account: automation, At: at(10), Until: at(20)},
 			},
 			now:  at(30),
-			want: work,
+			want: automation,
 		},
 		{
-			name: "limits recorded out of order",
+			name: "an earlier account reset while a later one is still limited",
 			limits: []Limit{
-				{Account: automation, At: at(10), Until: at(20)},
-				{Account: work, At: at(0), Until: at(600)},
+				{Account: automation, At: at(0), Until: at(20)},
+				{Account: defaultAccount, At: at(10), Until: at(600)},
 			},
 			now:  at(30),
-			want: defaultAccount,
+			want: automation,
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
@@ -202,19 +194,19 @@ func TestPickRotatesFromTheMostRecentlyLimitedAccount(t *testing.T) {
 }
 
 // config.toml can drop an account that docket recorded a limit for. That limit
-// must neither move the rotation nor offer a reset for an account docket no
-// longer runs.
+// must neither block an account docket runs nor offer a reset for an account
+// docket no longer runs.
 func TestPickIgnoresLimitsOnAccountsOutsideTheRotation(t *testing.T) {
 	accounts := []string{automation, defaultAccount, work}
 
 	t.Run("for the order", func(t *testing.T) {
 		limits := []Limit{
-			{Account: defaultAccount, At: at(0), Until: at(10)},
+			{Account: automation, At: at(0), Until: at(600)},
 			{Account: personal, At: at(20), Until: at(600)},
 		}
 
-		if got := Pick(accounts, nil, limits, at(30)); got != (Choice{Account: work}) {
-			t.Errorf("pick = %+v, want work, after the default account that was limited last", got)
+		if got := Pick(accounts, nil, limits, at(30)); got != (Choice{Account: defaultAccount}) {
+			t.Errorf("pick = %+v, want the default account, the first one under its limits", got)
 		}
 	})
 
@@ -483,9 +475,9 @@ func TestALimitMatchesByDirWhenEitherLoginIsUnknown(t *testing.T) {
 	}
 }
 
-// The rotation starts past every dir that the most recent limit applies to. docket
-// therefore stays on the dir it moved to after that limit resets.
-func TestPickStaysOffALoginAfterItsLimitResets(t *testing.T) {
+// Once a login's limit resets, the first dir signed in to it is free again,
+// wherever the other login sits in the order.
+func TestPickReturnsToALoginAfterItsLimitResets(t *testing.T) {
 	for _, tc := range []struct {
 		name     string
 		accounts []string
@@ -498,26 +490,26 @@ func TestPickStaysOffALoginAfterItsLimitResets(t *testing.T) {
 			logins := map[string]Login{automation: team, defaultAccount: team, personal: ownOrg}
 			limits := []Limit{{Account: defaultAccount, Login: team, At: at(0), Until: at(10)}}
 
-			if got := Pick(tc.accounts, logins, limits, at(30)); got != (Choice{Account: personal}) {
-				t.Errorf("pick = %+v, want personal, which never hit a limit", got)
+			if got := Pick(tc.accounts, logins, limits, at(30)); got != (Choice{Account: tc.accounts[0]}) {
+				t.Errorf("pick = %+v, want %q, the first account", got, tc.accounts[0])
 			}
 		})
 	}
 }
 
 // A limit on a dir that the user has since signed in to another login applies
-// to no dir in the rotation. It must not move the rotation.
+// to no dir in the rotation. It must not block that dir.
 func TestPickIgnoresALimitThatAppliesToNoDirInTheRotation(t *testing.T) {
 	accounts := []string{automation, personal}
 	logins := map[string]Login{automation: team, personal: ownOrg}
 	other := Login{AccountUUID: "ef2621bd-2fb9-4866-8453-6b8e4875b115", OrganizationUUID: "2bb69dd2-45b3-40ef-80fe-d40827cc4cd5"}
 	limits := []Limit{
-		{Account: automation, Login: team, At: at(0), Until: at(10)},
+		{Account: automation, Login: team, At: at(0), Until: at(600)},
 		{Account: personal, Login: other, At: at(20), Until: at(600)},
 	}
 
 	if got := Pick(accounts, logins, limits, at(30)); got != (Choice{Account: personal}) {
-		t.Errorf("pick = %+v, want personal, after automation, which was limited last", got)
+		t.Errorf("pick = %+v, want personal, whose old login's limit is not its own", got)
 	}
 }
 

internal/core/config/config.go

@@ -46,8 +46,8 @@ type Config struct {
 	// therefore turns ~/.claude in the file into "". It keeps a CLAUDE_CONFIG_DIR
 	// that docket inherited as given, because claude reads that value as given.
 	ClaudeConfigDir string `toml:"claude_config_dir"`
-	// ClaudeFallbackConfigDirs are the accounts a new review moves to once
-	// claude refuses a session for a usage limit, in the order docket tries
+	// ClaudeFallbackConfigDirs are the accounts a new review runs under while
+	// every account before it is over a usage limit, in the order docket tries
 	// them. Load turns ~/.claude into "" here too.
 	ClaudeFallbackConfigDirs []string `toml:"claude_fallback_config_dirs"`
 	// ClaudeDefaultDir is DefaultClaudeConfigDir with the tilde expanded. Load
@@ -157,8 +157,8 @@ func (c Config) claudeDir(configDir, name string) string {
 	return filepath.Join(dir, name)
 }
 
-// ClaudeAccounts are the accounts docket rotates new claude sessions through:
-// ClaudeConfigDir first, then each fallback, each account once.
+// ClaudeAccounts are the accounts new claude sessions run under, in priority
+// order: ClaudeConfigDir first, then each fallback, each account once.
 func (c Config) ClaudeAccounts() []string {
 	accounts := []string{c.ClaudeConfigDir}
 	for _, dir := range c.ClaudeFallbackConfigDirs {

internal/core/session/limit_test.go

@@ -119,17 +119,21 @@ func TestAPollStartsAReviewRefusedForUsageAgainUnderTheFallbackAccount(t *testin
 	}
 }
 
-// docket stays on the fallback after the poll moves a review there, until the
-// fallback hits a limit of its own.
-func TestANewReviewRunsUnderTheAccountTheRotationMovedTo(t *testing.T) {
+// A new review runs under the fallback while the primary is over its limit,
+// and under the primary again once that limit resets.
+func TestANewReviewReturnsToThePrimaryOnceItsLimitResets(t *testing.T) {
 	r := refusedForUsage(t)
 	r.poll(t)
-	r.svc.Now = func() time.Time { return r.resets.Add(time.Hour) }
 
-	rec := prepareWith(t, r.svc, pr.Ref{Org: "haacked", Repo: "docket", Number: 8}, "claude", review.IntentReview)
+	during := prepareWith(t, r.svc, pr.Ref{Org: "haacked", Repo: "docket", Number: 8}, "claude", review.IntentReview)
+	if during.ClaudeConfigDir != r.fallback {
+		t.Errorf("account = %q, want the fallback %q while the primary is limited", during.ClaudeConfigDir, r.fallback)
+	}
 
-	if rec.ClaudeConfigDir != r.fallback {
-		t.Errorf("account = %q, want the fallback %q, which has not hit a limit", rec.ClaudeConfigDir, r.fallback)
+	r.svc.Now = func() time.Time { return r.resets }
+	after := prepareWith(t, r.svc, pr.Ref{Org: "haacked", Repo: "docket", Number: 9}, "claude", review.IntentReview)
+	if after.ClaudeConfigDir != r.primary {
+		t.Errorf("account = %q, want the primary %q once its limit reset", after.ClaudeConfigDir, r.primary)
 	}
 }