johannSoCodeyPublic

agent: send the identity block on the compaction request too

Compaction was the one OAuth request path that skipped it: summarize
built its llm.Request by hand, outside (*Agent).request/systemBlocks,
so a session running on a borrowed Claude Code login sent a bare
request with no system field at all. The API doesn't answer that with
401 — it's a 429 rate_limit_error, which (*APIError).Retryable treats
as transient, so the request burned three attempts with backoff and
then aborted the whole turn. Any long OAuth session died right at the
compaction threshold, looking exactly like a quota problem.

Fixed by sending SystemPrefix as the request's only system block when
it is set, with a comment explaining why a lone system block on an
otherwise bare request is load-bearing and must not be "cleaned up."
Added compact_test.go cases covering both the with- and
without-SystemPrefix requests, using the existing fakeAPI/newTestAgent
stub that captures outbound requests.

Also, from the same review pass:

- main.go: after the OAuth client is rebuilt with the discovered token
  source, assert IsDefaultEndpoint still holds and fail closed if not.
  The property was accidental (the rebuild happens to reuse the same
  opts) rather than enforced; this guards against a future refactor
  breaking that quietly.
- claudecode.Load: a credentials file missing expiresAt parsed to the
  Unix epoch and surfaced as "your Claude Code login has expired" —
  wrong and misleading. Now treated like a missing access token: a
  parse error naming the path and what's missing. Test added next to
  TestLoadEmptyAccessTokenIsAnError.
- claudecode.ErrExpired: doc comment no longer references a cancelled
  plan task; it now explains the shipped behaviour (refresh isn't
  implemented, because probing the token endpoint got a persistent 429
  on every attempt) and why that's a complete feature on its own.
- llm/client.go and llm/stream.go: the "no credentials" errors now
  mention the Claude Code login as a real route, matching main.go's
  errNoCredentials text, instead of only naming ANTHROPIC_API_KEY and
  a nonexistent `ant auth login` command.
- cmd/harness/auth_test.go: fixed a self-contradicting comment (said
  "valid for an hour," the fixture and the next line both say 2035).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Browse Files

Jjonicommitted 4 weeks ago1 parent c7c2c1fcommit17474dc

Showing 8 changed files with 97 additions and 7 deletions.

+1−1cmd/harness/auth_test.goView File

@@ -9,7 +9,7 @@ import (
9	9	"harness/internal/llm"
10	10	)
11	11
12		- // fakeHome writes a credentials file with a token valid for an hour.
	12	+ // fakeHome writes a credentials file with a token that is not expired.
13	13	func fakeHome(t *testing.T) string {
14	14	t.Helper()
15	15	home := t.TempDir()

+9−0cmd/harness/main.goView File

@@ -90,6 +90,15 @@ func run() error {
90	90	}
91	91	if src != nil {
92	92	client = llm.New(append(opts, llm.WithTokenSource(src))...)
	93	+ // Not expected to ever trigger: resolveClaudeCode already checked
	94	+ // the endpoint before we agreed to borrow this credential, but
	95	+ // that check ran against the client we had before this rebuild.
	96	+ // This is a guard against a future refactor reordering things so
	97	+ // the two clients quietly diverge, not a condition anyone should
	98	+ // see fire.
	99	+ if !client.IsDefaultEndpoint() {
	100	+ return errors.New("internal: discovered credential on a non-default endpoint")
	101	+ }
93	102	authNote = note
94	103	systemPrefix = agent.ClaudeCodeIdentity
95	104	}

+16−2internal/agent/compact.goView File

@@ -114,11 +114,25 @@ func (a *Agent) summarize(ctx context.Context, history []llm.Message) (string, e
114	114	Content: []llm.ContentBlock{llm.TextBlock(compactPrompt)},
115	115	})
116	116
117		- resp, err := a.client.CreateMessage(ctx, llm.Request{
	117	+ req := llm.Request{
118	118	MaxTokens: 4096,
119	119	Messages: stripTrailingToolUse(msgs),
120	120	Thinking: llm.Adaptive(),
121		- })
	121	+ }
	122	+ // This is the one request in the agent that is built by hand rather than
	123	+ // through (*Agent).request/systemBlocks, and it is deliberately bare — no
	124	+ // tools, no harness prompt — so it is easy to forget that an OAuth
	125	+ // credential still requires the Claude Code identity string as the FIRST
	126	+ // system block. Skip it here and the API does not 401; it returns a 429
	127	+ // that (*APIError).Retryable treats as transient, so the request burns
	128	+ // three attempts with backoff before the whole turn aborts. Do not "clean
	129	+ // up" this lone system block just because there is no other system prompt
	130	+ // alongside it — for an OAuth session, this is the whole system prompt.
	131	+ if a.opts.SystemPrefix != "" {
	132	+ req.System = []llm.SystemBlock{{Type: "text", Text: a.opts.SystemPrefix}}
	133	+ }
	134	+
	135	+ resp, err := a.client.CreateMessage(ctx, req)
122	136	if err != nil {
123	137	return "", fmt.Errorf("compaction: %w", err)
124	138	}

+43−0internal/agent/compact_test.goView File

@@ -160,6 +160,49 @@ func TestCompactionFoldsHistoryAndKeepsTheTask(t *testing.T) {
160	160	}
161	161	}
162	162
	163	+ // TestSummarizeCarriesSystemPrefixWhenSet guards the compaction bug: the
	164	+ // summarization request is built by hand, outside (*Agent).request, so it is
	165	+ // the one path that can silently omit the OAuth identity block. An omitted
	166	+ // block does not 401 — it 429s, which the client retries three times before
	167	+ // giving up, so a long OAuth session would die at the compaction threshold
	168	+ // looking like a quota problem.
	169	+ func TestSummarizeCarriesSystemPrefixWhenSet(t *testing.T) {
	170	+ f := newFakeAPI(t)
	171	+ a := newTestAgent(t, f, t.TempDir(), &bytes.Buffer{})
	172	+ a.opts.SystemPrefix = ClaudeCodeIdentity
	173	+
	174	+ history := []llm.Message{{Role: llm.RoleUser, Content: []llm.ContentBlock{llm.TextBlock("hello")}}}
	175	+ if _, err := a.summarize(context.Background(), history); err != nil {
	176	+ t.Fatalf("summarize: %v", err)
	177	+ }
	178	+ if len(f.requests) != 1 {
	179	+ t.Fatalf("expected 1 request, got %d", len(f.requests))
	180	+ }
	181	+ got := f.requests[0].System
	182	+ if len(got) != 1 \|\| got[0].Text != ClaudeCodeIdentity {
	183	+ t.Errorf("System = %+v, want a single block with the Claude Code identity", got)
	184	+ }
	185	+ }
	186	+
	187	+ // TestSummarizeCarriesNoSystemWhenPrefixUnset is the other half: a session
	188	+ // that never borrowed the Claude Code login must not gain a system block it
	189	+ // never asked for.
	190	+ func TestSummarizeCarriesNoSystemWhenPrefixUnset(t *testing.T) {
	191	+ f := newFakeAPI(t)
	192	+ a := newTestAgent(t, f, t.TempDir(), &bytes.Buffer{})
	193	+
	194	+ history := []llm.Message{{Role: llm.RoleUser, Content: []llm.ContentBlock{llm.TextBlock("hello")}}}
	195	+ if _, err := a.summarize(context.Background(), history); err != nil {
	196	+ t.Fatalf("summarize: %v", err)
	197	+ }
	198	+ if len(f.requests) != 1 {
	199	+ t.Fatalf("expected 1 request, got %d", len(f.requests))
	200	+ }
	201	+ if got := f.requests[0].System; len(got) != 0 {
	202	+ t.Errorf("System = %+v, want none when SystemPrefix is unset", got)
	203	+ }
	204	+ }
	205	+
163	206	func TestStripTrailingToolUse(t *testing.T) {
164	207	msgs := []llm.Message{
165	208	{Role: llm.RoleUser, Content: []llm.ContentBlock{llm.TextBlock("go")}},

+11−2internal/claudecode/claudecode.goView File

@@ -71,6 +71,9 @@ func Load(home string) (*Credentials, error) {
71	71	if o.AccessToken == "" {
72	72	return nil, fmt.Errorf("claudecode: %s has no access token", path)
73	73	}
	74	+ if o.ExpiresAt == 0 {
	75	+ return nil, fmt.Errorf("claudecode: %s has no expiresAt", path)
	76	+ }
74	77	return &Credentials{
75	78	AccessToken: o.AccessToken,
76	79	RefreshToken: o.RefreshToken,
@@ -93,8 +96,14 @@ func (c *Credentials) RefreshExpired(now time.Time) bool {
93	96	return !now.Before(c.RefreshExpiresAt)
94	97	}
95	98
96		- // ErrExpired means the stored access token has lapsed. Task 7 may replace this
97		- // with a refresh; until then it is the whole story.
	99	+ // ErrExpired means the stored access token has lapsed. Refresh is not
	100	+ // implemented: probing the token endpoint got a persistent 429 on every
	101	+ // attempt, with no exchange ever happening, so there is no evidence of a
	102	+ // route that actually refreshes this credential — and guessing further at
	103	+ // auth endpoints while holding someone's live refresh token is not something
	104	+ // to do on a hunch. Falling back to "expired, run claude to refresh it" is a
	105	+ // complete feature on its own: tokens last hours, so the cost is an
	106	+ // occasional re-login, paid by the tool that owns the credential.
98	107	var ErrExpired = errors.New("claudecode: your Claude Code login has expired, run `claude` to refresh it")
99	108
100	109	// Source hands out the access token for as long as it is valid. It satisfies

+15−0internal/claudecode/claudecode_test.goView File

@@ -97,6 +97,21 @@ func TestLoadEmptyAccessTokenIsAnError(t *testing.T) {
97	97	}
98	98	}
99	99
	100	+ // A missing expiresAt parses to zero, which time.UnixMilli turns into 1970 —
	101	+ // indistinguishable from an actually-expired token unless Load rejects it up
	102	+ // front. Silently accepting it would surface as the misleading "your Claude
	103	+ // Code login has expired" rather than the schema-drift error this package
	104	+ // promises.
	105	+ func TestLoadMissingExpiresAtIsAnError(t *testing.T) {
	106	+ _, err := Load(write(t, `{"claudeAiOauth":{"accessToken":"tok"}}`))
	107	+ if err == nil \|\| errors.Is(err, ErrNotFound) {
	108	+ t.Fatalf("err = %v, want an error about the missing expiresAt", err)
	109	+ }
	110	+ if errors.Is(err, ErrExpired) {
	111	+ t.Fatal("a missing expiresAt is a parse error, not an expired token")
	112	+ }
	113	+ }
	114	+
100	115	func TestExpiredUsesASkewMargin(t *testing.T) {
101	116	base := time.Unix(1_700_000_000, 0)
102	117	c := &Credentials{ExpiresAt: base}

+1−1internal/llm/client.goView File

@@ -208,7 +208,7 @@ func (c Client) CreateMessage(ctx context.Context, req Request) (Response, err
208	208	// Checked once, up front: a missing credential is permanent, and retrying
209	209	// it three times with backoff just delays the error.
210	210	if !c.HasCredentials() {
211		- return nil, fmt.Errorf("llm: no credentials: set ANTHROPIC_API_KEY, or run `ant auth login`")
	211	+ return nil, fmt.Errorf("llm: no credentials: set ANTHROPIC_API_KEY, ANTHROPIC_AUTH_TOKEN, or log in with Claude Code")
212	212	}
213	213	a, err := c.resolveAuth(ctx)
214	214	if err != nil {

+1−1internal/llm/stream.goView File

@@ -35,7 +35,7 @@ type StreamEvent struct {
35	35	// returns the assembled response. The callback may be nil.
36	36	func (c Client) StreamMessage(ctx context.Context, req Request, onEvent func(StreamEvent)) (Response, error) {
37	37	if !c.HasCredentials() {
38		- return nil, fmt.Errorf("llm: no credentials: set ANTHROPIC_API_KEY, or run `ant auth login`")
	38	+ return nil, fmt.Errorf("llm: no credentials: set ANTHROPIC_API_KEY, ANTHROPIC_AUTH_TOKEN, or log in with Claude Code")
39	39	}
40	40	a, err := c.resolveAuth(ctx)
41	41	if err != nil {