diff --git a/adapters/telegram/.env.example b/adapters/telegram/.env.example index 92ea519..b5e87f7 100644 --- a/adapters/telegram/.env.example +++ b/adapters/telegram/.env.example @@ -20,6 +20,16 @@ AI_BACKEND=claude # claude | codex | openai-compatible # CODEX_HOME/auth.json (persisted in the codex_home volume) or CODEX_ACCESS_TOKEN. # Keep CODEX_API_KEY empty in subscription mode: startup fails if it is set, so a # subscription deploy cannot accidentally switch to usage-based API billing. +# +# First sign-in needs no shell on the host: the bot starts unauthorized and an +# allow-listed user sends /login IN A DIRECT MESSAGE (the reply carries a one-time +# code, so starting a sign-in is refused in group chats). Runs are blocked until +# it completes. CODEX_REQUIRE_AUTH=false only removes the block — without +# auth.json or CODEX_ACCESS_TOKEN runs then fail inside the Codex CLI instead. +# CODEX_HOME must stay non-empty: with an empty value startup still fails, since +# there would be nowhere to persist the login. +# Full flow, and who ends up owning the Codex account: +# docs/codex-integration-plan.md#subscription-setup CODEX_AUTH_MODE=subscription # subscription | billing CODEX_BIN=codex CODEX_HOME=/home/claude/.codex diff --git a/adapters/telegram/commands.go b/adapters/telegram/commands.go index f0d594a..add606b 100644 --- a/adapters/telegram/commands.go +++ b/adapters/telegram/commands.go @@ -4,6 +4,8 @@ import ( "strings" "github.com/go-telegram/bot/models" + + "github.com/duckbugio/flock/core/chat" ) // CommandName returns the slash command addressed at the start of msg, @@ -82,22 +84,14 @@ func StripCommandMention(text, botUsername string) string { return token[:at] + rest } -// HelpText is the static usage message replied to an allowed user who sends -// /help. It lists the slash commands the adapter understands. It is an -// engineering artifact (professional English, no duck flavor) and never reaches -// the Claude Runner. -const HelpText = "Flock Telegram assistant — available commands:\n\n" + - "/help — show this message\n" + - "/new — start a fresh session (forget the current conversation)\n" + - "/stop — stop the run currently in progress\n" + - "/schedule — manage scheduled jobs (when enabled)\n" + - "/goal — arm a goal an independent evaluator re-checks after every run " + - "(/goal off to disarm)\n\n" + - "Send any other message to run it through the assistant." +// helpTitle names the transport; everything below it is shared with the VK +// adapter (chat.HelpBody), so the two cannot drift apart. +const helpTitle = "Flock Telegram assistant — available commands:\n\n" + +// HelpText renders the usage message replied to an allowed user who sends /help. +// It is a thin alias over the shared render, so this adapter owns only its title. +func HelpText(login chat.LoginVisibility) string { return chat.HelpText(helpTitle, login) } -// WelcomeText is the static usage message replied to an allowed user who sends -// /start. It is a short greeting prepended to the command help so a brand-new -// user immediately sees what the bot does and how to use it. Like HelpText it is -// an engineering artifact (plain English, no duck flavor) and never reaches the -// Claude Runner — the duck greeting comes from the model on a real message. -const WelcomeText = "Hi! I'm the Flock assistant.\n\n" + HelpText +// WelcomeText is the reply to /start: the shared greeting prepended to the help, +// so a brand-new user immediately sees what the bot does and how to use it. +func WelcomeText(login chat.LoginVisibility) string { return chat.WelcomeText(helpTitle, login) } diff --git a/adapters/telegram/commands_test.go b/adapters/telegram/commands_test.go index fd07556..ea73109 100644 --- a/adapters/telegram/commands_test.go +++ b/adapters/telegram/commands_test.go @@ -6,6 +6,8 @@ import ( "testing" "github.com/go-telegram/bot/models" + + "github.com/duckbugio/flock/core/chat" ) // botCmd builds a message whose leading token is a bot_command entity, the shape @@ -96,12 +98,19 @@ func TestStripCommandMention(t *testing.T) { // it can never submit a run); the constant IS the entire payload the handler // sends. func TestHelpTextListsCommands(t *testing.T) { - if HelpText == "" { - t.Fatal("HelpText is empty") - } - for _, cmd := range []string{"/help", "/new", "/stop"} { - if !strings.Contains(HelpText, cmd) { - t.Fatalf("HelpText does not mention %q:\n%s", cmd, HelpText) + // Both renders: production picks between them with LoginVisibilityFor, and the + // WITHOUT-login one is what every Claude, Codex-billing and access-token + // deployment gets — so testing only the other would leave the common case + // uncovered. + for _, visibility := range []chat.LoginVisibility{chat.WithLogin, chat.WithoutLogin} { + got := HelpText(visibility) + if got == "" { + t.Fatalf("HelpText(%v) is empty", visibility) + } + for _, cmd := range []string{"/help", "/new", "/stop"} { + if !strings.Contains(got, cmd) { + t.Fatalf("HelpText(%v) does not mention %q:\n%s", visibility, cmd, got) + } } } } diff --git a/adapters/telegram/deploy/roles/claude_tg_bot/templates/env.j2 b/adapters/telegram/deploy/roles/claude_tg_bot/templates/env.j2 index 4f06267..d8903a3 100644 --- a/adapters/telegram/deploy/roles/claude_tg_bot/templates/env.j2 +++ b/adapters/telegram/deploy/roles/claude_tg_bot/templates/env.j2 @@ -29,6 +29,15 @@ CLAUDE_MAX_COST_PER_REQUEST={{ claude_max_cost_per_request }} # === CODEX AUTH === # subscription: use persisted {{ codex_home }}/auth.json or CODEX_ACCESS_TOKEN. +# With neither, the bot starts unauthorized and an allow-listed user completes +# the one-time sign-in from the chat with /login IN A DIRECT MESSAGE. Runs are +# blocked until it lands. CODEX_REQUIRE_AUTH=false only removes the block — +# without auth.json or CODEX_ACCESS_TOKEN runs then fail inside the Codex CLI +# instead. +# CODEX_HOME must stay non-empty: with an empty value startup still fails, +# since there would be nowhere to persist the login. +# Full flow, and who ends up owning the Codex account: +# docs/codex-integration-plan.md#subscription-setup # billing: requires CODEX_API_KEY and CODEX_BILLING_ACK=true. CODEX_BIN={{ codex_bin }} CODEX_MODEL={{ codex_model }} diff --git a/adapters/vk/commands.go b/adapters/vk/commands.go index 5c01dc8..0fa1db8 100644 --- a/adapters/vk/commands.go +++ b/adapters/vk/commands.go @@ -1,17 +1,19 @@ package vk -// HelpText is the static usage message for the VK adapter. It mirrors the -// Telegram adapter's HelpText and lists VK's command surface: VK has no native -// slash-command UI, so these are plain text commands the receiver intercepts. It -// is an engineering artifact (professional English, no duck flavor). -const HelpText = "Flock VK assistant — available commands:\n\n" + - "/help — show this message\n" + - "/new — start a fresh session (forget the current conversation)\n" + - "/stop — stop the run currently in progress\n" + - "/schedule — manage scheduled jobs (when enabled)\n" + - "/goal — arm a goal an independent evaluator re-checks after every run " + - "(/goal off to disarm)\n\n" + - "Send any other message to run it through the assistant." +import "github.com/duckbugio/flock/core/chat" + +// helpTitle names the transport; everything below it is shared with the Telegram +// adapter (chat.HelpBody), so the two cannot drift apart. VK has no native +// slash-command UI, so these are plain text commands the receiver intercepts. +const helpTitle = "Flock VK assistant — available commands:\n\n" + +// helpText renders the usage message. It is a thin alias over the shared render, +// so this adapter owns only its title. Unexported like welcomeText beside it: +// nothing outside this package renders VK's help. +func helpText(login chat.LoginVisibility) string { return chat.HelpText(helpTitle, login) } + +// welcomeText is the reply to /start, mirroring the Telegram adapter. +func welcomeText(login chat.LoginVisibility) string { return chat.WelcomeText(helpTitle, login) } // goalUsageText is the /goal usage reply, mirroring the Telegram adapter. const goalUsageText = "Usage:\n" + diff --git a/adapters/vk/receiver.go b/adapters/vk/receiver.go index ac95e4a..abbe0cf 100644 --- a/adapters/vk/receiver.go +++ b/adapters/vk/receiver.go @@ -12,6 +12,7 @@ import ( "github.com/duckbugio/flock/core/agent" "github.com/duckbugio/flock/core/chat" + "github.com/duckbugio/flock/core/codexauth" "github.com/duckbugio/flock/core/goal" "github.com/duckbugio/flock/core/schedule" ) @@ -20,11 +21,6 @@ import ( // turned off (the default). It mirrors the Telegram adapter's notice. const scheduleDisabledText = "Scheduler is disabled. Set ENABLE_SCHEDULER=true to enable it." -// welcomeText is the static reply to /start: a short greeting + the usage help, -// mirroring the Telegram adapter's WelcomeText. It is an engineering artifact -// (plain English, no duck flavor) and never reaches the Claude Runner. -const welcomeText = "Hi! I'm the Flock assistant.\n\n" + HelpText - // newSessionText is the static reply to /new: confirms the session was reset so // the next message starts a brand-new conversation. const newSessionText = "Started a fresh session. Your next message begins a new conversation." @@ -115,6 +111,7 @@ type Receiver struct { notices NoticeSender eventAck eventAckFunc sched *schedule.Manager + auth *codexauth.Manager logger *slog.Logger } @@ -134,6 +131,10 @@ type ReceiverConfig struct { // Scheduler serves the /schedule command. Nil when ENABLE_SCHEDULER is off, in // which case /schedule replies with the disabled notice. Scheduler *schedule.Manager + // CodexAuth serves /login and blocks runs while a Codex subscription deploy has + // no completed sign-in. A nil manager never blocks and reports /login as + // unnecessary. + CodexAuth *codexauth.Manager Logger *slog.Logger } @@ -160,6 +161,7 @@ func NewReceiver(cfg ReceiverConfig) *Receiver { notices: cfg.Notices, eventAck: cfg.EventAck, sched: cfg.Scheduler, + auth: cfg.CodexAuth, logger: log, } } @@ -328,6 +330,45 @@ func (r *Receiver) onMessageNew(ctx context.Context, msg messageObject) { return } + text := strings.TrimSpace(cleaned) + // Decided ONCE, and used by both the gate and the dispatch below. + w := r.classify(text, msg) + + // Authorization is checked BEFORE the guards, and so before the rate limiter + // counts this message: a run that cannot happen must not spend the user's + // window, or the first real messages after a successful /login would hit the + // limit. The check is one os.Stat and no paid work precedes it. The notice is + // sent once per peer until authorization returns — dropping the limiter as a + // throttle means supplying the restraint here, exactly as core/chat does for + // its own refusals. + if w.kind != workNone { + if r.auth.Blocked() { + // Warn, matching core/chat's identical refusal: an operator filtering on + // Warn would otherwise see the background refusals and miss the user-facing + // ones, which are the signal that the deployment needs /login right now. + r.logger.Warn("vk: codex unauthorized — refusing run", "peer_id", peerID, "user_id", msg.FromID) + // One registry, owned by the gate: this chat may already have been told by + // a refused background submission, and telling it again here would be the + // same sentence twice. + // ReplyNoticeFor, not NoticeFor: a refused background submission must not + // consume the answer this person is owed. + if notice := r.auth.ReplyNoticeFor(chatIDStr(peerID)); notice != "" { + // On its own goroutine, bounded, like /login and for the same reason: this + // is the poll loop, which handles updates serially, and the VK client has + // no timeout of its own. Waiting here would stall every chat — including + // the /login that fixes the state — on precisely the broken deployment + // where this fires. Nothing below waits for the reply. + go func() { + sendCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), codexauth.NotifyTimeout) + defer cancel() + r.notify(sendCtx, peerID, notice) + }() + } + return + } + r.auth.NoticeReset() + } + if r.guards != nil { if allow, reason := r.guards(msg.FromID); !allow { r.logger.Debug("vk: guardrail denied message", "peer_id", peerID, "user_id", msg.FromID, "reason", reason) @@ -336,38 +377,88 @@ func (r *Receiver) onMessageNew(ctx context.Context, msg messageObject) { } } - text := strings.TrimSpace(cleaned) - - // Voice: transcribe the first audio_message attachment (gate already passed, so - // we only pay for transcription on an accepted message). - if att := firstAudioMessage(msg.Attachments); att != nil && r.voice != nil && text == "" { - r.handleVoice(ctx, msg, att) + // No default branch on purpose: the exhaustive linter treats one as "all cases + // covered", so a new workKind without a case here would stop failing the build — + // which is the check worth having. Every case returns, so there is nothing after + // the switch. + switch w.kind { + case workNone: + return + case workVoice: + // Transcribe the audio_message attachment (gate already passed, so we only + // pay for transcription on an accepted message). + r.handleVoice(ctx, msg, w.voice) + return + case workDoc: + // Save to the per-chat uploads dir (outside every git tree) and submit a run + // referencing the saved path. + r.handleDocument(ctx, msg, w.doc, text) + return + case workPhoto: + // As workDoc, and the run also carries a vision content block. + r.handlePhoto(ctx, msg, w.photo, text) + return + case workText: + // Fold any quoted/replied-to/forwarded original into the prompt so the run + // sees the context the user is referring to, not just their new text. + // QuotedPrompt is a strict no-op when there is no quote, so a normal message + // stays unchanged. + qAuthor, qText := quotedContext(msg) + text = chat.QuotedPrompt(qAuthor, qText, text) + r.svc.Handle(ctx, chatIDStr(peerID), msg.FromID, msgIDStr(msg.ConversationMessageID), text) return } +} - // Inbound document / photo: save to the per-chat uploads dir (outside every git - // tree) and submit a run referencing the saved path (a photo also attaches a - // vision content block). +// workKind names what an accepted message would submit. workNone means the +// receiver would drop it, which is also what decides whether the unauthorized +// notice is worth sending. +type workKind int + +const ( + workNone workKind = iota + workText + workVoice + workDoc + workPhoto +) + +// work is what an accepted message would submit: its kind, and the attachment the +// matching handler needs. Carrying the attachment is the point — the handlers +// dereference it unconditionally, so finding it a second time in the dispatch +// switch would put a nil-panic in the poll loop behind nothing but an agreement +// between two functions to keep picking the same one. +type work struct { + kind workKind + voice *audioMsgAttach + doc *docAttachment + photo *photoAttachment +} + +// classify decides what an accepted message would submit. The decision is made +// ONCE and used by both the unauthorized gate and the dispatch switch — that +// single decision is the point, not the number of passes over the slice. text is +// the mention-stripped, trimmed body. The precedence is +// the dispatch order: a voice note counts only while there is no text to run +// instead, and attachments only when an uploader is configured. +func (r *Receiver) classify(text string, msg messageObject) work { + if text == "" && r.voice != nil { + if att := firstAudioMessage(msg.Attachments); att != nil { + return work{kind: workVoice, voice: att} + } + } if r.uploads != nil { if doc := firstDoc(msg.Attachments); doc != nil { - r.handleDocument(ctx, msg, doc, text) - return + return work{kind: workDoc, doc: doc} } if ph := firstPhoto(msg.Attachments); ph != nil { - r.handlePhoto(ctx, msg, ph, text) - return + return work{kind: workPhoto, photo: ph} } } - - if text == "" { - return + if text != "" { + return work{kind: workText} } - // Fold any quoted/replied-to/forwarded original into the prompt so the run sees - // the context the user is referring to, not just their new text. QuotedPrompt is - // a strict no-op when there is no quote, so a normal message stays unchanged. - qAuthor, qText := quotedContext(msg) - text = chat.QuotedPrompt(qAuthor, qText, text) - r.svc.Handle(ctx, chatIDStr(peerID), msg.FromID, msgIDStr(msg.ConversationMessageID), text) + return work{kind: workNone} } // dispatchReserved acts on a reserved command (caller already confirmed the name @@ -381,9 +472,9 @@ func (r *Receiver) dispatchReserved(ctx context.Context, name string, msg messag peerID := msg.PeerID switch name { case "start": - r.notify(ctx, peerID, welcomeText) + r.notify(ctx, peerID, welcomeText(chat.LoginVisibilityFor(r.auth.LoginAdvertised()))) case "help": - r.notify(ctx, peerID, HelpText) + r.notify(ctx, peerID, helpText(chat.LoginVisibilityFor(r.auth.LoginAdvertised()))) case "new": if err := r.svc.NewSession(chatIDStr(peerID)); err != nil { r.logger.Error("vk: reset session", "peer_id", peerID, "error", err) @@ -395,6 +486,56 @@ func (r *Receiver) dispatchReserved(ctx context.Context, name string, msg messag r.dispatchSchedule(ctx, msg) case "goal": r.dispatchGoal(ctx, msg) + case "login": + // On its own goroutine: this receiver processes updates SERIALLY inside the + // poll loop, and /login can block for seconds — Cancel waits out the login + // goroutine (up to cancelDrain) and each reply is a transport call bounded by + // NotifyTimeout. Blocking there would stall every chat's messages. Detaching + // is safe because the login itself is already detached (Dispatch derives its + // own context) and every notice builds its own. + go r.dispatchLogin(ctx, msg) + } +} + +// dispatchLogin serves /login: start, report, or cancel the Codex device-code +// sign-in. It stays available while the deployment is unauthorized — it is the +// only way out of that state. The sender is already allow-list gated by +// onMessageNew, which returns before any reserved command is dispatched. +// +// Whether a one-time code may be printed here is decided by codexauth from the +// Private flag, not by this adapter inspecting the arguments: /login status also +// re-shows a pending code, so an argument-based guard would let it straight past. +// A VK conversation has a peer id distinct from the sender's, which is exactly +// what makes it non-private. +// +// The sign-in outlives this call: Dispatch replies immediately and keeps working +// in the background, delivering the link, the one-time code, and the verdict to +// its subscribers. The callback builds its OWN context, because this update's is +// long gone by the time a user finishes in a browser. +func (r *Receiver) dispatchLogin(ctx context.Context, msg messageObject) { + peerID := msg.PeerID + // One bounded, detached sender for BOTH paths. This handler runs on its own + // goroutine, so the poll loop's context may be cancelled while it is still + // working — on the raw context the replies to /login status, /login cancel and + // the direct-message refusal would be dropped silently at shutdown, and none of + // them would carry a deadline of its own. + notify := func(text string) { + sendCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), codexauth.NotifyTimeout) + defer cancel() + r.notify(sendCtx, peerID, text) + } + sub := codexauth.Subscriber{ + ID: chatIDStr(peerID), + // VK states the 1:1 invariant directly in the update: in a direct message the + // peer IS the sender. Deriving it from the peer-id range instead would call + // every non-conversation peer private, community peers included — too loose a + // test for a flag that decides who may take over the bot's account. + Private: peerID == msg.FromID, + Notify: notify, + } + // An empty reply means Dispatch already delivered it through sub, in order. + if reply := r.auth.Dispatch(ctx, commandArgs(msg.Text), sub); reply != "" { + notify(reply) } } diff --git a/adapters/vk/receiver_test.go b/adapters/vk/receiver_test.go index c59421f..ad806f0 100644 --- a/adapters/vk/receiver_test.go +++ b/adapters/vk/receiver_test.go @@ -5,13 +5,18 @@ import ( "context" "encoding/json" "errors" + "os" "path/filepath" "strings" "sync" "testing" + "time" "github.com/duckbugio/flock/core/agent" "github.com/duckbugio/flock/core/chat" + "github.com/duckbugio/flock/core/codexauth" + "github.com/duckbugio/flock/core/codexauth/codexauthtest" + "github.com/duckbugio/flock/core/codexauth/logintest" "github.com/duckbugio/flock/core/goal" "github.com/duckbugio/flock/core/schedule" ) @@ -114,10 +119,69 @@ func (n *fakeNotice) Notify(_ context.Context, _ int64, text string) { n.texts = append(n.texts, text) } +// seen snapshots the notices delivered so far. /login is dispatched on its own +// goroutine (it must not block the poll loop), so its tests read through this +// rather than touching the slice. +func (n *fakeNotice) seen() []string { + n.mu.Lock() + defer n.mu.Unlock() + return append([]string(nil), n.texts...) +} + +// waitUntilNotices waits for exactly want notices, tolerating the detached send. +func waitUntilNotices(t *testing.T, n *fakeNotice, want int) { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) && len(n.seen()) < want { + time.Sleep(2 * time.Millisecond) + } + if got := n.seen(); len(got) != want { + t.Fatalf("got %d notices, want %d: %v", len(got), want, got) + } +} + +// awaitOne waits for exactly ONE notice and fails if a second arrives. +// +// Returning on the FIRST notice would have made every "exactly one" assertion +// mean "at least one": a second message almost never lands before the snapshot is +// read. That matters most for the privacy regressions here — they look for the +// device code in notice [0], so an implementation that also broadcast it to the +// conversation as a second message would keep them green. +func (n *fakeNotice) awaitOne(t *testing.T) []string { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) && len(n.seen()) == 0 { + time.Sleep(2 * time.Millisecond) + } + got := n.seen() + if len(got) != 1 { + t.Fatalf("got %d notices, want 1: %v", len(got), got) + } + // Give a stray second a chance to show up, so "exactly one" means it. + time.Sleep(50 * time.Millisecond) + if extra := n.seen(); len(extra) != 1 { + t.Fatalf("got %d notices, want 1: %v", len(extra), extra) + } + return got +} + // newTestReceiver builds a Receiver wired to the fakes, allowing user 42, group // 123, with the given requireMention. func newTestReceiver(svc Service, notices NoticeSender, requireMention bool, acked *[]string) *Receiver { + return newTestReceiverOpts(svc, notices, requireMention, acked, nil) +} + +// newTestReceiverWithAuth is newTestReceiver with a Codex sign-in manager wired +// through the config, as production does. +func newTestReceiverWithAuth(svc Service, notices NoticeSender, auth *codexauth.Manager) *Receiver { + return newTestReceiverOpts(svc, notices, false, nil, auth) +} + +func newTestReceiverOpts( + svc Service, notices NoticeSender, requireMention bool, acked *[]string, auth *codexauth.Manager, +) *Receiver { return NewReceiver(ReceiverConfig{ + CodexAuth: auth, Service: svc, GroupID: 123, RequireMention: requireMention, @@ -352,7 +416,7 @@ func TestReceiverStartTextCommand(t *testing.T) { if len(svc.handleCalls) != 0 { t.Errorf("/start should not start a run, got %d Handle calls", len(svc.handleCalls)) } - if len(notices.texts) != 1 || notices.texts[0] != welcomeText { + if len(notices.texts) != 1 || notices.texts[0] != welcomeText(chat.WithoutLogin) { t.Errorf("notice texts = %v, want one welcome notice", notices.texts) } } @@ -398,7 +462,7 @@ func TestReceiverHelpTextCommand(t *testing.T) { if len(svc.handleCalls) != 0 { t.Errorf("/help should not start a run, got %d Handle calls", len(svc.handleCalls)) } - if len(notices.texts) != 1 || notices.texts[0] != HelpText { + if len(notices.texts) != 1 || notices.texts[0] != helpText(chat.WithoutLogin) { t.Errorf("notice texts = %v, want one HelpText notice", notices.texts) } } @@ -497,7 +561,7 @@ func TestReceiverScheduleEnabledReachesDispatch(t *testing.T) { if err != nil { t.Fatalf("open store: %v", err) } - mgr := schedule.NewManager(store, func(string, string, int64) bool { return true }, nil, nil, nil) + mgr := schedule.NewManager(store, func(string, string, int64) bool { return true }, nil, nil, nil, nil) r := NewReceiver(ReceiverConfig{ Service: svc, GroupID: 123, @@ -623,3 +687,387 @@ func (f *fakeLongPoller) poll(_ context.Context, _ longPollServer) (longPollResp func isContextErr(err error) bool { return errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) } + +// unauthorizedCodexReceiver wires a receiver whose Codex backend has never been +// signed in (an empty CODEX_HOME, so no auth.json). The manager goes in through +// ReceiverConfig, the same way production wires it, so this covers NewReceiver's +// own plumbing instead of reaching past it into the private field. +func unauthorizedCodexReceiver(t *testing.T, svc Service, notices NoticeSender) *Receiver { + t.Helper() + auth, _ := logintest.Unauthorized(t) + return newTestReceiverWithAuth(svc, notices, auth) +} + +// TestReceiverBlocksRunsWhileCodexUnauthorized: a Codex deploy with no completed +// sign-in must answer with the actionable notice instead of starting a run that +// could only fail inside the CLI. +func TestReceiverBlocksRunsWhileCodexUnauthorized(t *testing.T) { + svc := &fakeService{} + notices := &fakeNotice{} + r := unauthorizedCodexReceiver(t, svc, notices) + + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{FromID: 42, PeerID: 200, Text: "build it"})) + + if len(svc.handleCalls) != 0 { + t.Errorf("an unauthorized Codex deploy started %d runs, want 0", len(svc.handleCalls)) + } + if got := notices.awaitOne(t); !strings.Contains(got[0], "/login") { + t.Errorf("notices = %v, want one notice pointing at /login", got) + } +} + +// TestReceiverLoginCommandStaysReachableWhileUnauthorized is the escape hatch: +// the very command that fixes the unauthorized state must not be blocked by it. +func TestReceiverLoginCommandStaysReachableWhileUnauthorized(t *testing.T) { + svc := &fakeService{} + notices := &fakeNotice{} + r := unauthorizedCodexReceiver(t, svc, notices) + + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{FromID: 42, PeerID: 42, Text: "/login status"})) + + if len(svc.handleCalls) != 0 { + t.Errorf("/login should not start a run, got %d Handle calls", len(svc.handleCalls)) + } + if got := notices.awaitOne(t); !strings.Contains(got[0], "NOT authorized") { + t.Errorf("notices = %v, want the login status reply", got) + } +} + +// TestReceiverLoginWithoutManagerIsInert: a deployment wired without a Codex auth +// manager (the Claude default) answers /login instead of panicking, and never +// blocks a run. +func TestReceiverLoginWithoutManagerIsInert(t *testing.T) { + svc := &fakeService{} + notices := &fakeNotice{} + r := newTestReceiver(svc, notices, false, nil) + + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{FromID: 42, PeerID: 42, Text: "/login"})) + if got := notices.awaitOne(t); got[0] != codexauth.NoLoginNeededText { + t.Errorf("notices = %v, want the no-login-needed reply", got) + } + + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{FromID: 42, PeerID: 200, Text: "build it"})) + if len(svc.handleCalls) != 1 { + t.Errorf("Handle calls = %d, want 1 (no manager must never block)", len(svc.handleCalls)) + } +} + +// TestReceiverLoginRefusedInConversation: the reply goes to the PEER, so in a +// community conversation the verification link and one-time code would be +// readable by every participant — and whoever acts on the code first binds THEIR +// ChatGPT account to the bot, sending every later run through it. +func TestReceiverLoginRefusedInConversation(t *testing.T) { + svc := &fakeService{} + notices := &fakeNotice{} + r := unauthorizedCodexReceiver(t, svc, notices) + + // 2000000000+ is VK's conversation (chat) peer range. + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{FromID: 42, PeerID: 2000000001, Text: "/login"})) + + got := notices.awaitOne(t) + if !strings.Contains(got[0], "direct message") { + t.Fatalf("notices = %v, want the refusal pointing at a direct message", got) + } + if strings.Contains(got[0], "http") { + t.Error("the refusal leaked a link into the conversation") + } +} + +// TestReceiverLoginStatusInConversationHidesAPendingCode is the regression for +// the hole an argument-based guard left: /login status ALSO re-shows a pending +// one-time code, so refusing only the code-starting arguments let it into the +// conversation anyway — where anyone, allow-listed or not, could bind their own +// ChatGPT account to the bot. +func TestReceiverLoginStatusInConversationHidesAPendingCode(t *testing.T) { + svc := &fakeService{} + notices := &fakeNotice{} + r := newTestReceiverWithAuth(svc, notices, logintest.PendingLogin(t)) + + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{ + FromID: 42, PeerID: 2000000001, Text: "/login status", + })) + + got := notices.awaitOne(t) + if strings.Contains(got[0], codexauthtest.DeviceCode) || strings.Contains(got[0], "http") { + t.Errorf("the pending code or link reached a conversation: %q", got[0]) + } + if !strings.Contains(got[0], "in progress") { + t.Errorf("status = %q, want it to still report the pending sign-in", got[0]) + } +} + +// TestReceiverLoginStatusInDirectMessageShowsTheCode: the same command in a 1:1 +// chat must still re-show the code — that is what makes a lost message +// recoverable. +func TestReceiverLoginStatusInDirectMessageShowsTheCode(t *testing.T) { + svc := &fakeService{} + notices := &fakeNotice{} + r := newTestReceiverWithAuth(svc, notices, logintest.PendingLogin(t)) + + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{FromID: 42, PeerID: 42, Text: "/login status"})) + + if got := notices.awaitOne(t); !strings.Contains(got[0], codexauthtest.DeviceCode) { + t.Errorf("notices = %v, want the pending code re-shown in a direct message", got) + } +} + +// TestReceiverLoginRejectsDisallowedSender: /login (and especially /login force) +// changes the provider credentials for the WHOLE process, so it must be as +// allow-list gated as any other command. onMessageNew returns before dispatching +// any reserved command for a disallowed sender; this pins that. +func TestReceiverLoginRejectsDisallowedSender(t *testing.T) { + svc := &fakeService{} + notices := &fakeNotice{} + r := unauthorizedCodexReceiver(t, svc, notices) + + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{FromID: 999, PeerID: 999, Text: "/login"})) + + if len(notices.texts) != 0 { + t.Errorf("a disallowed sender got %v, want silence", notices.texts) + } +} + +// TestHelpListsLoginOnlyWhereItApplies mirrors the Telegram adapter: the /login +// line appears only where an interactive sign-in exists, so the two adapters (and +// Telegram's command menu) cannot disagree. +func TestHelpListsLoginOnlyWhereItApplies(t *testing.T) { + if !strings.Contains(helpText(chat.WithLogin), "/login") { + t.Error("help omits /login where the sign-in is real") + } + if strings.Contains(helpText(chat.WithoutLogin), "/login") { + t.Error("help advertises /login where there is no sign-in") + } + if strings.Contains(welcomeText(chat.WithoutLogin), "/login") { + t.Error("welcome advertises /login where there is no sign-in") + } +} + +// TestPrivacyFollowsTheDirectMessageInvariant: VK states 1:1 directly — in a +// direct message the peer IS the sender. A range check on the peer id would call +// community peers private too, which is far too loose for the flag that decides +// who may see a code that takes over the bot's account. +func TestPrivacyFollowsTheDirectMessageInvariant(t *testing.T) { + auth := logintest.PendingLogin(t) + + tests := []struct { + name string + fromID int64 + peerID int64 + wantsCode bool + }{ + {"direct message", 42, 42, true}, + {"conversation", 42, 2000000001, false}, + {"community peer", 42, -1500, false}, + {"peer that is not the sender", 42, 200, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + notices := &fakeNotice{} + r := newTestReceiverWithAuth(&fakeService{}, notices, auth) + + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{ + FromID: tt.fromID, PeerID: tt.peerID, Text: "/login status", + })) + + shown := notices.awaitOne(t) + got := strings.Contains(shown[0], codexauthtest.DeviceCode) + if got != tt.wantsCode { + t.Errorf("code shown = %v, want %v (reply: %q)", got, tt.wantsCode, notices.awaitOne(t)[0]) + } + }) + } +} + +// TestHelpLineComesFromTheCanonicalSet: both adapters render the /login help +// line, so it has one home beside the canonical command set. Two literals would +// drift silently — the adapters would still share the applicability predicate but +// not the words. +func TestHelpLineComesFromTheCanonicalSet(t *testing.T) { + if !strings.Contains(helpText(chat.WithLogin), chat.LoginHelpLine) { + t.Error("the VK help does not render the canonical /login line") + } +} + +// TestSilentMessagesGetNoUnauthorizedNotice: a sticker or an empty message is +// dropped silently when the deployment is healthy, so an unauthorized one must +// not answer it either — that spends rate limit telling the user about work they +// never asked for. +func TestSilentMessagesGetNoUnauthorizedNotice(t *testing.T) { + svc := &fakeService{} + notices := &fakeNotice{} + r := unauthorizedCodexReceiver(t, svc, notices) + + // No text, no voice, no attachment the receiver would save. + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{FromID: 42, PeerID: 42, Text: " "})) + + if got := notices.seen(); len(got) != 0 { + t.Errorf("an empty message drew %v, want silence", got) + } + if len(svc.handleCalls) != 0 { + t.Errorf("an empty message started %d runs, want 0", len(svc.handleCalls)) + } +} + +// TestRealMessagesStillGetTheUnauthorizedNotice is the other half: a message that +// WOULD have run must still be answered. +func TestRealMessagesStillGetTheUnauthorizedNotice(t *testing.T) { + svc := &fakeService{} + notices := &fakeNotice{} + r := unauthorizedCodexReceiver(t, svc, notices) + + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{FromID: 42, PeerID: 42, Text: "build it"})) + + if got := notices.awaitOne(t); !strings.Contains(got[0], "/login") { + t.Errorf("notice = %q, want it to point at /login", got[0]) + } +} + +// TestLoginDoesNotBlockThePollLoop: this receiver processes updates serially, and +// /login can block for seconds (Cancel waits out the login goroutine; each reply +// is a bounded transport call). Handling it inline would stall every chat, so it +// runs detached — dispatch must return without waiting for the reply. +func TestLoginDoesNotBlockThePollLoop(t *testing.T) { + svc := &fakeService{} + notices := &fakeNotice{} + r := newTestReceiverWithAuth(svc, notices, logintest.PendingLogin(t)) + + done := make(chan struct{}) + go func() { + defer close(done) + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{FromID: 42, PeerID: 42, Text: "/login cancel"})) + }() + + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("dispatch blocked on /login; the poll loop would stall with it") + } + notices.awaitOne(t) +} + +// TestClassifyDrivesBothGateAndDispatch pins the single decision: whatever the +// receiver would submit is exactly what the unauthorized gate answers about, and +// the attachment travels WITH that decision. Two independent condition lists +// would agree only until the next attachment type is added to the dispatch switch. +func TestClassifyDrivesBothGateAndDispatch(t *testing.T) { + r := newTestReceiver(&fakeService{}, &fakeNotice{}, false, nil) + r.uploads = nil + r.voice = nil + + tests := []struct { + name string + text string + msg messageObject + want workKind + }{ + {"plain text", "build it", messageObject{}, workText}, + {"nothing at all", "", messageObject{}, workNone}, + {"attachment with no uploader", "", messageObject{Attachments: []attachment{{Type: "doc"}}}, workNone}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := r.classify(tt.text, tt.msg); got.kind != tt.want { + t.Errorf("classify kind = %d, want %d", got.kind, tt.want) + } + }) + } +} + +// stubUploader satisfies the uploader seam; classify only checks it is present. +type stubUploader struct{} + +func (stubUploader) Save(_ context.Context, _ int64, _, _ string) (string, error) { return "", nil } +func (stubUploader) MaxBytes() int64 { return 1 << 20 } + +// TestClassifyCarriesTheAttachment: every handler dereferences its argument +// unconditionally, so the decision must hand over the attachment it found rather +// than leave the dispatch to look again. +func TestClassifyCarriesTheAttachment(t *testing.T) { + r := newTestReceiver(&fakeService{}, &fakeNotice{}, false, nil) + r.uploads = stubUploader{} + + got := r.classify("", messageObject{Attachments: []attachment{{ + Type: "doc", Doc: &docAttachment{URL: "https://vk.example/doc", Title: "notes.txt"}, + }}}) + if got.kind != workDoc { + t.Fatalf("kind = %d, want workDoc", got.kind) + } + if got.doc == nil { + t.Fatal("classify returned workDoc with no document; the handler would panic") + } +} + +// TestUnauthorizedNoticeIsSentOncePerPeer: without the rate limiter throttling +// it — the gate now runs before the guards, so a refused message costs the user +// nothing — the restraint has to live here. A busy conversation would otherwise +// get a refusal for every message. +func TestUnauthorizedNoticeIsSentOncePerPeer(t *testing.T) { + svc := &fakeService{} + notices := &fakeNotice{} + r := unauthorizedCodexReceiver(t, svc, notices) + + for range 5 { + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{FromID: 42, PeerID: 42, Text: "build it"})) + } + + if got := notices.awaitOne(t); !strings.Contains(got[0], "/login") { + t.Errorf("notice = %q, want it to point at /login", got[0]) + } + if len(svc.handleCalls) != 0 { + t.Errorf("started %d runs while unauthorized, want 0", len(svc.handleCalls)) + } +} + +// TestUnauthorizedNoticeReturnsAfterAuthorization: told once, but told again the +// next time the deployment lapses — otherwise a returning outage is silent. +func TestUnauthorizedNoticeReturnsAfterAuthorization(t *testing.T) { + svc := &fakeService{} + notices := &fakeNotice{} + auth, home := logintest.Unauthorized(t) + r := newTestReceiverWithAuth(svc, notices, auth) + + send := func() { + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{FromID: 42, PeerID: 42, Text: "build it"})) + } + send() + notices.awaitOne(t) + + // Authorized: the message runs, and the "already told them" memory resets. + if err := os.WriteFile(filepath.Join(home, "auth.json"), []byte("{}"), 0o600); err != nil { + t.Fatalf("write auth.json: %v", err) + } + send() + if len(svc.handleCalls) != 1 { + t.Fatalf("Handle calls = %d, want 1 once authorized", len(svc.handleCalls)) + } + + // Lapsed again: announced afresh. + if err := os.Remove(filepath.Join(home, "auth.json")); err != nil { + t.Fatalf("remove auth.json: %v", err) + } + send() + waitUntilNotices(t, notices, 2) +} + +// TestUserIsAnsweredEvenAfterABackgroundRefusal is the scenario this split +// exists for: a container comes up with a lost auth.json, the restart replay +// refuses and explains to the chat, and the person who then writes must STILL be +// answered. Sharing one registry between the two channels met them with silence, +// in the one feature whose whole job is telling a human that /login is needed. +func TestUserIsAnsweredEvenAfterABackgroundRefusal(t *testing.T) { + svc := &fakeService{} + notices := &fakeNotice{} + auth, _ := logintest.Unauthorized(t) + r := newTestReceiverWithAuth(svc, notices, auth) + + // A background submission is refused first and takes its own notice. + if background := auth.NoticeFor("42"); background == "" { + t.Fatal("the background channel said nothing while unauthorized") + } + + r.dispatch(context.Background(), msgNewUpdate(t, messageObject{FromID: 42, PeerID: 42, Text: "build it"})) + + if got := notices.awaitOne(t); !strings.Contains(got[0], "/login") { + t.Errorf("notices = %v, want the user answered despite the background refusal", got) + } +} diff --git a/cmd/duck-vk/main.go b/cmd/duck-vk/main.go index 28fa443..17a0b87 100644 --- a/cmd/duck-vk/main.go +++ b/cmd/duck-vk/main.go @@ -78,33 +78,19 @@ func run() int { })) slog.SetDefault(logger) - runner, opts, provider, err := airunner.Build(cfg) + runner, opts, provider, auth, err := airunner.BuildProvider(cfg, logger) if err != nil { - logger.Error("invalid ai provider config", "provider", cfg.AIBackend, "error", err) return 1 } - if provider.Name == config.AIBackendCodex { - logger.Info("codex backend enabled", - "provider", provider.Name, - "display_name", provider.DisplayName, - "auth_mode", cfg.CodexAuthModeName(), - "sandbox", cfg.CodexSandbox, - "approval_policy", cfg.CodexApprovalPolicy, - "codex_home", cfg.CodexHome, - "capabilities", provider.Capabilities, - ) - } else { - logger.Info("ai provider enabled", - "provider", provider.Name, - "display_name", provider.DisplayName, - "model", opts.Model, - "capabilities", provider.Capabilities, - ) - } ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer cancel() + // A pending device login is deliberately detached from the update that + // started it, so nothing else would ever end it on shutdown: the polling + // Codex child would outlive this process as an orphan. + defer auth.Cancel() + // Best-effort git startup wiring (identity, default branch, inline credential // helper). Non-fatal: a git setup failure must not crash-loop the bot. if err := gitsetup.Apply(ctx, gitsetup.Config{ @@ -238,7 +224,13 @@ func run() int { Opts: opts, Timeout: cfg.ClaudeTimeout(), RetryAfter: vk.RetryAfter, - Logger: logger, + // The provider gate on every submit path — a user message and edits of one, a + // cron fire, the autonomy path (follow-ups, CI events, fix-ups), a poller + // relay, the restart replay. The adapters block the message path early + // (before paid work); this is what stops the rest from marching into an + // unauthenticated CLI and stranding a pending marker per fire. + Auth: auth, + Logger: logger, }) // One-shot follow-ups (the workspace followup/.md convention). @@ -266,6 +258,7 @@ func run() int { Notices: vk.NewNoticeSender(api, time.Now().UnixNano(), logger), EventAck: api.SendMessageEventAnswer, Scheduler: mgr, + CodexAuth: auth, Logger: logger, }) @@ -294,6 +287,7 @@ func run() int { logger.Warn("poller: bad chat id in branch", "chat_id", c.ChatID) continue } + //nolint:contextcheck // detached by design: the run (and its notice) must outlive this poll/replay context. svc.Inject(c.ChatID, formatPRComment(c)) } } @@ -350,7 +344,10 @@ func buildScheduler(ctx context.Context, cfg config.Config, svc *chat.Service, l // accumulate detached goroutines or durable markers. The creator is re-validated // against the live VK allow-list at fire time (cfg.IsVKAllowed) so a de-listed // user's stored jobs stop running and are pruned. Mirrors cmd/flock-telegram. - mgr := schedule.NewManager(store, svc.InjectScheduled, cfg.IsVKAllowed, time.Now, logger) + // svc.RunsBlocked pauses the whole tick while the provider is unauthorized: + // TickOnce records a matching minute whether or not the fire took, so ticking + // into a refusal would spend the job's occurrence on nothing. + mgr := schedule.NewManager(store, svc.InjectScheduled, cfg.IsVKAllowed, svc.RunsBlocked, time.Now, logger) go func() { if err := mgr.Run(ctx); err != nil && ctx.Err() == nil { logger.Error("scheduler stopped", "error", err) diff --git a/cmd/flock-telegram/codex_login_test.go b/cmd/flock-telegram/codex_login_test.go new file mode 100644 index 0000000..489a3f4 --- /dev/null +++ b/cmd/flock-telegram/codex_login_test.go @@ -0,0 +1,393 @@ +package main + +import ( + "context" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/go-telegram/bot" + "github.com/go-telegram/bot/models" + + "github.com/duckbugio/flock/adapters/telegram" + "github.com/duckbugio/flock/core/chat" + "github.com/duckbugio/flock/core/codexauth" + "github.com/duckbugio/flock/core/codexauth/codexauthtest" + "github.com/duckbugio/flock/core/codexauth/logintest" + "github.com/duckbugio/flock/internal/config" +) + +// unauthorizedCodexAuth returns a manager for a Codex deploy that has never been +// signed in: subscription mode over an empty CODEX_HOME (no auth.json). It also +// returns that home so a test can "complete" the login by planting the file. +func unauthorizedCodexAuth(t *testing.T) (*codexauth.Manager, string) { + t.Helper() + return logintest.Unauthorized(t) +} + +// The allowed sender and the private chat every case in this file uses. +const ( + loginTestUserID int64 = 42 + loginTestChatID int64 = 200 +) + +// textMessage builds a plain private-chat text message from the allowed sender. +func textMessage(text string) *models.Message { + return &models.Message{ + ID: 1, + Text: text, + From: &models.User{ID: loginTestUserID}, + Chat: models.Chat{ID: loginTestChatID, Type: models.ChatTypePrivate}, + } +} + +// quietBot builds a bot that answers every API call locally, so a handler that +// posts a reply is a no-op with no network. +func quietBot(t *testing.T) *bot.Bot { + t.Helper() + b, err := bot.New("123456:test-token", + bot.WithSkipGetMe(), + bot.WithNotAsyncHandlers(), + bot.WithHTTPClient(time.Minute, stubHTTPClient{}), + ) + if err != nil { + t.Fatalf("build test bot: %v", err) + } + return b +} + +// TestHandleMessageBlockedWhileCodexUnauthorized: with no completed Codex +// sign-in, a normal message must NOT reach the run Service — it would only fail +// deep inside the CLI, after paying for any transcription or download on the way. +func TestHandleMessageBlockedWhileCodexUnauthorized(t *testing.T) { + auth, _ := unauthorizedCodexAuth(t) + svc := &recordingSubmitter{} + cfg := config.Config{AllowedUsers: []int64{loginTestUserID}} + deps := messageDeps{cfg: cfg, service: svc, auth: auth} + + handleMessage(context.Background(), deps, quietBot(t), textMessage("build it"), false) + + if got := svc.seen(); len(got) != 0 { + t.Errorf("submitted %v while Codex was unauthorized, want nothing", got) + } +} + +// TestHandleMessageUnblocksAfterLogin: once the login has persisted auth.json the +// standard flow continues — in the SAME process, with no restart. That is the +// whole point of re-checking the file rather than caching a startup verdict. +func TestHandleMessageUnblocksAfterLogin(t *testing.T) { + auth, home := unauthorizedCodexAuth(t) + svc := &recordingSubmitter{} + cfg := config.Config{AllowedUsers: []int64{loginTestUserID}} + deps := messageDeps{cfg: cfg, service: svc, auth: auth} + b := quietBot(t) + + handleMessage(context.Background(), deps, b, textMessage("build it"), false) + if got := svc.seen(); len(got) != 0 { + t.Fatalf("submitted %v before the login, want nothing", got) + } + + if err := os.WriteFile(filepath.Join(home, "auth.json"), []byte("{}"), 0o600); err != nil { + t.Fatalf("write auth.json: %v", err) + } + + // A distinct prompt, so the assertion cannot pass on the blocked one leaking + // through late. + handleMessage(context.Background(), deps, b, textMessage("ship it"), false) + if got := svc.seen(); len(got) != 1 || got[0] != "ship it" { + t.Errorf("submitted %v after the login, want ['ship it']", got) + } +} + +// TestHandleMessageWithoutCodexAuthManager: the Claude default wires no manager, +// which must never block a message. +func TestHandleMessageWithoutCodexAuthManager(t *testing.T) { + svc := &recordingSubmitter{} + cfg := config.Config{AllowedUsers: []int64{loginTestUserID}} + deps := messageDeps{cfg: cfg, service: svc} + + handleMessage(context.Background(), deps, quietBot(t), textMessage("build it"), false) + + if got := svc.seen(); len(got) != 1 { + t.Errorf("submitted %v with no auth manager, want the message through", got) + } +} + +// TestLoginCommandIsRoutedToItsHandler: /login is reserved, so it must be served +// by the bot and never forwarded to the model as a prompt — and it must stay +// reachable while the unauthorized gate is closed, since it is the way out. +func TestLoginCommandIsRoutedToItsHandler(t *testing.T) { + auth, _ := unauthorizedCodexAuth(t) + svc := &recordingSubmitter{} + cfg := config.Config{AllowedUsers: []int64{loginTestUserID}} + + defaultHandler := func(ctx context.Context, b *bot.Bot, update *models.Update) { + if msg := update.Message; msg != nil { + handleMessage(ctx, messageDeps{cfg: cfg, service: svc, auth: auth}, b, msg, false) + } + } + b, err := bot.New("123456:test-token", + bot.WithSkipGetMe(), + bot.WithNotAsyncHandlers(), + bot.WithHTTPClient(time.Minute, stubHTTPClient{}), + bot.WithDefaultHandler(defaultHandler), + ) + if err != nil { + t.Fatalf("build test bot: %v", err) + } + for name, h := range reservedHandlers(cfg, nil, nil, auth) { + b.RegisterHandlerMatchFunc(commandMatch(name), h) + } + + b.ProcessUpdate(context.Background(), privateCommandUpdate(loginTestUserID, loginTestChatID, "/login status", len("/login"))) + + if got := svc.seen(); len(got) != 0 { + t.Errorf("/login leaked to the model as %v, want it handled by the bot", got) + } +} + +// TestLoginIsAReservedCommand pins /login into the canonical set, which is what +// makes both adapters intercept it and Telegram publish it in the command menu. +func TestLoginIsAReservedCommand(t *testing.T) { + if !chat.IsReservedCommand("login") { + t.Fatal("login is not a reserved command; it would be forwarded to the model") + } +} + +// groupCommandUpdate builds a GROUP-chat slash command from the allowed sender. +func groupCommandUpdate(text string, cmdLen int) *models.Update { + return &models.Update{Message: &models.Message{ + ID: 1, + Text: text, + From: &models.User{ID: loginTestUserID}, + Chat: models.Chat{ID: loginTestChatID, Type: models.ChatTypeGroup}, + Entities: []models.MessageEntity{{Type: models.MessageEntityTypeBotCommand, Offset: 0, Length: cmdLen}}, + }} +} + +// TestLoginRefusedInGroupChat: the reply goes to the CHAT, so in a group the +// verification link and one-time code would be readable by every member, +// allow-listed or not — and whoever acts on the code first binds THEIR ChatGPT +// account to the bot. The refusal must happen before any CLI is spawned. +func TestLoginRefusedInGroupChat(t *testing.T) { + // A deliberately unusable CODEX_BIN: if the refusal failed to short-circuit, + // the manager would try to run it, which the status assertion below catches. + auth, _ := logintest.Unauthorized(t) + + got := groupCommandReplies(t, auth, "/login") + if len(got) != 1 { + t.Fatalf("replies = %v, want exactly one", got) + } + if !strings.Contains(got[0], "direct message") { + t.Errorf("reply = %q, want the direct-message refusal", got[0]) + } + if strings.Contains(got[0], "http") { + t.Errorf("reply = %q leaked a link into the group", got[0]) + } + if auth.StatusText() != "Codex is NOT authorized. Send /login to sign in with your ChatGPT account." { + t.Errorf("a sign-in was started from a group chat: %q", auth.StatusText()) + } +} + +// TestLoginStatusInGroupHidesAPendingCode is the regression for the hole an +// argument-based guard left: /login status ALSO re-shows a pending one-time code, +// so refusing only the code-starting arguments let it into the group anyway. +func TestLoginStatusInGroupHidesAPendingCode(t *testing.T) { + auth := logintest.PendingLogin(t) + + got := groupCommandReplies(t, auth, "/login status") + if len(got) != 1 { + t.Fatalf("replies = %v, want exactly one", got) + } + if strings.Contains(got[0], codexauthtest.DeviceCode) || strings.Contains(got[0], "http") { + t.Errorf("the pending code or link reached a group: %q", got[0]) + } + if !strings.Contains(got[0], "in progress") { + t.Errorf("status = %q, want it to still report the pending sign-in", got[0]) + } +} + +// TestLoginStatusInPrivateShowsThePendingCode: the same command in a direct +// message must still re-show the code — that is what makes a lost message +// recoverable. +func TestLoginStatusInPrivateShowsThePendingCode(t *testing.T) { + auth := logintest.PendingLogin(t) + cfg := config.Config{AllowedUsers: []int64{loginTestUserID}} + replies := &capturingHTTPClient{} + b := commandBot(t, cfg, auth, replies) + + b.ProcessUpdate(context.Background(), + privateCommandUpdate(loginTestUserID, loginTestChatID, "/login status", len("/login"))) + + got := replies.seen() + if len(got) != 1 || !strings.Contains(got[0], codexauthtest.DeviceCode) { + t.Errorf("replies = %v, want the pending code re-shown in a direct message", got) + } +} + +// commandBot builds a bot with the reserved handlers registered and every API +// call answered locally by http. +func commandBot(t *testing.T, cfg config.Config, auth *codexauth.Manager, client bot.HttpClient) *bot.Bot { + t.Helper() + b, err := bot.New("123456:test-token", + bot.WithSkipGetMe(), + bot.WithNotAsyncHandlers(), + bot.WithHTTPClient(time.Minute, client), + ) + if err != nil { + t.Fatalf("build test bot: %v", err) + } + for name, h := range reservedHandlers(cfg, nil, nil, auth) { + b.RegisterHandlerMatchFunc(commandMatch(name), h) + } + return b +} + +// groupCommandReplies runs text as a group slash command and returns what the +// bot replied. +func groupCommandReplies(t *testing.T, auth *codexauth.Manager, text string) []string { + t.Helper() + replies := &capturingHTTPClient{} + b := commandBot(t, config.Config{AllowedUsers: []int64{loginTestUserID}}, auth, replies) + b.ProcessUpdate(context.Background(), groupCommandUpdate(text, len("/login"))) + return replies.seen() +} + +// capturingHTTPClient answers every Bot API call locally like stubHTTPClient, +// but records the raw payload of each sendMessage so a test can assert what the +// user was actually told (the encoding varies, so assertions substring-match). +type capturingHTTPClient struct { + mu sync.Mutex + texts []string +} + +func (c *capturingHTTPClient) Do(req *http.Request) (*http.Response, error) { + if req.Body != nil && strings.Contains(req.URL.Path, "sendMessage") { + body, _ := io.ReadAll(req.Body) + c.mu.Lock() + c.texts = append(c.texts, string(body)) + c.mu.Unlock() + } + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(`{"ok":true,"result":{"message_id":1}}`)), + Header: make(http.Header), + }, nil +} + +func (c *capturingHTTPClient) seen() []string { + c.mu.Lock() + defer c.mu.Unlock() + return append([]string(nil), c.texts...) +} + +// TestLoginStatusInGroupWithNothingPending: with no sign-in in flight there is +// no code to withhold, so the group is simply told the state. +func TestLoginStatusInGroupWithNothingPending(t *testing.T) { + auth, _ := unauthorizedCodexAuth(t) + + got := groupCommandReplies(t, auth, "/login status") + if len(got) != 1 { + t.Fatalf("replies = %v, want exactly one", got) + } + if !strings.Contains(got[0], "NOT authorized") { + t.Errorf("reply = %q, want the status, not a refusal", got[0]) + } +} + +// codexAuthForMenu is a manager for a Codex subscription deploy, i.e. one where +// every reserved command — including /login — applies. +var codexAuthForMenu = codexauth.NewManager(codexauth.Config{ + Backend: codexauth.BackendCodex, + AuthMode: codexauth.AuthSubscription, +}) + +// TestMenuOmitsLoginWithoutAnInteractiveSignIn: on Claude, an API-key backend, +// Codex billing, or a Codex deploy already carrying CODEX_ACCESS_TOKEN there is +// no sign-in to complete, so advertising /login would only lead a user to a +// command that answers "not needed". The handler stays registered, so typing it +// still gets that answer — and /login force still works, which is why the +// manager stays Applicable. +func TestMenuOmitsLoginWithoutAnInteractiveSignIn(t *testing.T) { + for _, tt := range []struct { + name string + auth *codexauth.Manager + }{ + {"claude", codexauth.NewManager(codexauth.Config{Backend: "claude"})}, + {"codex billing", codexauth.NewManager(codexauth.Config{ + Backend: codexauth.BackendCodex, AuthMode: codexauth.AuthBilling, + })}, + {"codex subscription with an access token", codexauth.NewManager(codexauth.Config{ + Backend: codexauth.BackendCodex, AuthMode: codexauth.AuthSubscription, HasAccessToken: true, + })}, + {"no manager", nil}, + } { + t.Run(tt.name, func(t *testing.T) { + for _, c := range reservedBotCommands(tt.auth) { + if c.Command == "login" { + t.Errorf("/login is published in the menu on a deployment with no sign-in") + } + } + if got, want := len(reservedBotCommands(tt.auth)), len(chat.ReservedCommands)-1; got != want { + t.Errorf("menu has %d entries, want %d (everything but /login)", got, want) + } + }) + } +} + +// TestMenuKeepsLoginOnCodexSubscription is the other half: where the sign-in is +// real, the command must be advertised. +func TestMenuKeepsLoginOnCodexSubscription(t *testing.T) { + found := false + for _, c := range reservedBotCommands(codexAuthForMenu) { + if c.Command == "login" { + found = true + } + } + if !found { + t.Error("/login is missing from the menu on a Codex subscription deployment") + } +} + +// TestHelpListsLoginOnlyWhereItApplies keeps /help and the published command menu +// on the SAME condition. The menu hides /login where no sign-in exists; a help +// text that still advertises it would send the user to a command that can only +// answer "not needed". +func TestHelpListsLoginOnlyWhereItApplies(t *testing.T) { + if !strings.Contains(telegram.HelpText(chat.WithLogin), "/login") { + t.Error("help omits /login where the sign-in is real") + } + if strings.Contains(telegram.HelpText(chat.WithoutLogin), "/login") { + t.Error("help advertises /login where there is no sign-in") + } + if !strings.Contains(telegram.WelcomeText(chat.WithLogin), "/login") { + t.Error("welcome omits /login where the sign-in is real") + } + if strings.Contains(telegram.WelcomeText(chat.WithoutLogin), "/login") { + t.Error("welcome advertises /login where there is no sign-in") + } + + // The menu and the help must agree, which is the whole point of one condition. + menuHasLogin := false + for _, c := range reservedBotCommands(codexAuthForMenu) { + if c.Command == loginCommand { + menuHasLogin = true + } + } + if menuHasLogin != strings.Contains(telegram.HelpText(chat.LoginVisibilityFor(codexAuthForMenu.Applicable())), "/login") { + t.Error("the command menu and /help disagree about /login") + } +} + +// TestHelpLineComesFromTheCanonicalSet: both adapters render the same /login help +// line from core/chat, so a wording change cannot land in one and not the other. +func TestHelpLineComesFromTheCanonicalSet(t *testing.T) { + if !strings.Contains(telegram.HelpText(chat.WithLogin), chat.LoginHelpLine) { + t.Error("the Telegram help does not render the canonical /login line") + } +} diff --git a/cmd/flock-telegram/commands_test.go b/cmd/flock-telegram/commands_test.go index 59a7f9c..54de99d 100644 --- a/cmd/flock-telegram/commands_test.go +++ b/cmd/flock-telegram/commands_test.go @@ -72,9 +72,11 @@ func TestCommandSenderAllowList(t *testing.T) { // TestReservedBotCommandsFromCanonicalSet asserts the published Telegram command // menu is built one-for-one from core/chat.ReservedCommands (the single source of // truth) — same names, same order, same descriptions — so the menu can never -// drift from the commands the handlers route on. +// drift from the commands the handlers route on. The manager here is a Codex +// subscription one, for which every reserved command applies; see +// TestMenuOmitsLoginWithoutAnInteractiveSignIn for the one conditional entry. func TestReservedBotCommandsFromCanonicalSet(t *testing.T) { - got := reservedBotCommands() + got := reservedBotCommands(codexAuthForMenu) if len(got) != len(chat.ReservedCommands) { t.Fatalf("reservedBotCommands has %d entries, want %d", len(got), len(chat.ReservedCommands)) } @@ -127,10 +129,10 @@ func TestStartCommandRouted(t *testing.T) { // TestStartWelcomeText asserts the static /start reply is the welcome text: a // short greeting prepended to the usage help, plain English with no duck flavor. func TestStartWelcomeText(t *testing.T) { - if !strings.HasPrefix(telegram.WelcomeText, "Hi!") { - t.Fatalf("WelcomeText should open with a greeting, got %q", telegram.WelcomeText) + if !strings.HasPrefix(telegram.WelcomeText(chat.WithoutLogin), "Hi!") { + t.Fatalf("WelcomeText should open with a greeting, got %q", telegram.WelcomeText(chat.WithoutLogin)) } - if !strings.Contains(telegram.WelcomeText, telegram.HelpText) { + if !strings.Contains(telegram.WelcomeText(chat.WithoutLogin), telegram.HelpText(chat.WithoutLogin)) { t.Fatalf("WelcomeText should include the usage help (HelpText)") } } @@ -143,9 +145,9 @@ func TestStartWelcomeText(t *testing.T) { // Claude as free text. Keying both off the same source and asserting equality makes // that impossible. func TestReservedHandlersMatchCanonicalSet(t *testing.T) { - // A nil *chat.Service and nil *schedule.Manager are fine: we only inspect the - // map's key set, never invoke a handler closure. - handlers := reservedHandlers(config.Config{}, nil, nil) + // A nil *chat.Service, *schedule.Manager and *codexauth.Manager are fine: we + // only inspect the map's key set, never invoke a handler closure. + handlers := reservedHandlers(config.Config{}, nil, nil, nil) if len(handlers) != len(chat.ReservedCommands) { t.Fatalf("reservedHandlers has %d entries, want %d (chat.ReservedCommands)", @@ -258,7 +260,7 @@ func newCommandTestBot(t *testing.T, cfg config.Config, svc messageSubmitter, re // Register the reserved-command handlers from the SAME canonical source main uses, // so the routing (reserved handler vs default) is identical to production. The // scheduler is nil here (the pass-through tests do not exercise /schedule). - for name, h := range reservedHandlers(cfg, reservedSvc, nil) { + for name, h := range reservedHandlers(cfg, reservedSvc, nil, nil) { b.RegisterHandlerMatchFunc(commandMatch(name), h) } return b @@ -337,7 +339,7 @@ func TestScheduleCommandEnabledReachesDispatch(t *testing.T) { if err != nil { t.Fatalf("open store: %v", err) } - mgr := schedule.NewManager(store, func(string, string, int64) bool { return true }, nil, nil, nil) + mgr := schedule.NewManager(store, func(string, string, int64) bool { return true }, nil, nil, nil, nil) defaultHandler := func(ctx context.Context, b *bot.Bot, update *models.Update) { if msg := update.Message; msg != nil { @@ -354,7 +356,7 @@ func TestScheduleCommandEnabledReachesDispatch(t *testing.T) { if err != nil { t.Fatalf("build test bot: %v", err) } - for name, h := range reservedHandlers(cfg, nil, mgr) { + for name, h := range reservedHandlers(cfg, nil, mgr, nil) { b.RegisterHandlerMatchFunc(commandMatch(name), h) } diff --git a/cmd/flock-telegram/main.go b/cmd/flock-telegram/main.go index f762437..0fe49cb 100644 --- a/cmd/flock-telegram/main.go +++ b/cmd/flock-telegram/main.go @@ -28,6 +28,7 @@ import ( "github.com/duckbugio/flock/core/agent" "github.com/duckbugio/flock/core/chat" "github.com/duckbugio/flock/core/claude" + "github.com/duckbugio/flock/core/codexauth" "github.com/duckbugio/flock/core/cost" "github.com/duckbugio/flock/core/dispatch" "github.com/duckbugio/flock/core/ghstar" @@ -76,33 +77,19 @@ func run() int { })) slog.SetDefault(logger) - runner, opts, provider, err := airunner.Build(cfg) + runner, opts, provider, auth, err := airunner.BuildProvider(cfg, logger) if err != nil { - logger.Error("invalid ai provider config", "provider", cfg.AIBackend, "error", err) return 1 } - if provider.Name == config.AIBackendCodex { - logger.Info("codex backend enabled", - "provider", provider.Name, - "display_name", provider.DisplayName, - "auth_mode", cfg.CodexAuthModeName(), - "sandbox", cfg.CodexSandbox, - "approval_policy", cfg.CodexApprovalPolicy, - "codex_home", cfg.CodexHome, - "capabilities", provider.Capabilities, - ) - } else { - logger.Info("ai provider enabled", - "provider", provider.Name, - "display_name", provider.DisplayName, - "model", opts.Model, - "capabilities", provider.Capabilities, - ) - } ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer cancel() + // A pending device login is deliberately detached from the update that + // started it, so nothing else would ever end it on shutdown: the polling + // Codex child would outlive this process as an orphan. + defer auth.Cancel() + // Best-effort git startup wiring (identity, default branch, and — when a host // + token are set — an inline credential helper that reads the token from the // env at call time). Non-fatal: a git setup failure must not crash-loop the bot. @@ -221,7 +208,7 @@ func run() int { guards := chat.GuardConfig{CostCapUSD: cfg.EffectiveCostCapUSD()} opts2 := []bot.Option{ - bot.WithDefaultHandler(textHandler(cfg, &svc, &vt, &up, limiter, costs, guards)), + bot.WithDefaultHandler(textHandler(cfg, &svc, &vt, &up, limiter, costs, guards, auth)), } b, err := bot.New(cfg.TelegramBotToken, opts2...) @@ -310,7 +297,13 @@ func run() int { Opts: opts, Timeout: cfg.ClaudeTimeout(), RetryAfter: telegram.RetryAfter, - Logger: logger, + // The provider gate on every submit path — a user message and edits of one, a + // cron fire, the autonomy path (follow-ups, CI events, fix-ups), a poller + // relay, the restart replay. The adapters block the message path early + // (before paid work); this is what stops the rest from marching into an + // unauthenticated CLI and stranding a pending marker per fire. + Auth: auth, + Logger: logger, }) // Background cron scheduler (OFF by default). When enabled, open the durable @@ -348,7 +341,7 @@ func run() int { // explicit address). The set of names handled here is asserted, by test, to equal // the canonical chat.ReservedCommands so a new reserved command can never be // published in the menu yet leak to the model for lack of a handler. - handlers := reservedHandlers(cfg, svc, mgr) + handlers := reservedHandlers(cfg, svc, mgr, auth) for name, h := range handlers { b.RegisterHandlerMatchFunc(commandMatch(name), h) } @@ -357,7 +350,7 @@ func run() int { // source the handlers above route on). Native Claude commands (/loop, /security-review, …) // are intentionally NOT listed — they pass through as free text. Best-effort: // a failure here only means an empty/stale menu, so log and continue. - if _, err := b.SetMyCommands(ctx, &bot.SetMyCommandsParams{Commands: reservedBotCommands()}); err != nil { + if _, err := b.SetMyCommands(ctx, &bot.SetMyCommandsParams{Commands: reservedBotCommands(auth)}); err != nil { logger.Warn("set telegram command menu", "error", err) } @@ -387,6 +380,7 @@ func run() int { logger.Warn("poller: bad chat id in branch", "chat_id", c.ChatID) continue } + //nolint:contextcheck // detached by design: the run (and its notice) must outlive this poll/replay context. svc.Inject(c.ChatID, formatPRComment(c)) } } @@ -426,6 +420,7 @@ func textHandler( limiter *ratelimit.Limiter, costs *cost.Store, guards chat.GuardConfig, + auth *codexauth.Manager, ) bot.HandlerFunc { return func(ctx context.Context, b *bot.Bot, update *models.Update) { service := *svc @@ -437,12 +432,18 @@ func textHandler( // Voice and media uploads apply only to new messages, never edits, so // neither the transcriber nor the uploader is threaded here. if edited := update.EditedMessage; edited != nil { - deps := messageDeps{cfg: cfg, service: service, limiter: limiter, costs: costs, guards: guards} + deps := messageDeps{ + cfg: cfg, service: service, limiter: limiter, costs: costs, + guards: guards, auth: auth, + } handleMessage(ctx, deps, b, edited, true) return } if msg := update.Message; msg != nil { - deps := messageDeps{cfg: cfg, service: service, vt: *vt, up: *up, limiter: limiter, costs: costs, guards: guards} + deps := messageDeps{ + cfg: cfg, service: service, vt: *vt, up: *up, + limiter: limiter, costs: costs, guards: guards, auth: auth, + } handleMessage(ctx, deps, b, msg, false) } } @@ -478,6 +479,9 @@ type messageDeps struct { limiter *ratelimit.Limiter costs *cost.Store guards chat.GuardConfig + // auth blocks runs while the Codex backend has no completed sign-in. Nil (and + // a nil *Manager) means "never blocks". + auth *codexauth.Manager } // handleMessage applies the allow-list and the group mention-gate to one message @@ -540,6 +544,28 @@ func handleMessage(ctx context.Context, deps messageDeps, b *bot.Bot, msg *model // call and is never submitted to the dispatcher; the user gets a single // professional notice. Disallowed users already returned above, so the guards // only ever see allow-listed senders. + // Codex with no completed sign-in cannot run anything. The check comes BEFORE + // the guards, and so before the rate limiter counts this message: a run that + // cannot happen must not spend the user's window, or the first real messages + // after a successful /login would hit the limit. It is one os.Stat, and no paid + // work precedes it. Only for a message that WOULD have run — a sticker or an + // empty message is dropped silently below, and answering those would explain + // work nobody asked for. The notice is sent once per chat until authorization + // returns; without the limiter throttling it, the restraint belongs here. + if strings.TrimSpace(cleaned) != "" || isVoice || isDocument || isPhoto { + if deps.auth.Blocked() { + // Warn, matching core/chat and the VK adapter: one state, one level. + slog.Warn("codex unauthorized — refusing run", "chat_id", msg.Chat.ID, "user_id", msg.From.ID) + // ReplyNoticeFor, not NoticeFor: a refused background submission must not + // consume the answer this person is owed. + if notice := deps.auth.ReplyNoticeFor(chatIDStr(msg.Chat.ID)); notice != "" { + sendCommandReply(ctx, b, msg.Chat.ID, notice) + } + return + } + deps.auth.NoticeReset() + } + if allow, reason := chat.CheckGuards(limiter, costs, guards, msg.From.ID); !allow { slog.Debug("guardrail denied message", "chat_id", msg.Chat.ID, "user_id", msg.From.ID, "reason", reason) sendCommandReply(ctx, b, msg.Chat.ID, reason) @@ -953,9 +979,16 @@ func starHandler(cfg config.Config, svc *chat.Service) bot.HandlerFunc { // reservedBotCommands maps the canonical reserved set (core/chat) to the Telegram // command-menu model published via SetMyCommands. It is the single place the menu // is built, so the menu can never drift from the commands the handlers route on. -func reservedBotCommands() []models.BotCommand { +func reservedBotCommands(auth *codexauth.Manager) []models.BotCommand { cmds := make([]models.BotCommand, 0, len(chat.ReservedCommands)) for _, c := range chat.ReservedCommands { + // /login is meaningless on a deployment with no interactive sign-in (Claude, + // an API-key backend, Codex billing): it would sit in the menu only to answer + // that it is not needed. The handler stays registered either way, so a user + // who types it still gets that answer. + if c.Name == loginCommand && !auth.LoginAdvertised() { + continue + } cmds = append(cmds, models.BotCommand{Command: c.Name, Description: c.Description}) } return cmds @@ -969,17 +1002,63 @@ func reservedBotCommands() []models.BotCommand { // asserts this map's key set equals chat.ReservedCommands. Adding a reserved command // is therefore a two-line change here (plus the canonical entry); forgetting the // handler fails the test rather than silently leaking the command to Claude. -func reservedHandlers(cfg config.Config, svc *chat.Service, sched *schedule.Manager) map[string]bot.HandlerFunc { +func reservedHandlers( + cfg config.Config, svc *chat.Service, sched *schedule.Manager, auth *codexauth.Manager, +) map[string]bot.HandlerFunc { return map[string]bot.HandlerFunc{ - "start": startHandler(cfg), - "help": helpHandler(cfg), - "new": newHandler(cfg, svc), - "stop": stopCommandHandler(cfg, svc), - "schedule": scheduleHandler(cfg, sched), - "goal": goalHandler(cfg, svc), + "start": startHandler(cfg, auth), + "help": helpHandler(cfg, auth), + "new": newHandler(cfg, svc), + "stop": stopCommandHandler(cfg, svc), + "schedule": scheduleHandler(cfg, sched), + "goal": goalHandler(cfg, svc), + loginCommand: loginHandler(cfg, auth), } } +// loginHandler serves /login: it starts (or reports, or cancels) the Codex +// device-code sign-in. The command is deliberately available even while the +// deployment is unauthorized — it is the only way out of that state — and, like +// every reserved handler, is allow-list gated and never starts a run. +// +// Whether a one-time code may be printed here is decided by codexauth from the +// Private flag, not by this handler inspecting the arguments: /login status also +// re-shows a pending code, so an argument-based guard would let it straight past. +// +// The device flow outlives this handler: Dispatch returns an immediate reply and +// keeps working in the background, delivering the link, the one-time code, and +// the final verdict through the notify callback. That callback therefore builds +// its OWN context: the update's ctx is cancelled as soon as the handler returns, +// minutes before a user finishes signing in, and sending on it would fail. +func loginHandler(cfg config.Config, auth *codexauth.Manager) bot.HandlerFunc { + return func(ctx context.Context, b *bot.Bot, update *models.Update) { + chatID, ok := commandSender(cfg, update.Message) + if !ok { + return + } + // One bounded, detached sender for BOTH paths, as in the VK adapter: /login + // cancel waits out the login goroutine, so the update's own context can be + // gone by the time the reply is sent and the answer would vanish silently. + notify := func(text string) { + sendCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), codexauth.NotifyTimeout) + defer cancel() + sendCommandReply(sendCtx, b, chatID, text) + } + sub := codexauth.Subscriber{ + ID: chatIDStr(chatID), + Private: update.Message.Chat.Type == models.ChatTypePrivate, + Notify: notify, + } + // An empty reply means Dispatch already delivered it through sub, in order. + if reply := auth.Dispatch(ctx, commandArgs(update.Message.Text), sub); reply != "" { + notify(reply) + } + } +} + +// loginCommand is the reserved command name the sign-in flow is published under. +const loginCommand = "login" + // commandMatch routes a message whose leading bot command is name, tolerating // the @botname suffix Telegram adds in groups (/new@duck_bot). It only routes; // the allow-list gate lives in each handler via commandSender. @@ -1010,25 +1089,25 @@ func commandSender(cfg config.Config, msg *models.Message) (int64, bool) { // allow-list, is not mention-gated, and never touches the dispatcher or session // store (no Claude run) — without it /start would fall through to the text // handler and be forwarded to the model as a prompt. -func startHandler(cfg config.Config) bot.HandlerFunc { +func startHandler(cfg config.Config, auth *codexauth.Manager) bot.HandlerFunc { return func(ctx context.Context, b *bot.Bot, update *models.Update) { chatID, ok := commandSender(cfg, update.Message) if !ok { return } - sendCommandReply(ctx, b, chatID, telegram.WelcomeText) + sendCommandReply(ctx, b, chatID, telegram.WelcomeText(chat.LoginVisibilityFor(auth.LoginAdvertised()))) } } // helpHandler replies to /help from an allowed user with the static usage text. // It never touches the dispatcher or session store (no Claude run). -func helpHandler(cfg config.Config) bot.HandlerFunc { +func helpHandler(cfg config.Config, auth *codexauth.Manager) bot.HandlerFunc { return func(ctx context.Context, b *bot.Bot, update *models.Update) { chatID, ok := commandSender(cfg, update.Message) if !ok { return } - sendCommandReply(ctx, b, chatID, telegram.HelpText) + sendCommandReply(ctx, b, chatID, telegram.HelpText(chat.LoginVisibilityFor(auth.LoginAdvertised()))) } } @@ -1128,7 +1207,10 @@ func buildScheduler(ctx context.Context, cfg config.Config, svc *chat.Service, l // cron cannot accumulate detached goroutines or durable markers. The creator is // re-validated against the live allow-list at fire time (cfg.IsAllowed) so a // de-listed user's stored jobs stop running and are pruned. - mgr := schedule.NewManager(store, svc.InjectScheduled, cfg.IsAllowed, time.Now, logger) + // svc.RunsBlocked pauses the whole tick while the provider is unauthorized: + // TickOnce records a matching minute whether or not the fire took, so ticking + // into a refusal would spend the job's occurrence on nothing. + mgr := schedule.NewManager(store, svc.InjectScheduled, cfg.IsAllowed, svc.RunsBlocked, time.Now, logger) go func() { if err := mgr.Run(ctx); err != nil && ctx.Err() == nil { logger.Error("scheduler stopped", "error", err) @@ -1195,6 +1277,7 @@ func resumePending( } } } + //nolint:contextcheck // detached by design: the run (and its notice) must outlive this poll/replay context. svc.ResumePending(chatID, m) } } diff --git a/core/chat/postrun.go b/core/chat/postrun.go index d276645..aab56ae 100644 --- a/core/chat/postrun.go +++ b/core/chat/postrun.go @@ -82,6 +82,12 @@ type PostRunConfig struct { // resumes the job if the process dies mid-wait. ctx scopes only the budget // notice — the run itself executes under the Dispatcher's per-chat context. func (s *Service) InjectAuto(ctx context.Context, chatID ChatID, prompt string) { + // Before the budget check and before the marker: this path is recurring and + // unattended (the follow-up sweeper, CI events, verify/goal fix-ups), so an + // unauthorized provider would otherwise strand one marker per fire. + if s.blockSubmit(ctx, chatID) { + return + } if !s.autoAllowed(chatID) { s.log.Info("autonomy budget reached; dropping injected run", "chat_id", chatID) s.notifyBudgetOnce(ctx, chatID) diff --git a/core/chat/reserved.go b/core/chat/reserved.go index 40070f0..129f1ca 100644 --- a/core/chat/reserved.go +++ b/core/chat/reserved.go @@ -17,7 +17,7 @@ type ReservedCommand struct { // /security-review) is forwarded verbatim so the provider's own slash-command and // skill system runs it. Both adapters (Telegram, VK) decide what to intercept // from this list, and Telegram builds its command menu from it. The order here is -// the order the menu presents: start, help, new, stop, schedule, goal. +// the order the menu presents: start, help, new, stop, schedule, goal, login. var ReservedCommands = []ReservedCommand{ {Name: "start", Description: "Show a short welcome and usage"}, {Name: "help", Description: "List the bot's own commands"}, @@ -25,8 +25,85 @@ var ReservedCommands = []ReservedCommand{ {Name: "stop", Description: "Stop the run currently in progress"}, {Name: "schedule", Description: "Manage scheduled jobs"}, {Name: "goal", Description: "Arm a goal an independent evaluator re-checks after every run"}, + {Name: "login", Description: "Sign in to Codex on a subscription (send it in a direct message)"}, } +// HelpBody is the shared body of both adapters' /help: the command list and the +// closing line. Only the first line — which names the transport — differs, so +// that is all an adapter supplies. It lives here, beside the canonical set it has +// to stay in step with, for the same reason as LoginHelpLine: two byte-identical +// copies share a predicate, never their words, and drift in silence. +// +// login adds the /login entry, on the same condition that publishes /login in +// Telegram's command menu. +func HelpBody(login LoginVisibility) string { + body := helpCommands + if login == WithLogin { + body += LoginHelpLine + } + return body + helpTail +} + +// LoginVisibility says whether a rendered help text lists /login. It is a named +// type rather than a bare bool so a call site reads as its own documentation: +// HelpText(chat.WithLogin) instead of HelpText(true), which only means something +// after a trip to the declaration. +type LoginVisibility bool + +// Whether /login belongs in a help text. Derive it from the deployment with +// LoginVisibilityFor. +const ( + WithLogin LoginVisibility = true + WithoutLogin LoginVisibility = false +) + +// LoginVisibilityFor maps "does this deployment have an interactive sign-in" onto +// the flag, so callers do not convert a bool by hand. +func LoginVisibilityFor(applicable bool) LoginVisibility { + if applicable { + return WithLogin + } + return WithoutLogin +} + +// HelpText renders an adapter's /help: its own title line, then the shared body. +// The render lives here for the same reason its parts do — two adapters writing the +// same three-term concatenation drift as easily as two copies of the strings. +func HelpText(title string, login LoginVisibility) string { + return title + HelpBody(login) +} + +// WelcomeText renders an adapter's /start: the shared greeting, then its help. +func WelcomeText(title string, login LoginVisibility) string { + return WelcomeGreeting + HelpText(title, login) +} + +// helpCommands lists the reserved commands with the fuller wording a chat reply +// affords, next to the terse Description the command menu uses. +const helpCommands = "/help — show this message\n" + + "/new — start a fresh session (forget the current conversation)\n" + + "/stop — stop the run currently in progress\n" + + "/schedule — manage scheduled jobs (when enabled)\n" + + "/goal — arm a goal an independent evaluator re-checks after every run " + + "(/goal off to disarm)\n" + +// WelcomeGreeting opens the /start reply, ahead of the same help both adapters +// render. It lives here for the reason HelpBody does: two byte-identical copies +// share no link but a habit, and drift without anything failing. +const WelcomeGreeting = "Hi! I'm the Flock assistant.\n\n" + +// helpTail closes the usage message. +const helpTail = "\nSend any other message to run it through the assistant." + +// LoginHelpLine is the /login entry for an adapter's /help text. It lives here, +// beside the canonical command set, because both adapters render it and a second +// copy would drift silently — the only thing they would still share is the +// applicability predicate, not the words. Adapters append it only when an +// interactive sign-in exists, the same condition that publishes /login in +// Telegram's command menu. +const LoginHelpLine = "/login — sign in to Codex on a subscription; send it in a direct message " + + "(/login status, /login cancel)\n" + // IsReservedCommand reports whether name is one of the bot's reserved commands. // The match is case-insensitive (name is normalized to lower case) and exact: // only the bare command word counts, so "new" matches but "news" does not, and a diff --git a/core/chat/reserved_test.go b/core/chat/reserved_test.go index 82e639a..23ee434 100644 --- a/core/chat/reserved_test.go +++ b/core/chat/reserved_test.go @@ -1,6 +1,7 @@ package chat_test import ( + "strings" "testing" "github.com/duckbugio/flock/core/chat" @@ -46,9 +47,9 @@ func TestIsReservedCommand(t *testing.T) { // TestReservedCommandsShape guards the menu source: names are unique, lowercase, // non-empty, every command has a description, and the order is the documented -// start, help, new, stop, schedule, goal. +// start, help, new, stop, schedule, goal, login. func TestReservedCommandsShape(t *testing.T) { - wantOrder := []string{"start", "help", "new", "stop", "schedule", "goal"} + wantOrder := []string{"start", "help", "new", "stop", "schedule", "goal", "login"} if len(chat.ReservedCommands) != len(wantOrder) { t.Fatalf("ReservedCommands has %d entries, want %d", len(chat.ReservedCommands), len(wantOrder)) } @@ -89,3 +90,57 @@ func lower(s string) string { } return string(out) } + +// TestHelpBodyIsTheSharedSource: both adapters render this body, so the command +// list has one home beside the canonical set. Two byte-identical copies would +// share the applicability predicate but never the words, and would drift in +// silence — the reason LoginHelpLine moved here in the first place. +func TestHelpBodyIsTheSharedSource(t *testing.T) { + withLogin := chat.HelpBody(chat.WithLogin) + without := chat.HelpBody(chat.WithoutLogin) + + if !strings.Contains(withLogin, chat.LoginHelpLine) { + t.Error("HelpBody(true) omits the /login line") + } + if strings.Contains(without, "/login") { + t.Error("HelpBody(false) advertises /login where there is no sign-in") + } + // Every reserved command except /login is unconditional, so each must appear. + for _, c := range chat.ReservedCommands { + if c.Name == "login" || c.Name == "start" { + continue // /login is conditional; /start is not listed (it IS the greeting) + } + if !strings.Contains(without, "/"+c.Name) { + t.Errorf("HelpBody omits the reserved command /%s", c.Name) + } + } +} + +// TestLoginHelpLineNamesTheDirectMessageRequirement: this line is rendered in +// group chats and published in Telegram's global command menu, so the one +// requirement that decides whether the command works at all belongs in it. +func TestLoginHelpLineNamesTheDirectMessageRequirement(t *testing.T) { + if !strings.Contains(chat.LoginHelpLine, "direct message") { + t.Errorf("LoginHelpLine = %q, want the direct-message requirement", chat.LoginHelpLine) + } + for _, c := range chat.ReservedCommands { + if c.Name != "login" { + continue + } + if !strings.Contains(c.Description, "direct message") { + t.Errorf("the menu description %q omits the direct-message requirement", c.Description) + } + } +} + +// TestWelcomeGreetingHasOneHome: both adapters open /start with this line, so it +// lives beside the help body they also share. Two byte-identical copies share no +// link but a habit. +func TestWelcomeGreetingHasOneHome(t *testing.T) { + if chat.WelcomeGreeting == "" { + t.Fatal("WelcomeGreeting is empty") + } + if !strings.HasSuffix(chat.WelcomeGreeting, "\n\n") { + t.Errorf("WelcomeGreeting = %q, want it to end with a blank line before the help", chat.WelcomeGreeting) + } +} diff --git a/core/chat/rungate.go b/core/chat/rungate.go new file mode 100644 index 0000000..2e21893 --- /dev/null +++ b/core/chat/rungate.go @@ -0,0 +1,87 @@ +package chat + +import "context" + +// RunGate reports whether the configured AI provider can run at all right now. +// It exists for one case today: a Codex subscription deployment that has not +// completed its interactive sign-in yet (core/codexauth.Manager satisfies it). +// +// The gate lives HERE, at the submit paths every run enters through, rather than +// only in the adapters' message paths. Runs reach the provider from six places — +// a user message, an edit of one, a poller-injected PR comment, a cron fire, the +// autonomy path (workspace follow-ups, CI events, verify/goal fix-ups) and the +// restart replay of an interrupted run — and only the first two go through an +// adapter. Gating in the adapters alone would let the rest march into a CLI that +// cannot authenticate, once per tick, with nothing user-visible to explain it. +// +// Every one of those six calls blockSubmit; a seventh submit path must call it +// too. TestGateBlocksEveryRunSource enumerates them. +type RunGate interface { + // Blocked reports whether runs must be refused right now, without side effects. + Blocked() bool + // NoticeFor returns the explanation to send to dest after refusing a background + // submission, or "" when dest has already been told through that channel. The + // registry belongs to the gate, not to its callers, and the gate keeps a + // SEPARATE one for replies to a person's own message — so a refused poller + // relay cannot consume the answer a user is owed. + NoticeFor(dest string) string + // NoticeReset forgets every destination once the block lifts, so the next lapse + // is announced afresh. Named rather than a side effect of NoticeFor, so no + // caller has to discard a text it never meant to send. + NoticeReset() +} + +// RunsBlocked reports whether a run submitted right now would be refused. It +// lets a caller that would DESTROY work by submitting — the follow-up sweep takes +// each due item out of its store before firing it — skip the attempt entirely and +// try again later, instead of handing it to a gate that drops it. +func (s *Service) RunsBlocked() bool { + if s.auth == nil { + return false + } + return s.auth.Blocked() +} + +// blockSubmit reports whether a submission must be refused because the provider +// is unauthorized, and tells the chat once why. +// +// It is called at SUBMIT time, before a pending marker would be enqueued. That +// ordering is the point: blocking later, inside the run, would leave behind a +// marker for work that never started — and since the pending store is append-only +// and a blocked run can never reach the clean terminal that clears it, a poller +// or cron firing into an unauthorized deployment would accumulate markers without +// bound and replay every one of them on the next restart. Refusing before the +// marker exists means there is nothing to leak. ResumePending is the deliberate +// exception: its marker came from a previous boot, so it is left untouched and +// replays once the sign-in lands. +// +// The notice is sent at most ONCE per chat per unauthorized period: background +// sources are recurring (a five-minute cron would otherwise post a notice every +// five minutes), and the adapters already answer a user's own message directly. +// The refusal is logged every time, so the operator sees the full picture in the +// logs while the chat stays readable. The counter resets as soon as the provider +// is authorized again, so a later lapse is announced afresh. +func (s *Service) blockSubmit(ctx context.Context, chatID ChatID) bool { + if s.auth == nil { + return false + } + if !s.auth.Blocked() { + // A chat that stayed quiet through the recovery still hears about the NEXT + // lapse. + s.auth.NoticeReset() + return false + } + + s.log.Warn("provider is not authorized — refusing run", "chat_id", chatID) + + if notice := s.auth.NoticeFor(chatID); notice != "" { + // Via notify, which bounds the send: this runs on the CALLER's goroutine, and + // three of the six callers are loops that must not be stalled — the PR poller + // dispatches inline, and the restart replay walks every stored marker. A + // transport that hangs or sits in a 429 back-off would otherwise stop them + // for good. The refusal itself is already logged at Warn above, so notify's + // Debug-level delivery failure is enough. + s.notify(ctx, chatID, notice) + } + return true +} diff --git a/core/chat/rungate_test.go b/core/chat/rungate_test.go new file mode 100644 index 0000000..a34b5f8 --- /dev/null +++ b/core/chat/rungate_test.go @@ -0,0 +1,316 @@ +//nolint:testpackage // whitebox: the gate is asserted through the Service's own run paths. +package chat + +import ( + "context" + "log/slog" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/duckbugio/flock/core/agent" + "github.com/duckbugio/flock/core/dispatch" + "github.com/duckbugio/flock/core/pending" +) + +// countingRunner records how many runs actually reached the provider. +type countingRunner struct{ n atomic.Int64 } + +func (r *countingRunner) Run(_ context.Context, _ string, _ agent.Options) (<-chan agent.Event, error) { + r.n.Add(1) + out := make(chan agent.Event) + go func() { + defer close(out) + out <- agent.Event{Type: agent.Result, Result: &agent.RunResult{Text: "done", Subtype: "success"}} + }() + return out, nil +} + +func (r *countingRunner) calls() int64 { return r.n.Load() } + +// stubGate is a RunGate whose verdict a test flips at will, standing in for +// codexauth.Manager. It counts consultations, which is the precise signal a run +// reached the gate — letting a test assert "nothing ran" without racing. +type stubGate struct { + blocked atomic.Bool + n atomic.Int64 + told stubTold +} + +// stubTold mirrors the real gate's "told once per destination" registry. +type stubTold struct { + mu sync.Mutex + told map[string]bool +} + +func (s *stubTold) Should(dest string) bool { + s.mu.Lock() + defer s.mu.Unlock() + if s.told == nil { + s.told = map[string]bool{} + } + if s.told[dest] { + return false + } + s.told[dest] = true + return true +} + +func (s *stubTold) Clear() { + s.mu.Lock() + defer s.mu.Unlock() + clear(s.told) +} + +func (g *stubGate) Blocked() bool { + g.n.Add(1) + return g.blocked.Load() +} + +func (g *stubGate) NoticeFor(dest string) string { + if !g.blocked.Load() || !g.told.Should(dest) { + return "" + } + return gateNotice +} + +func (g *stubGate) NoticeReset() { + if g.blocked.Load() { + return + } + g.told.Clear() +} + +func (g *stubGate) calls() int64 { return g.n.Load() } + +// gateNotice is the text stubGate blocks with. +const gateNotice = "Codex is not authorized yet. Send /login." + +// notices counts how many times the gate's notice was delivered, ignoring the +// ordinary run output an authorized run also sends. +func notices(c *fakeChat) int { + n := 0 + for _, text := range allSent(c) { + if text == gateNotice { + n++ + } + } + return n +} + +// blockingGate returns a gate that starts out blocking. +func blockingGate() *stubGate { + g := &stubGate{} + g.blocked.Store(true) + return g +} + +// gatedService builds a Service whose runs pass through gate. +func gatedService(t *testing.T, r agent.Runner, c Transport, gate RunGate, p pendingStore) *Service { + t.Helper() + d := dispatch.New(4) + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + _ = d.Shutdown(ctx) + }) + s := New(Config{ + Runner: r, + Transport: c, + Dispatcher: d, + Workspace: &fakeWorkspace{}, + Pending: p, + Auth: gate, + Logger: slog.New(slog.DiscardHandler), + }) + s.tick = 5 * time.Millisecond + return s +} + +// TestGateBlocksEveryRunSource is the point of gating in the Service rather than +// only in the adapters: a user message is just one of four ways a run starts +// here. A cron fire, a poller relay and the restart replay never touch an +// adapter, so an adapter-only gate would march all of them into a CLI that +// cannot authenticate — once per schedule tick, with nothing user-visible. +func TestGateBlocksEveryRunSource(t *testing.T) { + tests := []struct { + name string + start func(s *Service) + }{ + {"user message", func(s *Service) { s.Handle(context.Background(), testChatID, 7, "m1", "build it") }}, + {"poller relay", func(s *Service) { s.Inject(testChatID, "a reviewer commented") }}, + {"cron fire", func(s *Service) { s.InjectScheduled(testChatID, "nightly", 7) }}, + {"restart replay", func(s *Service) { s.ResumePending(testChatID, pending.Marker{ID: "p1", Prompt: "resume me"}) }}, + {"autonomy follow-up", func(s *Service) { s.InjectAuto(context.Background(), testChatID, "ci went red") }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, r, gate := newFakeChat(), &countingRunner{}, blockingGate() + s := gatedService(t, r, c, gate, nil) + + tt.start(s) + waitUntil(t, func() bool { return gate.calls() >= 1 }) + + if got := r.calls(); got != 0 { + t.Errorf("the provider ran %d times while unauthorized, want 0", got) + } + waitUntil(t, func() bool { return notices(c) == 1 }) + }) + } +} + +// TestGateNotifiesOnceUntilAuthorized: background sources recur (a five-minute +// cron would otherwise post a notice every five minutes), so the chat is told +// once — and told again after a later lapse, or a returning outage would go +// unexplained. +func TestGateNotifiesOnceUntilAuthorized(t *testing.T) { + c, gate := newFakeChat(), blockingGate() + s := gatedService(t, &countingRunner{}, c, gate, nil) + + s.Inject(testChatID, "first") + s.Inject(testChatID, "second") + s.Inject(testChatID, "third") + waitUntil(t, func() bool { return gate.calls() >= 3 }) + + if got := notices(c); got != 1 { + t.Fatalf("sent %d notices for three blocked runs, want 1: %v", got, allSent(c)) + } + + // Authorized again — the "already told them" memory must reset. + gate.blocked.Store(false) + s.Inject(testChatID, "now allowed") + waitUntil(t, func() bool { return gate.calls() >= 4 }) + + // ...and a fresh lapse is announced again. + gate.blocked.Store(true) + s.Inject(testChatID, "blocked again") + waitUntil(t, func() bool { return notices(c) == 2 }) +} + +// TestGateKeepsThePendingMarker: a blocked replay must NOT clear the marker of +// the run it refused, or an interrupted run would be lost to the very outage +// that stopped it. The marker survives to be replayed after the sign-in. +func TestGateKeepsThePendingMarker(t *testing.T) { + c, p, gate := newFakeChat(), newFakePending(), blockingGate() + s := gatedService(t, &countingRunner{}, c, gate, p) + + if _, err := p.Enqueue(testChatID, pending.Marker{ID: "p1", Prompt: "resume me"}); err != nil { + t.Fatalf("seed marker: %v", err) + } + s.ResumePending(testChatID, pending.Marker{ID: "p1", Prompt: "resume me"}) + waitUntil(t, func() bool { return gate.calls() >= 1 }) + + if got := p.removes(); len(got) != 0 { + t.Errorf("a blocked run cleared the pending marker (%v); the interrupted work is lost", got) + } + if p.count() != 1 { + t.Errorf("pending markers = %d, want the interrupted run still queued", p.count()) + } +} + +// TestGateAllowsRunsWhenAuthorized keeps the gate honest: an authorized gate — +// or none at all, which is every non-Codex deployment — must not change behavior. +func TestGateAllowsRunsWhenAuthorized(t *testing.T) { + for _, tt := range []struct { + name string + gate RunGate + }{ + {"authorized gate", &stubGate{}}, + {"no gate wired", nil}, + } { + t.Run(tt.name, func(t *testing.T) { + c, r := newFakeChat(), &countingRunner{} + s := gatedService(t, r, c, tt.gate, nil) + + s.Handle(context.Background(), testChatID, 7, "m1", "build it") + waitUntil(t, func() bool { return r.calls() == 1 }) + }) + } +} + +// TestBlockedSubmitLeavesNoPendingMarker is the marker-leak regression. The +// submit paths enqueue a marker BEFORE handing the job to the dispatcher, and a +// blocked run can never reach the clean terminal that clears it — so blocking +// inside the run would strand one marker per refused message. The pending store +// is append-only, and a poller or cron firing into an unauthorized deployment +// would grow it without bound and replay every marker on the next restart. +func TestBlockedSubmitLeavesNoPendingMarker(t *testing.T) { + tests := []struct { + name string + start func(s *Service) + }{ + {"user message", func(s *Service) { s.Handle(context.Background(), testChatID, 7, "m1", "build it") }}, + {"edited message", func(s *Service) { s.HandleEdit(context.Background(), testChatID, 7, "m1", "edited") }}, + {"poller relay", func(s *Service) { s.Inject(testChatID, "a reviewer commented") }}, + {"autonomy follow-up", func(s *Service) { s.InjectAuto(context.Background(), testChatID, "ci went red") }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, p, gate := newFakeChat(), newFakePending(), blockingGate() + s := gatedService(t, &countingRunner{}, c, gate, p) + + for range 5 { + tt.start(s) + } + waitUntil(t, func() bool { return gate.calls() >= 5 }) + + if got := p.count(); got != 0 { + t.Errorf("five blocked submits left %d pending markers, want 0", got) + } + }) + } +} + +// TestBlockedScheduledFireIsReportedDropped: the scheduler reads the bool to know +// the fire did not take, exactly as it does for the cost cap. +func TestBlockedScheduledFireIsReportedDropped(t *testing.T) { + c, gate := newFakeChat(), blockingGate() + s := gatedService(t, &countingRunner{}, c, gate, newFakePending()) + + if s.InjectScheduled(testChatID, "nightly", 7) { + t.Error("InjectScheduled reported the fire enqueued while unauthorized") + } +} + +// TestNotifyResetIsProcessWide: authorization is process-wide, so one chat +// submitting after recovery proves the outage is over for all of them. Clearing +// only the submitting chat would leave a chat that stayed quiet through the +// recovery window silently un-notified when the next lapse hit it. +func TestNotifyResetIsProcessWide(t *testing.T) { + c, gate := newFakeChat(), blockingGate() + s := gatedService(t, &countingRunner{}, c, gate, newFakePending()) + + // The quiet chat is told once, then says nothing more. + s.Inject(testChatID, "first") + waitUntil(t, func() bool { return notices(c) == 1 }) + + // Recovery happens while ANOTHER chat submits; the quiet chat stays silent. + gate.blocked.Store(false) + s.Inject("other-chat", "now allowed") + waitUntil(t, func() bool { return gate.calls() >= 2 }) + + // The next lapse must be announced to the quiet chat again. + gate.blocked.Store(true) + s.Inject(testChatID, "blocked again") + waitUntil(t, func() bool { return notices(c) == 2 }) +} + +// TestBackgroundNoticeDoesNotRepeatItself: the background channel explains once +// per chat, however many refused submissions follow. +func TestBackgroundNoticeDoesNotRepeatItself(t *testing.T) { + c, gate := newFakeChat(), blockingGate() + s := gatedService(t, &countingRunner{}, c, gate, newFakePending()) + + s.Inject(testChatID, "a reviewer commented") + waitUntil(t, func() bool { return notices(c) == 1 }) + s.Inject(testChatID, "another comment") + waitUntil(t, func() bool { return gate.calls() >= 2 }) + + if got := notices(c); got != 1 { + t.Errorf("sent %d notices for two refused background submissions, want 1", got) + } + if notice := gate.NoticeFor(testChatID); notice != "" { + t.Errorf("the background channel would have repeated itself: %q", notice) + } +} diff --git a/core/chat/service.go b/core/chat/service.go index e07780e..c0dc8b6 100644 --- a/core/chat/service.go +++ b/core/chat/service.go @@ -128,6 +128,7 @@ type Service struct { log *slog.Logger tick time.Duration nowFunc func() time.Time + auth RunGate mu sync.Mutex // guards runChat, lastMsg, verifyRetry, budgetNotified and snapCache runChat map[string]ChatID // active runID -> chatID, for mapping Stop back to a chat @@ -179,7 +180,12 @@ type Config struct { // (Telegram supplies its 429 detector here). Nil means the transport has no // rate-limit signal, so a delivery error is never treated as throttling. RetryAfter func(err error) (time.Duration, bool) - Logger *slog.Logger + // Auth blocks every run while the configured provider is unauthorized (today: + // a Codex subscription deploy whose interactive sign-in has not happened yet). + // Nil means no gate — the Claude and API-key paths authenticate from env and + // have nothing to wait for. + Auth RunGate + Logger *slog.Logger } // defaultMaxMessageRunes is the chunk limit used when a Transport reports a @@ -221,6 +227,7 @@ func New(cfg Config) *Service { log: log, tick: tickInterval, nowFunc: time.Now, + auth: cfg.Auth, runChat: map[string]ChatID{}, lastMsg: map[ChatID]MessageID{}, @@ -253,8 +260,11 @@ func (s *Service) Handle(_ context.Context, chatID ChatID, userID int64, msgID M // behaves exactly like Handle (trailing-arg text path). Edit-tracking and // dispatch semantics are identical to Handle. func (s *Service) HandleMedia( - _ context.Context, chatID ChatID, userID int64, msgID MessageID, prompt string, images []agent.ImageInput, + ctx context.Context, chatID ChatID, userID int64, msgID MessageID, prompt string, images []agent.ImageInput, ) { + if s.blockSubmit(ctx, chatID) { + return + } s.mu.Lock() s.lastMsg[chatID] = msgID // A real user message opens a fresh chapter: the post-run verification retry @@ -280,6 +290,9 @@ func (s *Service) HandleMedia( // allow-listed user holds); these synthetic relays are rare and are not rate-/ // cost-gated on the inbound path. func (s *Service) Inject(chatID ChatID, prompt string) { + if s.blockSubmit(context.Background(), chatID) { + return + } // Enqueue before Submit so a poller-injected run is captured for auto-resume // too (it should resume if killed mid-flight, same as a user message). id := s.enqueuePending(chatID, prompt) @@ -305,6 +318,9 @@ func (s *Service) Inject(chatID ChatID, prompt string) { // and true when the run was enqueued. Inject, by contrast, runs as the sentinel // user 0, is durable (enqueues a marker), and blocks on a full buffer. func (s *Service) InjectScheduled(chatID ChatID, prompt string, userID int64) bool { + if s.blockSubmit(context.Background(), chatID) { + return false + } if s.costs != nil && !s.costs.Allowed(userID, s.costCapUSD) { return false } @@ -321,6 +337,12 @@ func (s *Service) InjectScheduled(chatID ChatID, prompt string, userID int64) bo // It deliberately does not touch edit-tracking state (like Inject), so replay // never interferes with a real user's supersede logic. func (s *Service) ResumePending(chatID ChatID, m pending.Marker) { + // The marker is deliberately left in place: it was not created here, and an + // interrupted run must not be lost to the outage that is blocking it. It + // replays after the sign-in, on the next restart. + if s.blockSubmit(context.Background(), chatID) { + return + } s.dispatch.Submit(chatID, func(ctx context.Context) { s.run(ctx, chatID, 0, m.Prompt, nil, m.ID) }) @@ -340,8 +362,11 @@ func (s *Service) HandleEdit(ctx context.Context, chatID ChatID, userID int64, m // HandleEditMedia is HandleEdit with optional image attachments (see // HandleMedia). An empty images slice behaves exactly like HandleEdit. func (s *Service) HandleEditMedia( - _ context.Context, chatID ChatID, userID int64, msgID MessageID, prompt string, images []agent.ImageInput, + ctx context.Context, chatID ChatID, userID int64, msgID MessageID, prompt string, images []agent.ImageInput, ) { + if s.blockSubmit(ctx, chatID) { + return + } s.mu.Lock() last, ok := s.lastMsg[chatID] supersede := ok && last == msgID diff --git a/core/codexauth/codexauth.go b/core/codexauth/codexauth.go new file mode 100644 index 0000000..fefdc07 --- /dev/null +++ b/core/codexauth/codexauth.go @@ -0,0 +1,783 @@ +package codexauth + +import ( + "context" + "errors" + "fmt" + "log/slog" + "os" + "path/filepath" + "strings" + "sync" + "time" +) + +// Codex auth modes, mirroring internal/config's CODEX_AUTH_MODE values. Only the +// subscription mode has an interactive login: billing authenticates with an API +// key from the environment and has nothing for a user to confirm in a browser. +const ( + AuthSubscription = "subscription" + AuthBilling = "billing" +) + +// BackendCodex is the AI_BACKEND value this package applies to. Any other +// backend (Claude, an OpenAI-compatible endpoint) authenticates from env alone, +// so /login is a no-op there. +const BackendCodex = "codex" + +// Config describes the deployment's Codex auth setup. It is built once at +// startup (see airunner.CodexAuthConfig) and read for every /login. +type Config struct { + // Backend is the resolved AI_BACKEND name. + Backend string + // AuthMode is the resolved CODEX_AUTH_MODE ("subscription" or "billing"). + AuthMode string + // Bin is the Codex CLI path (CODEX_BIN); empty means "codex" on PATH. + Bin string + // Home is CODEX_HOME — the directory holding the persisted auth.json that a + // successful login writes and every later run reads. + Home string + // Env is the child environment for the Codex CLI, already stripped of + // CODEX_API_KEY in subscription mode. Empty inherits the parent's. + Env []string + // HasAccessToken records that CODEX_ACCESS_TOKEN is configured, which + // authorizes Codex without any interactive login. + HasAccessToken bool + // RequireAuth mirrors CODEX_REQUIRE_AUTH. When false the operator has + // explicitly declared that Codex is authorized by means this process cannot + // see (a home mounted later, credentials in config.toml), so runs are never + // blocked — /login stays available, it just is not demanded. + RequireAuth bool + // Timeout bounds one login attempt. Zero means DeviceCodeTTL plus a minute of + // slack, so an abandoned login cannot outlive the code it waits on. + Timeout time.Duration + Logger *slog.Logger +} + +// bin returns the Codex CLI to execute. +func (c Config) bin() string { + if b := strings.TrimSpace(c.Bin); b != "" { + return b + } + return "codex" +} + +// childEnv returns the environment for the login child. +// +// CODEX_HOME is forced to the configured value so the login writes auth.json +// exactly where the runner later looks for it — a login into a different home is +// the subtlest way for "logged in" and "still unauthorized" to be true at once. +// +// CODEX_API_KEY is dropped in subscription mode. Env normally arrives already +// stripped (airunner.CodexEnv), but this is the one place a stray inherited key +// could turn a subscription sign-in into API-billing auth, so the invariant is +// enforced here too rather than assumed. +func (c Config) childEnv() []string { + env := c.Env + if len(env) == 0 { + env = os.Environ() + } + dropAPIKey := strings.EqualFold(strings.TrimSpace(c.AuthMode), AuthSubscription) + home := strings.TrimSpace(c.Home) + + out := make([]string, 0, len(env)+1) + for _, kv := range env { + if home != "" && strings.HasPrefix(kv, "CODEX_HOME=") { + continue + } + if dropAPIKey && strings.HasPrefix(kv, "CODEX_API_KEY=") { + continue + } + out = append(out, kv) + } + if home == "" { + return out + } + return append(out, "CODEX_HOME="+home) +} + +// authFile is the persisted login file a successful device login writes. +func (c Config) authFile() string { + home := strings.TrimSpace(c.Home) + if home == "" { + return "" + } + return filepath.Join(home, "auth.json") +} + +// timeout bounds a single login attempt. +func (c Config) timeout() time.Duration { + if c.Timeout > 0 { + return c.Timeout + } + return DeviceCodeTTL + time.Minute +} + +// logger never returns nil. +func (c Config) logger() *slog.Logger { + if c.Logger != nil { + return c.Logger + } + return slog.Default() +} + +// Manager owns the at-most-one in-flight device login for the process and +// answers the /login command. Codex auth is process-wide state (a single +// auth.json under CODEX_HOME), so the login is deliberately NOT per chat: a +// second /login while one is pending re-shows the SAME code rather than starting +// a competing flow that would invalidate it. +type Manager struct { + cfg Config + + // Two "already told them" registries for the refusal notice, one per channel it + // travels: notified for a refused BACKGROUND submission (a poller relay, a + // restart replay, a CI event) and notifiedUser for the reply to a person's own + // message. They are independent because they answer different questions — "has + // this chat been told the deployment is stuck" and "has this person been + // answered" — and sharing one cell means whichever fires first silences the + // other. A background refusal must never consume the answer a user is owed: + // this whole feature exists so a human learns that /login is needed. + // + // Both live here, with the state they describe, rather than one per caller — + // per-caller registries kept each caller's repeats down and still told one chat + // the same sentence twice. Each has its own lock so a notice never contends + // with a login. + notified onceNotifier + notifiedUser onceNotifier + + mu sync.Mutex + // cur is the attempt in flight, nil when idle. Holding the whole attempt behind + // ONE pointer is what keeps the state machine honest: "is a login pending", + // "whose code is this", and "who is waiting to hear the outcome" are answers + // about one attempt, and swapping the pointer under the lock moves all of them + // at once. Kept as separate fields they could — and did — disagree: a Cancel + // that timed out cleared the cancel func but left the attempt marked running, + // so the next /login re-showed a code that had just been killed. + cur *session +} + +// session is one device-login attempt. The run goroutine owns its lifetime; every +// field is guarded by Manager.mu. +type session struct { + // cancelled records that this attempt was ended deliberately, so its verdict is + // not also announced: Cancel already told the chat, and "The Codex sign-in was + // cancelled" arriving after "Cancelled the pending Codex login" is the same news + // twice. + cancelled bool + cancel context.CancelFunc + done chan struct{} // closed when this attempt's goroutine has returned + // prompt is the verification link and one-time code, once issued. + prompt Prompt + hasPrompt bool + // subs are the destinations waiting on THIS attempt's outcome. + subs []Subscriber +} + +// NewManager returns a Manager for cfg. +func NewManager(cfg Config) *Manager { + return &Manager{ + cfg: cfg, + notified: onceNotifier{told: map[string]bool{}}, + notifiedUser: onceNotifier{told: map[string]bool{}}, + } +} + +// onceNotifier tells each destination something at most once, until Clear. +type onceNotifier struct { + mu sync.Mutex + told map[string]bool +} + +// Should reports whether dest still needs to be told, marking it as told. +func (n *onceNotifier) Should(dest string) bool { + n.mu.Lock() + defer n.mu.Unlock() + if n.told == nil { + n.told = map[string]bool{} + } + if n.told[dest] { + return false + } + n.told[dest] = true + return true +} + +// Clear forgets every destination, so the next occurrence is announced again. +func (n *onceNotifier) Clear() { + n.mu.Lock() + defer n.mu.Unlock() + clear(n.told) +} + +// Applicable reports whether an interactive login means anything on this +// deployment: only the Codex backend in subscription mode has one. +func (m *Manager) Applicable() bool { + if m == nil { + return false + } + return strings.EqualFold(strings.TrimSpace(m.cfg.Backend), BackendCodex) && + strings.EqualFold(strings.TrimSpace(m.cfg.AuthMode), AuthSubscription) +} + +// LoginAdvertised reports whether /login should be OFFERED — in a command menu +// or a help text. It is narrower than Applicable: a deployment carrying +// CODEX_ACCESS_TOKEN is already authorized without a browser, so listing the +// command would lead the user to a reply that says it is not needed, which is the +// exact reason the other backends are excluded. +// +// Applicable stays wider on purpose: /login force is still a legitimate way to +// replace a workspace token with a personal subscription, so the command keeps +// working where it is simply not advertised. +func (m *Manager) LoginAdvertised() bool { + if m == nil { + return false + } + return m.Applicable() && !m.cfg.HasAccessToken +} + +// Authorized reports whether Codex can run right now: a configured access token, +// or a persisted auth.json under CODEX_HOME. It re-checks the file on every call +// rather than caching a startup verdict, so the very first run after a +// successful /login is allowed through without a restart. +// +// CODEX_REQUIRE_AUTH=false opts out of the check entirely, exactly as it does at +// startup: an operator who disabled it has taken responsibility for credentials +// this process cannot observe, and must not have their runs blocked. +func (m *Manager) Authorized() bool { + if !m.Applicable() || !m.cfg.RequireAuth { + return true + } + return m.CredentialsPresent() +} + +// CredentialsPresent reports whether Codex has something to authenticate WITH: a +// configured access token, or a persisted auth.json under CODEX_HOME. +// +// This is the FACTUAL answer; Authorized is the POLICY answer runs are gated on, +// and the two deliberately differ when CODEX_REQUIRE_AUTH=false — that deploy has +// no credential and is not blocked. Startup logs both, because "authorized=true" +// alone would reassure an operator in exactly the configuration where the first +// run is about to fail inside the CLI. +// +// A present auth.json is not proof the credential still WORKS: an expired or +// corrupt token reads as present. Re-validating would mean spawning +// `codex login status` on every message, which is far too expensive for the +// message path; /login force re-authenticates when a stale credential shows up. +func (m *Manager) CredentialsPresent() bool { + if m == nil { + return false + } + if m.cfg.HasAccessToken { + return true + } + f := m.cfg.authFile() + if f == "" { + return false + } + _, err := os.Stat(f) + return err == nil +} + +// NeedsLogin is the inverse of Authorized, for callers that read better that way. +func (m *Manager) NeedsLogin() bool { return !m.Authorized() } + +// Blocked reports whether runs must be refused right now. It has no side effects, +// so a caller that only needs to know — the follow-up sweeper and the cron +// scheduler, which skip their whole pass rather than consume work — can ask +// freely. +func (m *Manager) Blocked() bool { + if m == nil { + return false + } + return !m.Authorized() +} + +// NoticeFor returns the explanation to send to dest after refusing a BACKGROUND +// submission, or "" when dest has already been told through that channel. +func (m *Manager) NoticeFor(dest string) string { + if m == nil || !m.Blocked() || !m.notified.Should(dest) { + return "" + } + return blockedNoticeText +} + +// ReplyNoticeFor returns the explanation to send to dest in reply to a PERSON's +// own message, or "" when that person has already been answered. +// +// Separate from NoticeFor on purpose: a refused poller relay or restart replay +// must not consume the answer a user is owed. Sharing one cell meant a container +// that came up with a lost auth.json replayed its markers, spent the chat's only +// notice on that, and then met the user with silence — in the one feature whose +// entire job is telling a human that /login is needed. +func (m *Manager) ReplyNoticeFor(dest string) string { + if m == nil || !m.Blocked() || !m.notifiedUser.Should(dest) { + return "" + } + return blockedNoticeText +} + +// NoticeReset forgets every destination once authorization is back, so the NEXT +// lapse is announced afresh — including to a chat that stayed quiet through the +// recovery. It is a named operation rather than a side effect of asking for a +// notice, so no caller has to discard a text it never meant to send. +func (m *Manager) NoticeReset() { + if m == nil || m.Blocked() { + return + } + m.notified.Clear() + m.notifiedUser.Clear() +} + +// blockedNoticeText says what is wrong and where the fix is accepted. "In a +// direct message", because that is the only place /login can start: the callers +// cannot see the destination, and sending a user to /login in a group would only +// earn them a second refusal — possibly in the only chat they use with the bot. +const blockedNoticeText = "Codex is not authorized yet, so I can't run anything.\n\n" + + "Send /login in a direct message with me and I'll walk you through the one-time browser sign-in." + +// NoLoginNeededText is the reply when this deployment has no interactive login +// at all — including when no Manager was wired up. +const NoLoginNeededText = "No interactive login is needed on this deployment: " + + "the AI provider authenticates from its configured credentials." + +// StatusText describes the current auth state for a PRIVATE destination. It +// never reveals a stored secret, but it does re-show a pending sign-in's +// one-time code, which is why the destination matters — see statusText. +func (m *Manager) StatusText() string { return m.statusText(true) } + +// statusText describes the current auth state. When the destination is not +// private the pending code is withheld: /login status is otherwise a way to +// print it into a group, straight past the guard on starting a sign-in there. +func (m *Manager) statusText(private bool) string { + if !m.Applicable() { + return m.notApplicableText() + } + m.mu.Lock() + cur := m.cur + var ( + prompt Prompt + hasPrompt bool + ) + if cur != nil { + prompt, hasPrompt = cur.prompt, cur.hasPrompt + } + m.mu.Unlock() + + switch { + case cur != nil && hasPrompt && !private: + return pendingElsewhereText + case cur != nil && hasPrompt: + return "Codex device login is in progress — finish it in the browser:\n\n" + promptText(prompt) + case cur != nil && !private: + // Not "the code arrives in a moment": a non-private destination never + // subscribes, so nothing would ever arrive there. + return pendingElsewhereText + case cur != nil: + return "Codex device login is starting; the link and code arrive in a moment." + case m.cfg.HasAccessToken: + return "Codex is authorized with a configured access token. No browser login needed." + case m.CredentialsPresent() && private: + return "Codex is authorized (persisted login in " + m.cfg.Home + ")." + case m.CredentialsPresent(): + // The host path is deployment detail; a group chat has no business with it. + return "Codex is authorized." + case !m.cfg.RequireAuth: + return "No persisted Codex login found, but CODEX_REQUIRE_AUTH=false, so runs are not blocked. " + + "Send /login if Codex reports it is unauthenticated." + default: + return "Codex is NOT authorized. Send /login to sign in with your ChatGPT account." + } +} + +// notApplicableText explains why /login does nothing on this deployment. It is +// nil-safe: an adapter wired without a Manager still gets a sensible reply +// instead of a panic. +func (m *Manager) notApplicableText() string { + if m == nil { + return NoLoginNeededText + } + backend := strings.TrimSpace(m.cfg.Backend) + if backend == "" { + backend = "the configured provider" + } + if strings.EqualFold(backend, BackendCodex) { + return "Codex runs in API-billing mode here, which authenticates from CODEX_API_KEY. " + + "There is no browser login to complete." + } + return "No interactive login is needed: this deployment runs " + backend + + ", which authenticates from its configured credentials." +} + +// LoginPrivateOnlyText refuses to START a sign-in outside a 1:1 chat. +const LoginPrivateOnlyText = "Send /login in a direct message with me, not here.\n\n" + + "The reply carries a one-time code that authorizes an account for the whole bot, " + + "and everyone in this chat would see it." + +// cancelPrivateOnlyText refuses to abort a sign-in from outside a 1:1 chat. It is +// separate from LoginPrivateOnlyText because the reason differs: a cancel reply +// carries no code, so explaining the refusal in terms of one would be simply +// untrue. What is at stake is control — the sign-in belongs to the direct message +// running it, and its confirmation would reveal that one is under way. +const cancelPrivateOnlyText = "Send /login cancel in a direct message with me, not here.\n\n" + + "A sign-in is controlled from the direct message it runs in." + +// pendingElsewhereText reports a pending sign-in without reprinting its code. It +// is deliberately neutral about WHOSE direct message: the code is re-shown to any +// private destination that asks, not only the one that started the login, and +// telling the user otherwise would misdescribe the very thing this message is +// about. (That any allow-listed user can finish a started sign-in with their own +// account is a documented property — one Codex identity serves the deployment.) +const pendingElsewhereText = "A Codex sign-in is in progress. Its link and one-time code go only to direct " + + "messages — send /login in a direct message with me to see them again." + +// LoginUsage is the /login help. +const LoginUsage = "Usage:\n" + + "/login — start the Codex browser sign-in (or re-show the pending code)\n" + + "/login status — show the current authorization state\n" + + "/login cancel — abort a pending sign-in\n" + + "/login force — sign in again even if already authorized" + +// NotifyTimeout bounds ONE background notice delivery. Both adapters use it for +// their Subscriber.Notify: a pending login reports minutes after the update that +// started it is gone, so each notice needs a fresh, bounded context of its own. +const NotifyTimeout = 30 * time.Second + +// Subscriber is where a /login reply and a pending login's background notices +// are delivered. +// +// Private is the security-critical field: the one-time code authorizes an +// account for the WHOLE bot, so whoever acts on it first binds their ChatGPT +// account to it and every later run (and its history) goes through that account. +// A code may therefore only ever reach a 1:1 destination. The judgement lives +// HERE rather than in each adapter because it cannot be derived from the command +// arguments: /login status also prints the pending code, so an argument-based +// guard silently misses it. Manager redacts instead, on every path that could +// carry a code. +// +// ID identifies the destination so the same one asking twice is collapsed to a +// single subscription rather than being told everything twice. Notify does the +// delivery and may be called minutes later, from another goroutine, so it must +// build its own context rather than close over the update's. +type Subscriber struct { + ID string + Private bool + Notify func(string) +} + +// deliver sends text when the subscriber can receive it. +func (s Subscriber) deliver(text string) { + if s.Notify != nil { + s.Notify(text) + } +} + +// normalizeArgs canonicalizes a /login argument for comparison. +func normalizeArgs(args string) string { return strings.ToLower(strings.TrimSpace(args)) } + +// Dispatch serves the /login command and returns the immediate reply for the +// caller to send — or "" when it has already been delivered through sub, which is +// how the reply is kept ahead of the notices that follow it. +// +// The device flow outlives the command: after the immediate reply, the pending +// login keeps running in the background and reports the verification link, the +// one-time code, and the final outcome to every subscriber. +// +// Subscribing is per Dispatch call, so a SECOND /login (from another chat, or the +// same one) both re-shows the pending code and starts receiving the outcome — +// without it, only the chat that happened to start the login would ever learn +// whether it succeeded. +// +// base only seeds the login's values; its cancellation does NOT abort the login +// (the update's context is dead long before a user finishes in a browser). +// /login cancel, the timeout, and process shutdown are the ways it ends. +func (m *Manager) Dispatch(base context.Context, args string, sub Subscriber) string { + if !m.Applicable() { + return m.notApplicableText() + } + switch normalizeArgs(args) { + case "": + return m.start(base, false, sub) + case "force", "again", "relogin": + return m.start(base, true, sub) + case "status": + return m.statusText(sub.Private) + case "cancel", "abort", "stop": + // The only state-MUTATING branch, so it obeys the same rule as start: a + // conversation could otherwise abort a sign-in running in someone else's + // direct message, and the confirmation would itself reveal that one is + // underway. Control of the login stays on the 1:1 channel that owns it. + if !sub.Private { + return cancelPrivateOnlyText + } + if m.Cancel() { + return "Cancelled the pending Codex login." + } + return "No Codex login is pending." + case "help": + return LoginUsage + default: + return "Unknown /login argument.\n\n" + LoginUsage + } +} + +// start launches a device login unless one is already pending (in which case the +// pending code is re-shown, never replaced) or Codex is already authorized and +// force was not asked for. +func (m *Manager) start(base context.Context, force bool, sub Subscriber) string { + // Refused before anything else: every reply below can carry the one-time code, + // and subscribing a group destination would also feed it the code when the + // prompt is broadcast. + if !sub.Private { + return LoginPrivateOnlyText + } + m.mu.Lock() + if cur := m.cur; cur != nil { + subscribeLocked(cur, sub) + prompt, hasPrompt := cur.prompt, cur.hasPrompt + m.mu.Unlock() + // Through ack for the same reason the fresh-start branch below uses it: this + // caller is already subscribed (a line above), so a reply the ADAPTER sends + // can be overtaken by a broadcast — leaving "the code arrives in a moment" + // sitting under a code that already arrived. + if hasPrompt { + return m.ack(sub, "A Codex login is already pending — finish this one:\n\n"+promptText(prompt)) + } + return m.ack(sub, "A Codex login is already starting; the link and code arrive in a moment.") + } + // CredentialsPresent, not Authorized: with CODEX_REQUIRE_AUTH=false the policy + // answer is always "authorized", which would make /login unusable on exactly + // the deployments most likely to need it. + if !force && m.CredentialsPresent() { + m.mu.Unlock() + return m.StatusText() + "\n\nSend /login force to sign in again." + } + + // context.WithoutCancel: the login must survive the update whose handler + // started it. The timeout is the real bound. + //nolint:gosec // G118: cancel is owned by the session — run defers it, Cancel calls it. + ctx, cancel := context.WithTimeout(context.WithoutCancel(base), m.cfg.timeout()) + cur := &session{cancel: cancel, done: make(chan struct{})} + subscribeLocked(cur, sub) + m.cur = cur + m.mu.Unlock() + + // Delivered through the SAME channel as every later notice, and BEFORE the run + // goroutine exists. Returning it for the adapter to send separately raced the + // broadcast: a CLI that printed its banner quickly could get the link and code + // out first, so the user read "the code arrives in a moment" underneath the + // code that had already arrived. + reply := m.ack(sub, "Starting the Codex sign-in — the link and one-time code arrive in a moment.") + go m.run(ctx, cur) + return reply +} + +// ack delivers an immediate reply through sub, so it cannot be overtaken by a +// notice the caller has not sent yet. It returns the text unsent only when the +// subscriber cannot receive it, leaving the caller to deliver it; an empty return +// means "already delivered, send nothing". +func (m *Manager) ack(sub Subscriber, text string) string { + if sub.Notify == nil { + return text + } + sub.deliver(text) + return "" +} + +// subscribeLocked adds sub to this attempt's notice list, collapsing a +// destination that is already subscribed. The caller must hold Manager.mu. Only +// private destinations subscribe: the notices carry the one-time code. +func subscribeLocked(s *session, sub Subscriber) { + if sub.Notify == nil || !sub.Private { + return + } + for _, existing := range s.subs { + if existing.ID == sub.ID { + return + } + } + s.subs = append(s.subs, sub) +} + +// subscribers snapshots this attempt's destinations, so a delivery (which does +// network I/O) never holds the lock. +func (m *Manager) subscribers(s *session) []Subscriber { + m.mu.Lock() + defer m.mu.Unlock() + return append([]Subscriber(nil), s.subs...) +} + +// deliverAll sends text to every destination in subs. +func deliverAll(subs []Subscriber, text string) { + for _, sub := range subs { + sub.deliver(text) + } +} + +// run drives one login attempt and reports its result to the destinations waiting +// on THAT attempt. Closing s.done is what lets Cancel wait for the goroutine to +// unwind. +func (m *Manager) run(ctx context.Context, s *session) { + defer close(s.done) + defer s.cancel() + log := m.cfg.logger() + + // Guard against the very symptom this package exists to remove. If the parser + // never pairs a URL with a code — a changed banner, a code printed with an + // unexpected prefix — the user would otherwise read "the link and one-time code + // arrive in a moment" and then hear nothing for DeviceCodeTTL, which is exactly + // "it just hangs" wearing a friendlier hat. Say so instead, and name the way out. + warn := time.AfterFunc(promptWarnAfter, func() { + m.mu.Lock() + silent := m.cur == s && !s.hasPrompt + subs := append([]Subscriber(nil), s.subs...) + m.mu.Unlock() + if !silent { + return + } + log.Warn("codex printed no device-login code yet", "after", promptWarnAfter) + deliverAll(subs, noPromptYetText) + }) + defer warn.Stop() + + err := Login(ctx, m.cfg, func(p Prompt) { + m.mu.Lock() + s.prompt, s.hasPrompt = p, true + m.mu.Unlock() + // No URL: it pairs with the code, which is why redact strips it from captured + // output. Codex may also start emitting verification_uri_complete, which + // embeds the code in the link itself. + log.Info("codex device login prompt issued", "expires_in", p.ExpiresIn) + deliverAll(m.subscribers(s), promptText(p)) + }) + + // Detach and snapshot in ONE critical section, and only if this attempt is + // still the current one. A cancelled attempt has already been detached, and a + // successor may be running by now: without the identity check this would clear + // the successor's state, and without the snapshot the verdict below would be + // delivered to whoever is subscribed at that instant — the successor's chat + // reading a failure as its own, the original never hearing its outcome. + m.mu.Lock() + if m.cur == s { + m.cur = nil + } + cancelled := s.cancelled + subs := append([]Subscriber(nil), s.subs...) + m.mu.Unlock() + + if cancelled { + // Cancel already answered the user; the verdict would only repeat it. + log.Info("codex device login cancelled") + return + } + deliverAll(subs, m.outcome(err, log)) +} + +// outcome turns a finished attempt into the text its subscribers are told, and +// logs it. +func (m *Manager) outcome(err error, log *slog.Logger) string { + if err != nil { + log.Warn("codex device login failed", "error", err) + return failureText(err) + } + // A clean exit is necessary but not sufficient: what actually authorizes the + // deployment is the credential on disk, and that is what the run gate reads. + // Announcing success on the exit code alone would tell the user they are + // signed in and then refuse their very next message. + if !m.CredentialsPresent() { + log.Error("codex login exited cleanly but persisted no credential", "codex_home", m.cfg.Home) + return "The Codex CLI finished the sign-in but saved no credential in " + m.cfg.Home + ".\n\n" + + "Check that the directory is writable, then send /login to try again." + } + log.Info("codex device login succeeded", "codex_home", m.cfg.Home) + return "Codex is authorized. Send your next message and the team gets to work." +} + +// promptWarnAfter is how long the CLI may stay silent before the user is told +// that no code has appeared. A variable so tests need not wait it out. +var promptWarnAfter = 45 * time.Second + +// noPromptYetText reports a sign-in that has produced no code yet. +const noPromptYetText = "The Codex CLI still hasn't printed a sign-in code.\n\n" + + "It may just be slow to start. If nothing arrives shortly, /login cancel stops it and /login tries again." + +// cancelDrain bounds how long Cancel waits for the login goroutine to unwind. +const cancelDrain = 5 * time.Second + +// Cancel aborts a pending login, reporting whether there was one. +// +// The attempt is detached IMMEDIATELY, before any waiting: from the caller's next +// instruction the Manager is idle, so a /login that follows starts fresh instead +// of being answered "already pending" with the code just killed. The wait that +// follows is only for the goroutine to finish unwinding, and a timeout is +// therefore harmless — there is no state left to be inconsistent. That matters +// because the wait is genuinely reachable: the goroutine may be inside a notice +// delivery, which is bounded by NotifyTimeout, longer than cancelDrain. +// +// Killing the child does NOT depend on that wait either: cancelling the context +// makes Login signal the whole process group at once (SIGTERM, SIGKILL after +// killGrace). So `defer auth.Cancel()` on shutdown still guarantees no orphaned +// polling process, even when the drain times out. +func (m *Manager) Cancel() bool { + if m == nil { + return false + } + m.mu.Lock() + s := m.cur + if s == nil { + m.mu.Unlock() + return false + } + s.cancelled = true + m.cur = nil + m.mu.Unlock() + + s.cancel() + select { + case <-s.done: + case <-time.After(cancelDrain): + } + return true +} + +// promptText renders the user-facing sign-in instructions. The code is the whole +// point of the message, so it goes on its own line, unadorned, ready to copy. +func promptText(p Prompt) string { + var b strings.Builder + _, _ = b.WriteString("Codex sign-in — two steps:\n\n1. Open this link and sign in to your ChatGPT account:\n") + _, _ = b.WriteString(p.URL) + _, _ = b.WriteString("\n\n2. Enter this one-time code there:\n") + _, _ = b.WriteString(p.Code) + if p.ExpiresIn > 0 { + _, _ = fmt.Fprintf(&b, "\n\nThe code expires in %s.", humanDuration(p.ExpiresIn)) + } + _, _ = b.WriteString("\n\nI'll confirm here as soon as it goes through. /login cancel aborts it.") + return b.String() +} + +// failureText turns a login error into an actionable reply, keeping the CLI's +// own words for the cases we cannot classify. +func failureText(err error) string { + switch { + case errors.Is(err, context.DeadlineExceeded): + return "The Codex sign-in timed out — the one-time code expired before it was confirmed. " + + "Send /login to get a fresh code." + case errors.Is(err, context.Canceled): + return "The Codex sign-in was cancelled." + case errors.Is(err, ErrDeviceAuthUnsupported): + return "Codex sign-in is unavailable: " + err.Error() + default: + return "Codex sign-in failed: " + err.Error() + "\n\nSend /login to try again." + } +} + +// humanDuration renders a code lifetime the way a person would say it. +func humanDuration(d time.Duration) string { + switch { + case d >= time.Hour: + return fmt.Sprintf("%d h", int(d.Hours())) + case d >= time.Minute: + return fmt.Sprintf("%d min", int(d.Minutes())) + default: + return fmt.Sprintf("%d s", int(d.Seconds())) + } +} diff --git a/core/codexauth/codexauth_test.go b/core/codexauth/codexauth_test.go new file mode 100644 index 0000000..5989d3f --- /dev/null +++ b/core/codexauth/codexauth_test.go @@ -0,0 +1,1032 @@ +//nolint:testpackage // intentionally whitebox to test the device-login parser and manager internals +package codexauth + +import ( + "context" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" +) + +// writeAuthFile plants the auth.json a completed Codex login leaves behind. +func writeAuthFile(t *testing.T, home string) { + t.Helper() + if err := os.WriteFile(filepath.Join(home, "auth.json"), []byte(`{"tokens":{}}`), 0o600); err != nil { + t.Fatalf("write auth.json: %v", err) + } +} + +// collector accumulates the background notices a login delivers. +type collector struct { + ch chan string +} + +func newCollector() *collector { return &collector{ch: make(chan string, 8)} } + +// persistAuth is the shell step a fake CLI uses to stand in for the browser +// half of the flow: a real successful login writes auth.json into CODEX_HOME, +// and that file — not the exit code — is what authorizes later runs. +const persistAuth = "sleep 0.3\necho '{}' > \"$CODEX_HOME/auth.json\"\n" + +// dmSub is a private (1:1) destination — the only kind allowed to see a code. +var dmSub = Subscriber{ID: "dm", Private: true} + +func (c *collector) notify(text string) { c.ch <- text } + +// sub is the collector as a Subscriber for destination id. +func (c *collector) sub(id string) Subscriber { + return Subscriber{ID: id, Private: true, Notify: c.notify} +} + +// awaitCode drains notices until the one carrying the device code, returning it. +// The immediate acknowledgement now arrives through the SAME subscriber (that +// ordering is the point), so a test that only wants the prompt must skip past it. +func (c *collector) awaitCode(t *testing.T) string { + t.Helper() + deadline := time.After(10 * time.Second) + for { + select { + case text := <-c.ch: + if strings.Contains(text, deviceCode) { + return text + } + case <-deadline: + t.Fatal("no device code within 10s") + return "" + } + } +} + +// next returns the next notice, failing the test if none arrives in time. +func (c *collector) next(t *testing.T) string { + t.Helper() + select { + case s := <-c.ch: + return s + case <-time.After(10 * time.Second): + t.Fatal("no login notice within 10s") + return "" + } +} + +// TestApplicableOnlyForCodexSubscription: every other setup authenticates from +// env alone, so /login must say so instead of spawning a CLI. +func TestApplicableOnlyForCodexSubscription(t *testing.T) { + tests := []struct { + name string + backend string + authMode string + want bool + }{ + {"codex subscription", BackendCodex, AuthSubscription, true}, + {"codex subscription mixed case", "Codex", "Subscription", true}, + {"codex billing", BackendCodex, AuthBilling, false}, + {"claude", "claude", AuthSubscription, false}, + {"openai-compatible", "openai-compat", "", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := NewManager(Config{Backend: tt.backend, AuthMode: tt.authMode}) + if got := m.Applicable(); got != tt.want { + t.Errorf("Applicable() = %v, want %v", got, tt.want) + } + }) + } +} + +// TestAuthorizedSources: a persisted auth.json or a configured access token each +// authorize Codex; neither present means the deployment needs a login. +func TestAuthorizedSources(t *testing.T) { + t.Run("no credentials", func(t *testing.T) { + m := NewManager(Config{Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: t.TempDir()}) + if m.Authorized() { + t.Error("Authorized() = true with no auth.json and no access token") + } + if !m.NeedsLogin() { + t.Error("NeedsLogin() = false, want true") + } + }) + t.Run("persisted auth file", func(t *testing.T) { + home := t.TempDir() + writeAuthFile(t, home) + m := NewManager(Config{Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: home}) + if !m.Authorized() { + t.Error("Authorized() = false with a persisted auth.json") + } + }) + t.Run("access token", func(t *testing.T) { + m := NewManager(Config{ + Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: t.TempDir(), HasAccessToken: true, + }) + if !m.Authorized() { + t.Error("Authorized() = false with CODEX_ACCESS_TOKEN configured") + } + }) + t.Run("non-codex backend never needs a login", func(t *testing.T) { + m := NewManager(Config{Backend: "claude"}) + if !m.Authorized() { + t.Error("Authorized() = false for a non-Codex backend") + } + }) +} + +// TestAuthorizedRechecksTheAuthFile is what lets the very first message after a +// successful /login run WITHOUT a restart: the verdict is re-read from disk, not +// cached from startup. +func TestAuthorizedRechecksTheAuthFile(t *testing.T) { + home := t.TempDir() + m := NewManager(Config{Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: home}) + if m.Authorized() { + t.Fatal("Authorized() = true before any login") + } + writeAuthFile(t, home) + if !m.Authorized() { + t.Error("Authorized() = false after auth.json appeared; the verdict was cached") + } +} + +// TestBlockedAndNoticeFor: an unauthorized deployment refuses runs and explains +// itself ONCE per destination — the registry lives with the auth state, because +// the same refusal reaches a chat both from an adapter answering a user and from +// core/chat refusing a background submission. A registry per caller keeps each +// caller's repeats down and still tells one chat twice. +func TestBlockedAndNoticeFor(t *testing.T) { + home := t.TempDir() + m := NewManager(Config{Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: home}) + + if !m.Blocked() { + t.Fatal("Blocked() = false while unauthorized") + } + if notice := m.NoticeFor("chat-1"); !strings.Contains(notice, "/login") { + t.Errorf("notice %q should tell the user to send /login", notice) + } + if repeat := m.NoticeFor("chat-1"); repeat != "" { + t.Errorf("the same chat was told twice: %q", repeat) + } + if other := m.NoticeFor("chat-2"); other == "" { + t.Error("a different chat was silenced by the first one's notice") + } + + writeAuthFile(t, home) + if m.Blocked() { + t.Error("Blocked() = true after a completed login") + } + // NoticeReset (not a discarded NoticeFor) clears the registry, so a returning + // lapse is announced afresh — including to a chat that stayed quiet through the + // recovery. + m.NoticeReset() + if err := os.Remove(filepath.Join(home, "auth.json")); err != nil { + t.Fatalf("remove auth.json: %v", err) + } + if again := m.NoticeFor("chat-1"); again == "" { + t.Error("a returning lapse was not announced") + } +} + +// TestNilManagerIsInert: an adapter wired without a Manager must degrade to "no +// login needed" rather than panic. +func TestNilManagerIsInert(t *testing.T) { + var m *Manager + if m.Applicable() { + t.Error("nil Manager reported Applicable") + } + if !m.Authorized() { + t.Error("nil Manager reported unauthorized") + } + if m.Blocked() { + t.Error("nil Manager blocked a run") + } + if m.Cancel() { + t.Error("nil Manager cancelled a login") + } + if got := m.Dispatch(context.Background(), "", Subscriber{ID: "dm", Private: true}); got != NoLoginNeededText { + t.Errorf("nil Manager Dispatch = %q, want NoLoginNeededText", got) + } +} + +// TestDispatchNotApplicable: /login on a Claude or billing deployment explains +// itself instead of running anything. +func TestDispatchNotApplicable(t *testing.T) { + claude := NewManager(Config{Backend: "claude"}) + if got := claude.Dispatch(context.Background(), "", Subscriber{ID: "dm", Private: true}); !strings.Contains(got, "claude") { + t.Errorf("Dispatch = %q, want it to name the configured backend", got) + } + billing := NewManager(Config{Backend: BackendCodex, AuthMode: AuthBilling}) + dm := Subscriber{ID: "dm", Private: true} + if got := billing.Dispatch(context.Background(), "", dm); !strings.Contains(got, "CODEX_API_KEY") { + t.Errorf("Dispatch = %q, want the billing-mode explanation", got) + } +} + +// TestDispatchFullLoginPath walks the whole user-visible journey the feature +// exists for: an unauthorized deployment blocks runs, /login hands back a link +// and a one-time code while the CLI is still polling, the completed sign-in is +// confirmed, and the standard flow is unblocked — no restart involved. +func TestDispatchFullLoginPath(t *testing.T) { + home := t.TempDir() + // The fake CLI prints the real banner, then "completes" the browser step by + // writing the auth.json a real login would persist. + bin := fakeCodex(t, printBanner+"sleep 0.2\necho '{}' > \"$CODEX_HOME/auth.json\"\nexit 0\n") + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: home, + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + + if !m.Blocked() { + t.Fatal("runs were not blocked before the login") + } + + c := newCollector() + if reply := m.Dispatch(context.Background(), "", c.sub("chat-1")); reply != "" { + t.Errorf("immediate reply = %q, want it delivered through the subscriber instead", reply) + } + + // Order matters: the acknowledgement must not surface UNDER the code it + // promises, which is what sending it outside this channel used to risk. + if ack := c.next(t); !strings.Contains(ack, "Starting") { + t.Errorf("first notice = %q, want the acknowledgement first", ack) + } + prompt := c.next(t) + if !strings.Contains(prompt, "https://auth.openai.com/codex/device") { + t.Errorf("prompt %q is missing the verification link", prompt) + } + if !strings.Contains(prompt, "XER9-NWCA2") { + t.Errorf("prompt %q is missing the one-time code", prompt) + } + if !strings.Contains(prompt, "15 min") { + t.Errorf("prompt %q is missing the code lifetime", prompt) + } + + done := c.next(t) + if !strings.Contains(strings.ToLower(done), "authorized") { + t.Errorf("final notice = %q, want a success confirmation", done) + } + if m.Blocked() { + t.Error("runs are still blocked after a successful login") + } +} + +// TestDispatchReshowsPendingPrompt: a second /login while one is pending must +// re-show the SAME code, not start a competing flow that would invalidate it. +func TestDispatchReshowsPendingPrompt(t *testing.T) { + runs := filepath.Join(t.TempDir(), "runs") + bin := fakeCodex(t, "echo x >> "+runs+"\n"+printBanner+"sleep 30\n") + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + t.Cleanup(func() { m.Cancel() }) + + c := newCollector() + m.Dispatch(context.Background(), "", c.sub("chat-1")) + c.awaitCode(t) + + // The re-show goes through the subscriber (so a broadcast cannot overtake it), + // which is why Dispatch itself returns nothing for the adapter to send. + if again := m.Dispatch(context.Background(), "", c.sub("chat-1")); again != "" { + t.Errorf("second /login returned %q, want it delivered through the subscriber", again) + } + reshow := c.next(t) + if !strings.Contains(reshow, deviceCode) { + t.Errorf("re-show = %q, want the pending code", reshow) + } + if !strings.Contains(reshow, "already pending") { + t.Errorf("re-show = %q, want it to say a login is already pending", reshow) + } + + //nolint:gosec // the path is a test-owned temp file + data, err := os.ReadFile(runs) + if err != nil { + t.Fatalf("read run log: %v", err) + } + if got := strings.Count(string(data), "x"); got != 1 { + t.Errorf("the CLI ran %d times, want 1: a second login would invalidate the first code", got) + } +} + +// TestDispatchCancel aborts a pending sign-in and reports the cancellation. +func TestDispatchCancel(t *testing.T) { + bin := fakeCodex(t, printBanner+"sleep 30\n") + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + + if got := m.Dispatch(context.Background(), "cancel", dmSub); !strings.Contains(got, "No Codex login is pending") { + t.Errorf("cancel with nothing pending = %q", got) + } + + c := newCollector() + m.Dispatch(context.Background(), "", c.sub("chat-1")) + c.awaitCode(t) + + if got := m.Dispatch(context.Background(), "cancel", c.sub("chat-1")); !strings.Contains(got, "Cancelled") { + t.Errorf("cancel = %q, want a cancellation acknowledgement", got) + } + // Told ONCE. The attempt's own verdict ("The Codex sign-in was cancelled") would + // land right behind Dispatch's acknowledgement and say the same thing again. + select { + case extra := <-c.ch: + t.Errorf("the cancellation was reported twice; extra notice: %q", extra) + case <-time.After(500 * time.Millisecond): + } +} + +// TestDispatchAlreadyAuthorized: /login on an authorized deployment reports the +// state and offers the explicit re-login rather than silently replacing a +// working credential. +func TestDispatchAlreadyAuthorized(t *testing.T) { + home := t.TempDir() + writeAuthFile(t, home) + m := NewManager(Config{Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: home}) + + got := m.Dispatch(context.Background(), "", Subscriber{ID: "dm", Private: true}) + if !strings.Contains(got, "authorized") || !strings.Contains(got, "/login force") { + t.Errorf("Dispatch = %q, want the authorized state plus the force hint", got) + } +} + +// TestDispatchStatusAndUsage covers the read-only arguments. +func TestDispatchStatusAndUsage(t *testing.T) { + m := NewManager(Config{Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: t.TempDir()}) + + if got := m.Dispatch(context.Background(), "status", dmSub); !strings.Contains(got, "NOT authorized") { + t.Errorf("status = %q, want the unauthorized state", got) + } + if got := m.Dispatch(context.Background(), "help", Subscriber{ID: "dm", Private: true}); got != LoginUsage { + t.Errorf("help = %q, want LoginUsage", got) + } + if got := m.Dispatch(context.Background(), "wat", dmSub); !strings.Contains(got, "Unknown /login argument") { + t.Errorf("unknown arg = %q, want the usage hint", got) + } +} + +// TestStatusTextNeverLeaksSecrets: an access-token deploy reports only THAT one +// is configured. +func TestStatusTextNeverLeaksSecrets(t *testing.T) { + m := NewManager(Config{ + Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: t.TempDir(), HasAccessToken: true, + }) + got := m.StatusText() + if !strings.Contains(got, "access token") { + t.Errorf("StatusText = %q, want it to mention the configured access token", got) + } +} + +// TestDispatchSurvivesADeadRequestContext is the guarantee that makes the whole +// flow usable: the update whose handler ran /login is finished (and its context +// cancelled) long before a human finishes signing in, so the login must NOT be +// tied to it. +func TestDispatchSurvivesADeadRequestContext(t *testing.T) { + bin := fakeCodex(t, printBanner+persistAuth+"exit 0\n") + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + + ctx, cancel := context.WithCancel(context.Background()) + c := newCollector() + m.Dispatch(ctx, "", c.sub("chat-1")) + cancel() // the update's context dies immediately, as it does in production + + c.awaitCode(t) + if done := c.next(t); !strings.Contains(strings.ToLower(done), "authorized") { + t.Errorf("final notice = %q, want the login to have completed anyway", done) + } +} + +// TestRequireAuthFalseNeverBlocks: CODEX_REQUIRE_AUTH=false is an explicit +// operator opt-out — they have taken responsibility for credentials this process +// cannot observe (a home mounted later, config.toml). Blocking their runs would +// break a deployment that used to work, so the gate must stay open while /login +// stays offered. +func TestRequireAuthFalseNeverBlocks(t *testing.T) { + m := NewManager(Config{ + Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: false, Home: t.TempDir(), + }) + + if !m.Authorized() { + t.Error("Authorized() = false with CODEX_REQUIRE_AUTH=false") + } + if m.Blocked() { + t.Error("runs were blocked with CODEX_REQUIRE_AUTH=false") + } + status := m.StatusText() + if !strings.Contains(status, "CODEX_REQUIRE_AUTH=false") { + t.Errorf("status = %q, want it to explain why nothing is blocked", status) + } + // /login must still be usable: without a persisted credential it starts a real + // sign-in rather than reporting "already authorized". + if got := m.StatusText(); strings.Contains(got, "already") { + t.Errorf("status = %q, want it not to claim an existing login", got) + } +} + +// TestSubscriptionLoginDropsInheritedAPIKey: a stray CODEX_API_KEY in the parent +// environment must never reach a subscription sign-in — that is the one path that +// could silently convert a subscription deploy to usage-based API billing. +func TestSubscriptionLoginDropsInheritedAPIKey(t *testing.T) { + cfg := Config{ + Backend: BackendCodex, AuthMode: AuthSubscription, Home: t.TempDir(), + Env: []string{"PATH=/usr/bin", "CODEX_API_KEY=sk-live", "CODEX_HOME=/elsewhere"}, + } + env := cfg.childEnv() + for _, kv := range env { + if strings.HasPrefix(kv, "CODEX_API_KEY=") { + t.Error("CODEX_API_KEY reached a subscription login") + } + if kv == "CODEX_HOME=/elsewhere" { + t.Error("the inherited CODEX_HOME was not overridden") + } + } + if env[len(env)-1] != "CODEX_HOME="+cfg.Home { + t.Errorf("CODEX_HOME = %q, want the configured home", env[len(env)-1]) + } +} + +// TestNoCodeReachesANonPrivateDestination is the regression for the guard that +// an argument-based check missed: /login status ALSO prints a pending code, so +// the decision has to key off the destination, not the arguments. A group asking +// anything at all must never see the link or the code. +func TestNoCodeReachesANonPrivateDestination(t *testing.T) { + bin := fakeCodex(t, printBanner+"sleep 30\n") + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + t.Cleanup(func() { m.Cancel() }) + + // A sign-in is pending, started from a direct message. + dm := newCollector() + m.Dispatch(context.Background(), "", Subscriber{ID: "dm", Private: true, Notify: dm.notify}) + dm.awaitCode(t) + + group := Subscriber{ID: "group", Private: false, Notify: func(string) {}} + for _, args := range []string{"", "status", "force", "again"} { + t.Run("args="+args, func(t *testing.T) { + got := m.Dispatch(context.Background(), args, group) + if strings.Contains(got, deviceCode) { + t.Errorf("/login %q leaked the one-time code into a group: %q", args, got) + } + if strings.Contains(got, deviceURL) { + t.Errorf("/login %q leaked the verification link into a group: %q", args, got) + } + }) + } +} + +// TestGroupStatusStillReportsSomethingUseful: withholding the code must not turn +// into silence — a user in a group is told a sign-in is pending and where it went. +func TestGroupStatusStillReportsSomethingUseful(t *testing.T) { + m := NewManager(Config{ + Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: t.TempDir(), + }) + if got := m.Dispatch(context.Background(), "status", Subscriber{ID: "group"}); !strings.Contains(got, "NOT authorized") { + t.Errorf("group status = %q, want the state (no code is involved when none is pending)", got) + } + if got := m.Dispatch(context.Background(), "", Subscriber{ID: "group"}); !strings.Contains(got, "direct message") { + t.Errorf("group /login = %q, want the direct-message refusal", got) + } +} + +// TestGroupSubscriberNeverJoinsTheBroadcast: a non-private destination must not +// be subscribed either, or the prompt broadcast would hand it the code. +func TestGroupSubscriberNeverJoinsTheBroadcast(t *testing.T) { + // Runs to completion, so the private subscriber has a verdict to receive and the + // group's silence is a real result rather than nothing having happened. + bin := fakeCodex(t, printBanner+persistAuth+"exit 0\n") + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + t.Cleanup(func() { m.Cancel() }) + + group := newCollector() + dm := newCollector() + m.Dispatch(context.Background(), "", Subscriber{ID: "dm", Private: true, Notify: dm.notify}) + dm.awaitCode(t) + m.Dispatch(context.Background(), "", Subscriber{ID: "group", Private: false, Notify: group.notify}) + + if got := dm.next(t); !strings.Contains(strings.ToLower(got), "authorized") { + t.Fatalf("the private subscriber did not get the outcome: %q", got) + } + select { + case leaked := <-group.ch: + t.Errorf("a group destination received a login notice: %q", leaked) + case <-time.After(200 * time.Millisecond): + } +} + +// TestCancelWaitsForTheChild: Cancel must not return while the login goroutine is +// still unwinding. Shutdown relies on it to reap the polling child, and a /login +// issued straight after a cancel must start fresh rather than be told "already +// pending" alongside a code that is already dead. +func TestCancelWaitsForTheChild(t *testing.T) { + bin := fakeCodex(t, printBanner+"sleep 30\n") + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + + c := newCollector() + m.Dispatch(context.Background(), "", c.sub("dm")) + c.awaitCode(t) + + if !m.Cancel() { + t.Fatal("Cancel reported nothing pending") + } + // Immediately after Cancel the manager must already be idle. + if got := m.StatusText(); strings.Contains(got, "in progress") { + t.Errorf("StatusText = %q right after Cancel; the run was still marked pending", got) + } + if got := m.Dispatch(context.Background(), "", c.sub("dm")); strings.Contains(got, "already pending") { + t.Errorf("a fresh /login after cancel was refused with a dead code: %q", got) + } + m.Cancel() +} + +// TestEveryAskerLearnsTheOutcome: a second /login while one is pending re-shows +// the same code — and must also start receiving the verdict. Without the +// subscription, only the chat that happened to start the login would ever learn +// whether it worked. +func TestEveryAskerLearnsTheOutcome(t *testing.T) { + bin := fakeCodex(t, printBanner+persistAuth+"exit 0\n") + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + + first, second := newCollector(), newCollector() + m.Dispatch(context.Background(), "", first.sub("chat-1")) + first.awaitCode(t) + + // A different chat asks while the login is pending and is re-shown the code + // through its own subscription. + if again := m.Dispatch(context.Background(), "", second.sub("chat-2")); again != "" { + t.Fatalf("second /login returned %q, want it delivered through the subscriber", again) + } + second.awaitCode(t) + + for name, c := range map[string]*collector{"starter": first, "joiner": second} { + if got := c.next(t); !strings.Contains(strings.ToLower(got), "authorized") { + t.Errorf("%s got %q as the final notice, want the success confirmation", name, got) + } + } +} + +// TestTheSameChatAskingTwiceIsNotToldTwice: subscriptions are keyed by +// destination, so a user tapping /login twice does not double every later notice. +func TestTheSameChatAskingTwiceIsNotToldTwice(t *testing.T) { + bin := fakeCodex(t, printBanner+persistAuth+"exit 0\n") + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + + c := newCollector() + m.Dispatch(context.Background(), "", c.sub("chat-1")) + c.awaitCode(t) + // Asking again re-shows the code to the asker — that is what asking is for — + // but must not double every notice that FOLLOWS. + m.Dispatch(context.Background(), "", c.sub("chat-1")) + c.awaitCode(t) + + if got := c.next(t); !strings.Contains(strings.ToLower(got), "authorized") { + t.Fatalf("final notice = %q, want the success confirmation", got) + } + select { + case extra := <-c.ch: + t.Errorf("the same chat was told twice; extra notice: %q", extra) + case <-time.After(300 * time.Millisecond): + } +} + +// TestCleanExitWithoutACredentialIsNotSuccess: what authorizes the deployment is +// the credential on disk, which is what the run gate reads. Announcing success on +// the CLI's exit code alone would tell the user they are signed in and then +// refuse their very next message. +func TestCleanExitWithoutACredentialIsNotSuccess(t *testing.T) { + home := t.TempDir() + bin := fakeCodex(t, printBanner+"exit 0\n") // exits clean, writes nothing + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: home, + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + + c := newCollector() + m.Dispatch(context.Background(), "", c.sub("dm")) + c.awaitCode(t) + + got := c.next(t) + if strings.Contains(strings.ToLower(got), "codex is authorized") { + t.Errorf("reported success with no persisted credential: %q", got) + } + if !strings.Contains(got, home) { + t.Errorf("final notice = %q, want it to name the directory that stayed empty", got) + } + if !m.Blocked() { + t.Error("runs were unblocked by a login that persisted nothing") + } +} + +// TestAckPrecedesTheCode pins the message ORDER: the acknowledgement must not +// surface underneath the code it promises. +func TestAckPrecedesTheCode(t *testing.T) { + bin := fakeCodex(t, printBanner+"sleep 30\n") + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + t.Cleanup(func() { m.Cancel() }) + + c := newCollector() + if reply := m.Dispatch(context.Background(), "", c.sub("dm")); reply != "" { + t.Fatalf("Dispatch returned %q, want it delivered through the subscriber", reply) + } + // Returning the acknowledgement for the caller to send separately raced the + // broadcast: a CLI that printed its banner quickly got the code out first, and + // the user read "the code arrives in a moment" underneath the code. + if got := c.next(t); !strings.Contains(got, "Starting") { + t.Errorf("first notice = %q, want the acknowledgement before the code", got) + } + if got := c.next(t); !strings.Contains(got, deviceCode) { + t.Errorf("second notice = %q, want the code", got) + } +} + +// TestDispatchStillReturnsAReplyWithoutASubscriber: a caller that cannot receive +// notices (no Notify) must still get the text to send itself. +func TestDispatchStillReturnsAReplyWithoutASubscriber(t *testing.T) { + m := NewManager(Config{ + Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: t.TempDir(), + Bin: filepath.Join(t.TempDir(), "no-such-codex"), + }) + t.Cleanup(func() { m.Cancel() }) + + if got := m.Dispatch(context.Background(), "", Subscriber{ID: "dm", Private: true}); got == "" { + t.Error("Dispatch swallowed the reply for a subscriber that cannot receive it") + } +} + +// TestGroupStatusHidesTheHostPath: the authorized branch names CODEX_HOME, which +// is deployment detail a group chat has no business with. /login status reaches +// that branch from anywhere, so the path is private-only too. +func TestGroupStatusHidesTheHostPath(t *testing.T) { + home := t.TempDir() + writeAuthFile(t, home) + m := NewManager(Config{ + Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: home, + }) + + group := m.Dispatch(context.Background(), "status", Subscriber{ID: "group"}) + if strings.Contains(group, home) { + t.Errorf("group status leaked the host path: %q", group) + } + if !strings.Contains(group, "authorized") { + t.Errorf("group status = %q, want it to still report the state", group) + } + if dm := m.Dispatch(context.Background(), "status", dmSub); !strings.Contains(dm, home) { + t.Errorf("direct-message status = %q, want the path an operator needs", dm) + } +} + +// TestCancelIsPrivateOnly: cancel is the one state-MUTATING argument, so it obeys +// the same rule as starting. Otherwise a conversation could abort a sign-in +// running in someone else's direct message — and the confirmation would itself +// reveal that one is under way. +func TestCancelIsPrivateOnly(t *testing.T) { + bin := fakeCodex(t, printBanner+"sleep 30\n") + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + t.Cleanup(func() { m.Cancel() }) + + c := newCollector() + m.Dispatch(context.Background(), "", c.sub("dm")) + c.awaitCode(t) + + got := m.Dispatch(context.Background(), "cancel", Subscriber{ID: "group"}) + if got != cancelPrivateOnlyText { + t.Errorf("group cancel = %q, want the cancel-specific refusal", got) + } + // The refusal must explain ITSELF: a cancel reply carries no code, so borrowing + // the start refusal's reasoning would simply be untrue. + if strings.Contains(got, "one-time code") { + t.Errorf("cancel refusal = %q, want it not to claim a code is at stake", got) + } + if !strings.Contains(m.StatusText(), "in progress") { + t.Error("a group aborted a sign-in owned by a direct message") + } + + if reply := m.Dispatch(context.Background(), "cancel", dmSub); !strings.Contains(reply, "Cancelled") { + t.Errorf("direct-message cancel = %q, want it to work", reply) + } +} + +// TestCancelLeavesNoPendingStateEvenOnTimeout is the state-machine regression. +// The drain is reachable in normal operation — the login goroutine may be inside +// a notice delivery, bounded by NotifyTimeout, which is longer than cancelDrain. +// When that happened, Cancel cleared the cancel func but left the attempt marked +// running: the next /login re-showed the code it had just killed, and the next +// /login cancel answered "nothing is pending". The attempt is now detached before +// any waiting, so a timeout leaves nothing behind. +func TestCancelLeavesNoPendingStateEvenOnTimeout(t *testing.T) { + bin := fakeCodex(t, printBanner+"sleep 30\n") + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + t.Cleanup(func() { m.Cancel() }) + + // A subscriber that takes the acknowledgement and then wedges on the prompt, + // standing in for a transport that drops into a 429 back-off. The login + // goroutine is thus stuck inside a delivery — which is what makes the drain + // reachable, since a delivery is bounded by NotifyTimeout, not by cancelDrain. + release := make(chan struct{}) + t.Cleanup(func() { close(release) }) + var delivered atomic.Int64 + stuck := Subscriber{ID: "dm", Private: true, Notify: func(string) { + if delivered.Add(1) > 1 { + <-release + } + }} + + m.Dispatch(context.Background(), "", stuck) + waitFor(t, func() bool { return delivered.Load() > 1 }) + + if !m.Cancel() { + t.Fatal("Cancel reported nothing pending") + } + if got := m.StatusText(); strings.Contains(got, "in progress") { + t.Errorf("StatusText = %q after Cancel; the attempt was left marked pending", got) + } + if m.Cancel() { + t.Error("a second Cancel found something pending") + } + // The decisive symptom: a fresh /login must not be answered with the dead code. + if got := m.Dispatch(context.Background(), "", dmSub); strings.Contains(got, "already pending") { + t.Errorf("a fresh /login after cancel = %q, want a new attempt", got) + } +} + +// TestOutcomeGoesToTheAttemptThatProducedIt: the verdict is delivered to a +// snapshot of THIS attempt's subscribers. Reading the live list instead let a +// successor's chat receive the predecessor's failure — seconds after its own +// "Starting the sign-in" — while the chat that actually ran it heard nothing. +func TestOutcomeGoesToTheAttemptThatProducedIt(t *testing.T) { + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: fakeCodex(t, printBanner+"sleep 30\n"), + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + t.Cleanup(func() { m.Cancel() }) + + first, second := newCollector(), newCollector() + m.Dispatch(context.Background(), "", first.sub("chat-a")) + first.awaitCode(t) + + // Cancel detaches attempt A; B starts while A is still unwinding. + m.Cancel() + m.Dispatch(context.Background(), "", second.sub("chat-b")) + second.awaitCode(t) + + // Nothing belonging to A may surface in B's chat. Reading the live subscriber + // list instead of A's own snapshot is what used to send A's verdict to B — + // seconds after B's "Starting the sign-in", where it reads as B's own result. + for _, text := range drain(second) { + low := strings.ToLower(text) + if strings.Contains(low, "cancelled") || strings.Contains(low, "failed") { + t.Errorf("the successor's chat received the predecessor's verdict: %q", text) + } + } + // And A, having been cancelled, was told exactly once — by Cancel itself. + for _, text := range drain(first) { + if strings.Contains(strings.ToLower(text), "cancelled") { + t.Errorf("a cancelled attempt also announced its verdict: %q", text) + } + } +} + +// drain returns every notice delivered so far, without blocking. +func drain(c *collector) []string { + var out []string + for { + select { + case text := <-c.ch: + out = append(out, text) + default: + return out + } + } +} + +// waitFor polls cond until it holds or the test times out. +func waitFor(t *testing.T, cond func() bool) { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) { + if cond() { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatal("condition not met before deadline") +} + +// TestSilenceWithoutACodeIsReported guards against the package's own failure +// mode returning by the back door. If the parser never pairs a URL with a code — +// a changed banner, a code printed with an unexpected prefix — the user would +// read "the link and one-time code arrive in a moment" and then hear nothing for +// DeviceCodeTTL. That is "it just hangs" with friendlier wording, and it is the +// symptom this package exists to remove. +func TestSilenceWithoutACodeIsReported(t *testing.T) { + restore := promptWarnAfter + promptWarnAfter = 50 * time.Millisecond + t.Cleanup(func() { promptWarnAfter = restore }) + + // A CLI that prints something unparseable and then polls, as a drifted banner + // would. + bin := fakeCodex(t, "echo 'signing you in, please hold'\nsleep 30\n") + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: bin, + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + t.Cleanup(func() { m.Cancel() }) + + c := newCollector() + m.Dispatch(context.Background(), "", c.sub("dm")) + if ack := c.next(t); !strings.Contains(ack, "Starting") { + t.Fatalf("first notice = %q, want the acknowledgement", ack) + } + + got := c.next(t) + if !strings.Contains(got, "hasn't printed a sign-in code") { + t.Errorf("notice = %q, want the no-code warning", got) + } + if !strings.Contains(got, "/login cancel") { + t.Errorf("notice = %q, want it to name the way out", got) + } +} + +// TestNoSilenceWarningOnceTheCodeArrives: the guard must stay quiet on the happy +// path, or every login would carry a spurious "no code yet". +func TestNoSilenceWarningOnceTheCodeArrives(t *testing.T) { + restore := promptWarnAfter + promptWarnAfter = 50 * time.Millisecond + t.Cleanup(func() { promptWarnAfter = restore }) + + m := NewManager(Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + RequireAuth: true, + Bin: fakeCodex(t, printBanner+"sleep 30\n"), + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + t.Cleanup(func() { m.Cancel() }) + + c := newCollector() + m.Dispatch(context.Background(), "", c.sub("dm")) + c.awaitCode(t) + + select { + case extra := <-c.ch: + t.Errorf("a warning fired after the code arrived: %q", extra) + case <-time.After(300 * time.Millisecond): + } +} + +// TestBlockedNoticePointsAtADirectMessage: the run gate cannot see the +// destination, so the notice it sends must be correct in a group too — telling a +// user to send /login where it will be refused earns them two refusals in a row, +// possibly in the only chat they use with the bot. +func TestBlockedNoticePointsAtADirectMessage(t *testing.T) { + m := NewManager(Config{ + Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: t.TempDir(), + }) + if !m.Blocked() { + t.Fatal("Blocked() = false while unauthorized") + } + notice := m.NoticeFor("dm") + if !strings.Contains(notice, "direct message") { + t.Errorf("notice = %q, want it to name where /login is accepted", notice) + } +} + +// TestLoginAdvertisedNarrowerThanApplicable: a deployment carrying +// CODEX_ACCESS_TOKEN is authorized without a browser, so /login must not be +// offered there — it could only answer that it is not needed. The command itself +// stays available, because /login force is a legitimate way to replace a +// workspace token with a personal subscription. +func TestLoginAdvertisedNarrowerThanApplicable(t *testing.T) { + withToken := NewManager(Config{ + Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, + Home: t.TempDir(), HasAccessToken: true, + }) + if !withToken.Applicable() { + t.Error("Applicable() = false; /login force must stay usable with a token configured") + } + if withToken.LoginAdvertised() { + t.Error("LoginAdvertised() = true with an access token; the command would answer 'not needed'") + } + + plain := NewManager(Config{ + Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: t.TempDir(), + }) + if !plain.LoginAdvertised() { + t.Error("LoginAdvertised() = false on the deployment whose whole setup is /login") + } + + var nilManager *Manager + if nilManager.LoginAdvertised() { + t.Error("a nil Manager advertised /login") + } +} + +// TestBackgroundRefusalDoesNotConsumeTheUsersAnswer: the two channels keep +// independent registries. A container that comes up with a lost auth.json +// replays its markers, which refuses and explains — and the person who then +// writes to the bot must still be answered, because telling a human that /login +// is needed is the entire point of the feature. +func TestBackgroundRefusalDoesNotConsumeTheUsersAnswer(t *testing.T) { + m := NewManager(Config{ + Backend: BackendCodex, AuthMode: AuthSubscription, RequireAuth: true, Home: t.TempDir(), + }) + + if background := m.NoticeFor("chat-1"); background == "" { + t.Fatal("the background channel said nothing while unauthorized") + } + if reply := m.ReplyNoticeFor("chat-1"); reply == "" { + t.Error("a background refusal swallowed the answer owed to the user") + } + // Each channel still refuses to repeat itself. + if repeat := m.ReplyNoticeFor("chat-1"); repeat != "" { + t.Errorf("the user was answered twice: %q", repeat) + } + if repeat := m.NoticeFor("chat-1"); repeat != "" { + t.Errorf("the background channel repeated itself: %q", repeat) + } +} diff --git a/core/codexauth/codexauthtest/fakecli.go b/core/codexauth/codexauthtest/fakecli.go new file mode 100644 index 0000000..13c31d3 --- /dev/null +++ b/core/codexauth/codexauthtest/fakecli.go @@ -0,0 +1,92 @@ +// Package codexauthtest provides the shared stand-in for the Codex CLI used by +// every test that drives a device login — core/codexauth's own, and both +// adapters'. +// +// It exists so the captured CLI output has ONE home. The banner below is real, +// copied verbatim from `codex login --device-auth`, ANSI escapes included: those +// escapes are half of what the parser has to survive, so a simplified local copy +// would let an adapter test pass against output the production parser would never +// see. With three copies, a change to Codex's output meant three edits and two +// chances to be quietly left behind. +package codexauthtest + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// The verification link and one-time code the banner carries. +const ( + DeviceURL = "https://auth.openai.com/codex/device" + DeviceCode = "XER9-NWCA2" +) + +// Banner is what `codex login --device-auth` really prints, captured from the +// CLI: an ANSI-colored preamble, the verification URL, and the one-time code on +// its own line. The escapes are REAL — surviving them is the point. +const Banner = "\n" + + "Welcome to Codex [v\x1b[90m0.146.0\x1b[0m]\n" + + "\x1b[90mOpenAI's command-line coding agent\x1b[0m\n" + + "\n" + + "Follow these steps to sign in with ChatGPT using device code authorization:\n" + + "\n" + + "1. Open this link in your browser and sign in to your account\n" + + " \x1b[94m" + DeviceURL + "\x1b[0m\n" + + "\n" + + "2. Enter this one-time code \x1b[90m(expires in 15 minutes)\x1b[0m\n" + + " \x1b[94m" + DeviceCode + "\x1b[0m\n" + +// PrintBanner is the shell snippet emitting Banner verbatim. A quoted heredoc +// keeps the apostrophes and escapes intact without shell interpretation. +const PrintBanner = "cat <<'CODEX_BANNER_EOF'" + Banner + "CODEX_BANNER_EOF\n" + +// Polling is a fake CLI that issues the prompt and then blocks, like the real one +// waiting on the browser step. +const Polling = PrintBanner + "sleep 30\n" + +// execPerm makes the written stand-in runnable, which is the whole point of it. +const execPerm os.FileMode = 0o755 + +// WriteCLI writes an executable stand-in for the Codex CLI whose body is the +// given shell script and returns its path. The script receives the same +// arguments the real CLI would. +func WriteCLI(t *testing.T, script string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "codex") + if err := os.WriteFile(path, []byte("#!/bin/sh\n"+script), execPerm); err != nil { + t.Fatalf("write fake codex: %v", err) + } + return path +} + +// promptWait bounds how long AwaitCode waits for the fake CLI's prompt. +const promptWait = 10 * time.Second + +// AwaitCode drains delivered notices until the one carrying DeviceCode and +// returns it. A pending login delivers its acknowledgement through the SAME +// channel as the prompt, and deliberately ahead of it, so a caller that wants the +// code has to skip past what precedes it. +// +// This package stops at the fixtures on purpose: a helper that built a +// codexauth.Manager would have to import codexauth, and codexauth's own tests +// import this package — that is an import cycle. What actually drifts (the +// captured banner and its tokens) is shared; the few lines of manager glue stay +// with each caller. +func AwaitCode(t *testing.T, notices <-chan string) string { + t.Helper() + deadline := time.After(promptWait) + for { + select { + case text := <-notices: + if strings.Contains(text, DeviceCode) { + return text + } + case <-deadline: + t.Fatal("the fake login never issued a prompt") + return "" + } + } +} diff --git a/core/codexauth/login.go b/core/codexauth/login.go new file mode 100644 index 0000000..561cfe4 --- /dev/null +++ b/core/codexauth/login.go @@ -0,0 +1,450 @@ +// Package codexauth drives the Codex CLI's DEVICE-CODE login from a chat, so a +// headless deploy (container, VPS) can be authorized without a shell on the host. +// +// Why device code and not plain `codex login`: the plain flow starts a loopback +// OAuth server on http://localhost:1455 and prints the authorize URL whose +// redirect_uri points back at that loopback. Inside a container nobody can reach +// it — the operator opens the URL on their laptop, the callback lands on THEIR +// localhost, and the CLI waits for a callback that never arrives. That is the +// "no code is shown and the CLI hangs forever" symptom. Codex itself says so on +// stderr ("On a remote or headless machine? Use `codex login --device-auth`"). +// +// `codex login --device-auth` instead prints, on STDOUT, a verification URL plus +// a one-time code and then BLOCKS, polling the token endpoint until the user +// finishes in a browser. Two consequences shape this package: +// +// 1. The URL and code must be read INCREMENTALLY from the live pipe. Anything +// that waits for the process to exit (cmd.Output/CombinedOutput) shows +// nothing at all until the login is already over — the same apparent hang. +// 2. Codex colors that output with ANSI escapes even when stdout is not a TTY, +// so the text has to be de-escaped before the URL and code can be matched. +package codexauth + +import ( + "bufio" + "context" + "errors" + "fmt" + "io" + "os/exec" + "regexp" + "strconv" + "strings" + "sync" + "syscall" + "time" +) + +// DeviceCodeTTL is how long Codex says a one-time code stays valid. The login +// process is capped slightly above it so an abandoned login cannot leak a child +// process for longer than the code it is waiting on. +const DeviceCodeTTL = 15 * time.Minute + +// killGrace is how long the child gets to exit after SIGTERM before SIGKILL. +const killGrace = 500 * time.Millisecond + +// outputTailBytes is how much CLI output we keep to enrich a failure message. +const outputTailBytes = 4096 + +// initScanBuf is the line scanner's initial buffer size, and maxScanLine caps a +// single output line so a pathological stream cannot grow it without bound. +const ( + initScanBuf = 4096 + maxScanLine = 1 << 20 +) + +// ErrDeviceAuthUnsupported reports a Codex CLI too old to know --device-auth. +// Without it there is no headless login path at all, so the operator has to +// upgrade the CLI rather than retry. +var ErrDeviceAuthUnsupported = errors.New( + "this Codex CLI does not support `codex login --device-auth`; upgrade the Codex CLI to authorize headlessly") + +// ErrNoPrompt reports that Codex exited before printing a verification URL and +// one-time code. The wrapped message carries the captured CLI output. +var ErrNoPrompt = errors.New("codex printed no device-login code") + +// Prompt is the user-actionable half of a device login: open URL in a browser, +// sign in, and enter Code there. ExpiresIn is Codex's own stated validity window +// (zero when it did not state one). +type Prompt struct { + URL string + Code string + ExpiresIn time.Duration +} + +// Login runs `codex login --device-auth` to completion and reports the outcome. +// +// onPrompt is called ONCE, as soon as the verification URL and one-time code have +// been parsed off the live output — long before the call returns — so the caller +// can relay them to the user while the CLI keeps polling. It may be nil. It runs +// ON the output-reading goroutine, which keeps it strictly ordered before the +// outcome the caller reports afterwards; in exchange it must not block +// indefinitely (adapters send their notice under a bounded context). +// +// Login blocks until the user finishes in the browser (nil), until Codex fails +// (error), or until ctx is done. On ctx cancellation the whole child process +// GROUP is signalled, so no polling child is left behind, and ctx.Err() is +// returned. Success is defined by Codex exiting 0, which is also when auth.json +// under CODEX_HOME has been written. +func Login(ctx context.Context, cfg Config, onPrompt func(Prompt)) error { + //nolint:gosec,noctx // bin is operator config; ctx drives the process-group kill below. + cmd := exec.Command(cfg.bin(), "login", "--device-auth") + cmd.Env = cfg.childEnv() + + // The device flow needs no stdin, but an inherited terminal would let the CLI + // block on a read. A closed stdin keeps it strictly non-interactive. + cmd.Stdin = nil + // Own process group: cancelling must kill the poller, not just the shell. + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + + stdout, err := cmd.StdoutPipe() + if err != nil { + return fmt.Errorf("codexauth: stdout pipe: %w", err) + } + stderr, err := cmd.StderrPipe() + if err != nil { + return fmt.Errorf("codexauth: stderr pipe: %w", err) + } + + if err := cmd.Start(); err != nil { + return fmt.Errorf("codexauth: start %s: %w", cfg.bin(), err) + } + + done := make(chan struct{}) + var ( + killMu sync.Mutex + killTimer *time.Timer + ) + var watch sync.WaitGroup + watch.Add(1) + go func() { + defer watch.Done() + select { + case <-ctx.Done(): + t := killGroup(cmd) + killMu.Lock() + killTimer = t + killMu.Unlock() + case <-done: + } + }() + + // Both streams are parsed: --device-auth prints the prompt on stdout, while + // diagnostics (including the "use --device-auth" hint and argument errors) + // arrive on stderr. Whichever carries the URL and code, we find it. + // + // Each stream gets its OWN parser, and a prompt is only released when the URL + // and the code came from the SAME one. The two readers are separate goroutines, + // so a shared parser would interleave them in nondeterministic order: a stray + // URL on stderr (an upgrade notice, a policy link) could then be paired with the + // real code from stdout and the user would be sent, silently, to the wrong page + // with a valid code. Only one prompt is emitted overall, whichever stream + // completes a pair first. + out := newTail(outputTailBytes) + var ( + emitMu sync.Mutex + emitted bool + sawPromptOnAny bool + ) + var streams sync.WaitGroup + for _, rd := range []io.Reader{stdout, stderr} { + streams.Add(1) + go func(rd io.Reader) { + defer streams.Done() + parser := &promptParser{} + scanLines(rd, func(line string) { + out.WriteString(line + "\n") + p, ok := parser.feed(line) + if !ok { + return + } + emitMu.Lock() + first := !emitted + emitted, sawPromptOnAny = true, true + emitMu.Unlock() + if first && onPrompt != nil { + onPrompt(p) + } + }) + }(rd) + } + streams.Wait() + + waitErr := cmd.Wait() + close(done) + watch.Wait() + killMu.Lock() + if killTimer != nil { + killTimer.Stop() + } + killMu.Unlock() + + emitMu.Lock() + sawPrompt := sawPromptOnAny + emitMu.Unlock() + + return loginResult(ctx, waitErr, out.String(), sawPrompt) +} + +// loginResult maps the child's exit onto the caller-facing error contract: a +// cancelled/expired ctx wins over any exit status (the kill caused it), an +// unknown --device-auth flag is reported as its own sentinel so the caller can +// tell the operator to upgrade, and a clean exit that never printed a prompt is +// still a failure — the user was never given anything to act on. +func loginResult(ctx context.Context, waitErr error, output string, sawPrompt bool) error { + if ctx.Err() != nil { + return ctx.Err() + } + if isUnsupportedFlag(output) { + return ErrDeviceAuthUnsupported + } + if waitErr != nil { + var ee *exec.ExitError + if errors.As(waitErr, &ee) { + return fmt.Errorf("codex login exited with code %d: %s", ee.ExitCode(), condense(redact(output))) + } + return fmt.Errorf("codex login failed: %w: %s", waitErr, condense(redact(output))) + } + if !sawPrompt { + return fmt.Errorf("%w: %s", ErrNoPrompt, condense(redact(output))) + } + return nil +} + +// isUnsupportedFlag detects a Codex CLI that does not know --device-auth. clap +// (the CLI parser Codex uses) words this as "unexpected argument"; older/other +// builds say "unrecognized". Matching either keeps the check robust without +// pinning a version. +func isUnsupportedFlag(output string) bool { + low := strings.ToLower(output) + if !strings.Contains(low, "device-auth") { + return false + } + return strings.Contains(low, "unexpected argument") || + strings.Contains(low, "unrecognized") || + strings.Contains(low, "unknown flag") || + strings.Contains(low, "invalid value") +} + +// redact removes the device-login secrets from captured CLI output before it is +// put in an error — which is logged, and may be shown in a chat. The one-time +// code is a live credential for as long as it lasts: anyone who reads it out of +// a log can complete the sign-in with their own account. The verification link +// goes too, since it is only useful together with the code and adds nothing to a +// diagnosis. +func redact(output string) string { + output = urlRe.ReplaceAllString(output, "") + return codeRe.ReplaceAllString(output, "") +} + +// condense collapses captured CLI output into one line for an error message, +// dropping blank lines so a mostly-decorative banner does not bury the cause. +func condense(output string) string { + fields := strings.FieldsFunc(output, func(r rune) bool { return r == '\n' || r == '\r' }) + kept := make([]string, 0, len(fields)) + for _, f := range fields { + if f = strings.TrimSpace(f); f != "" { + kept = append(kept, f) + } + } + if len(kept) == 0 { + return "(no output)" + } + return strings.Join(kept, " | ") +} + +// scanLines feeds rd's output to onLine one ANSI-stripped line at a time, +// splitting on \n AND \r so a progress line rewritten in place still surfaces. +// It returns when rd hits EOF. +func scanLines(rd io.Reader, onLine func(string)) { + sc := bufio.NewScanner(rd) + sc.Buffer(make([]byte, 0, initScanBuf), maxScanLine) + sc.Split(splitLines) + for sc.Scan() { + line := stripANSI(sc.Text()) + if strings.TrimSpace(line) == "" { + continue + } + onLine(line) + } +} + +// splitLines is a bufio.SplitFunc that breaks on either \n or \r, so output the +// CLI redraws with a carriage return is not withheld until the next newline. +func splitLines(data []byte, atEOF bool) (advance int, token []byte, err error) { + for i, b := range data { + if b == '\n' || b == '\r' { + return i + 1, data[:i], nil + } + } + if atEOF && len(data) > 0 { + return len(data), data, nil + } + return 0, nil, nil +} + +// ansiRe matches CSI/OSC-style escape sequences. Codex colors its login output +// even when stdout is a pipe, so the raw text carries escapes around exactly the +// two tokens we need (the URL and the code) and must be cleaned before matching. +var ansiRe = regexp.MustCompile(`\x1b\[[0-9;?]*[ -/]*[@-~]|\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)`) + +// stripANSI removes terminal escape sequences from s. +func stripANSI(s string) string { return ansiRe.ReplaceAllString(s, "") } + +var ( + // urlRe matches the verification link. Trailing punctuation is trimmed by the + // character class so a URL at the end of a sentence stays intact. + urlRe = regexp.MustCompile(`https?://[^\s"'<>)\]]+`) + // codeRe matches a one-time code such as "XER9-NWCA2". + codeRe = regexp.MustCompile(`\b[A-Z0-9]{4,8}-[A-Z0-9]{4,8}\b`) + // ttlRe matches Codex's stated validity window, e.g. "(expires in 15 minutes)". + ttlRe = regexp.MustCompile(`(?i)expires in (\d+) (second|minute|hour)s?`) +) + +// promptParser accumulates the device-login prompt across the CLI's output +// lines. Codex prints the URL and the code on separate lines, so neither alone +// is actionable; the prompt is released exactly once, on the line that completes +// the pair. +type promptParser struct { + url string + urlScore int + code string + ttl time.Duration + sent bool +} + +// feed consumes one ANSI-stripped output line. ok is true only for the line that +// first completes the URL+code pair. +func (p *promptParser) feed(line string) (Prompt, bool) { + if p.sent { + return Prompt{}, false + } + if m := urlRe.FindString(line); m != "" { + // Keep the BEST candidate rather than the first: an upgrade notice or a + // policy link printed before the verification URL must not win just because + // it came first, or the user would be sent to the wrong page with a code + // that looks perfectly valid. A single-URL run is unaffected — it scores + // whatever it scores and stays. + if candidate := strings.TrimRight(m, ".,;"); p.url == "" || urlScore(candidate) > p.urlScore { + p.url, p.urlScore = candidate, urlScore(candidate) + } + } + if p.code == "" { + if c, ok := matchCode(line); ok { + p.code = c + } + } + if p.ttl == 0 { + p.ttl = matchTTL(line) + } + if p.url == "" || p.code == "" { + return Prompt{}, false + } + p.sent = true + return Prompt{URL: p.url, Code: p.code, ExpiresIn: p.ttl}, true +} + +// URL candidate ranks, best first: the device path is unmistakable, a known host +// is good evidence, and anything else is only used for lack of an alternative. +const ( + scoreDevicePath = 2 + scoreKnownHost = 1 + scoreUnknown = 0 +) + +// urlScore ranks a URL by how much it looks like Codex's device-verification +// link. It is a preference, never a filter: an unrecognized URL is still used +// when it is the only one, so a changed domain degrades to today's behavior +// instead of breaking the flow outright. +func urlScore(u string) int { + low := strings.ToLower(u) + switch { + case strings.Contains(low, "/device"): + return scoreDevicePath + case strings.Contains(low, "openai.com"), strings.Contains(low, "chatgpt.com"): + return scoreKnownHost + default: + return scoreUnknown + } +} + +// matchCode extracts a one-time code from line. A bare uppercase hyphenated +// token is only accepted when it is the WHOLE line (how Codex prints it) or when +// the line is explicitly about a code — otherwise an unrelated token in a banner +// or inside a URL could be mistaken for one. +func matchCode(line string) (string, bool) { + trimmed := strings.TrimSpace(line) + if urlRe.MatchString(trimmed) { + return "", false + } + m := codeRe.FindString(trimmed) + if m == "" { + return "", false + } + if m == trimmed || strings.Contains(strings.ToLower(trimmed), "code") { + return m, true + } + return "", false +} + +// matchTTL reads Codex's stated code lifetime off a line, returning 0 when the +// line does not state one. +func matchTTL(line string) time.Duration { + m := ttlRe.FindStringSubmatch(line) + if m == nil { + return 0 + } + n, err := strconv.Atoi(m[1]) + if err != nil || n <= 0 { + return 0 + } + switch strings.ToLower(m[2]) { + case "second": + return time.Duration(n) * time.Second + case "hour": + return time.Duration(n) * time.Hour + default: + return time.Duration(n) * time.Minute + } +} + +// killGroup signals the child's whole process group, escalating to SIGKILL after +// killGrace. It mirrors core/codex's runner so a cancelled login cannot leave a +// polling Codex process behind. +func killGroup(cmd *exec.Cmd) *time.Timer { + if cmd.Process == nil { + return nil + } + pgid := cmd.Process.Pid + _ = syscall.Kill(-pgid, syscall.SIGTERM) + return time.AfterFunc(killGrace, func() { + _ = syscall.Kill(-pgid, syscall.SIGKILL) + }) +} + +// tail is a bounded io.Writer keeping only the last max bytes written, used to +// enrich failures with the CLI's own words without unbounded buffering. +type tail struct { + mu sync.Mutex + max int + buf []byte +} + +func newTail(limit int) *tail { return &tail{max: limit} } + +// WriteString appends s, dropping the oldest bytes past the cap. +func (t *tail) WriteString(s string) { + t.mu.Lock() + defer t.mu.Unlock() + t.buf = append(t.buf, s...) + if len(t.buf) > t.max { + t.buf = t.buf[len(t.buf)-t.max:] + } +} + +// String returns the retained tail. +func (t *tail) String() string { + t.mu.Lock() + defer t.mu.Unlock() + return string(t.buf) +} diff --git a/core/codexauth/login_test.go b/core/codexauth/login_test.go new file mode 100644 index 0000000..2338b56 --- /dev/null +++ b/core/codexauth/login_test.go @@ -0,0 +1,433 @@ +//nolint:testpackage // intentionally whitebox to test the device-login parser and manager internals +package codexauth + +import ( + "context" + "errors" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/duckbugio/flock/core/codexauth/codexauthtest" +) + +// The fake CLI, its captured banner and the tokens it carries live in +// codexauthtest so core/codexauth, the VK adapter and the Telegram adapter all +// assert against the SAME real output. +const ( + deviceURL = codexauthtest.DeviceURL + deviceCode = codexauthtest.DeviceCode + printBanner = codexauthtest.PrintBanner +) + +// fakeCodex writes an executable stand-in for the Codex CLI running script. +func fakeCodex(t *testing.T, script string) string { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("the fake CLI is a POSIX shell script") + } + return codexauthtest.WriteCLI(t, script) +} + +// testConfig points a Config at the fake CLI with a scratch CODEX_HOME and a +// minimal environment (an empty Env would inherit the test runner's). +func testConfig(t *testing.T, bin string) Config { + t.Helper() + return Config{ + Backend: BackendCodex, + AuthMode: AuthSubscription, + Bin: bin, + Home: t.TempDir(), + Env: []string{"PATH=" + os.Getenv("PATH")}, + } +} + +// TestLoginEmitsPromptWhileProcessStillRunning is the core regression for the +// reported symptom: with `codex login --device-auth` the URL and code are printed +// and the CLI THEN blocks for minutes, polling. Anything that waits for the +// process to exit before reading (cmd.Output/CombinedOutput) shows the user +// nothing and looks like a hang. The prompt must arrive while the child is still +// running. +func TestLoginEmitsPromptWhileProcessStillRunning(t *testing.T) { + bin := fakeCodex(t, printBanner+"sleep 30\n") + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + prompts := make(chan Prompt, 1) + errs := make(chan error, 1) + go func() { + errs <- Login(ctx, testConfig(t, bin), func(p Prompt) { prompts <- p }) + }() + + var got Prompt + select { + case got = <-prompts: + case err := <-errs: + t.Fatalf("Login returned %v before emitting a prompt", err) + case <-time.After(10 * time.Second): + t.Fatal("no prompt within 10s: the CLI output is not being read incrementally") + } + + if got.URL != deviceURL { + t.Errorf("URL = %q, want the device verification link", got.URL) + } + if got.Code != deviceCode { + t.Errorf("Code = %q, want XER9-NWCA2", got.Code) + } + if got.ExpiresIn != 15*time.Minute { + t.Errorf("ExpiresIn = %v, want 15m", got.ExpiresIn) + } + + // Cancelling must reap the still-polling child and report the ctx error. + cancel() + select { + case err := <-errs: + if !errors.Is(err, context.Canceled) { + t.Errorf("Login after cancel = %v, want context.Canceled", err) + } + case <-time.After(10 * time.Second): + t.Fatal("Login did not return after cancel: the child was not killed") + } +} + +// TestLoginSuccess: a CLI that prints the prompt and exits 0 is a completed +// sign-in. +func TestLoginSuccess(t *testing.T) { + bin := fakeCodex(t, printBanner+"exit 0\n") + var got Prompt + err := Login(context.Background(), testConfig(t, bin), func(p Prompt) { got = p }) + if err != nil { + t.Fatalf("Login = %v, want nil", err) + } + if got.Code == "" { + t.Error("onPrompt was never called") + } +} + +// TestLoginDeviceAuthUnsupported: a Codex CLI too old to know --device-auth has +// no headless login path at all, so the caller must be told to upgrade rather +// than to retry. +func TestLoginDeviceAuthUnsupported(t *testing.T) { + bin := fakeCodex(t, "echo \"error: unexpected argument '--device-auth' found\" >&2\nexit 2\n") + err := Login(context.Background(), testConfig(t, bin), nil) + if !errors.Is(err, ErrDeviceAuthUnsupported) { + t.Fatalf("Login = %v, want ErrDeviceAuthUnsupported", err) + } +} + +// TestLoginNoPrompt: a clean exit that never printed a code is still a failure — +// the user was given nothing to act on — and the error carries the CLI's output. +func TestLoginNoPrompt(t *testing.T) { + bin := fakeCodex(t, "echo 'nothing useful here'\nexit 0\n") + err := Login(context.Background(), testConfig(t, bin), nil) + if !errors.Is(err, ErrNoPrompt) { + t.Fatalf("Login = %v, want ErrNoPrompt", err) + } + if !strings.Contains(err.Error(), "nothing useful here") { + t.Errorf("error %q should quote the CLI output", err) + } +} + +// TestLoginFailureQuotesOutput: a non-zero exit reports the code and the CLI's +// own words, including stderr. +func TestLoginFailureQuotesOutput(t *testing.T) { + bin := fakeCodex(t, "echo 'network unreachable' >&2\nexit 7\n") + err := Login(context.Background(), testConfig(t, bin), nil) + if err == nil { + t.Fatal("Login = nil, want an error") + } + if !strings.Contains(err.Error(), "7") || !strings.Contains(err.Error(), "network unreachable") { + t.Errorf("error %q should carry the exit code and stderr", err) + } +} + +// TestLoginForcesConfiguredCodexHome: the login must write auth.json where the +// RUNNER later looks for it. An inherited CODEX_HOME pointing elsewhere would +// leave the deployment "logged in" and unauthorized at the same time. +func TestLoginForcesConfiguredCodexHome(t *testing.T) { + bin := fakeCodex(t, printBanner+"echo \"$CODEX_HOME\" > \"$CODEX_HOME/seen\"\nexit 0\n") + cfg := testConfig(t, bin) + cfg.Env = append(cfg.Env, "CODEX_HOME=/somewhere/else") + + if err := Login(context.Background(), cfg, nil); err != nil { + t.Fatalf("Login = %v, want nil", err) + } + seen, err := os.ReadFile(filepath.Join(cfg.Home, "seen")) + if err != nil { + t.Fatalf("read CODEX_HOME probe: %v", err) + } + if strings.TrimSpace(string(seen)) != cfg.Home { + t.Errorf("child CODEX_HOME = %q, want %q", strings.TrimSpace(string(seen)), cfg.Home) + } +} + +// TestLoginTimeoutReported: the device code expires, so an abandoned login ends +// with the deadline rather than hanging forever. +func TestLoginTimeoutReported(t *testing.T) { + bin := fakeCodex(t, printBanner+"sleep 30\n") + ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) + defer cancel() + + err := Login(ctx, testConfig(t, bin), nil) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("Login = %v, want context.DeadlineExceeded", err) + } +} + +// TestLoginMissingBinary: an unusable CODEX_BIN fails fast at start rather than +// leaving the caller waiting on a process that never existed. +func TestLoginMissingBinary(t *testing.T) { + cfg := testConfig(t, filepath.Join(t.TempDir(), "does-not-exist")) + if err := Login(context.Background(), cfg, nil); err == nil { + t.Fatal("Login = nil, want a start error") + } +} + +// TestStripANSI: Codex colors its login output even when stdout is a pipe, so the +// URL and code arrive wrapped in escapes. +func TestStripANSI(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + {"colored code", " \x1b[94mXER9-NWCA2\x1b[0m", " XER9-NWCA2"}, + {"plain text", "no escapes here", "no escapes here"}, + {"osc sequence", "\x1b]0;title\x07done", "done"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := stripANSI(tt.in); got != tt.want { + t.Errorf("stripANSI(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +// TestPromptParser covers the pairing rule: neither the URL nor the code alone is +// actionable, so the prompt is released exactly once, on the line completing the +// pair, and never again. +func TestPromptParser(t *testing.T) { + p := &promptParser{} + if _, ok := p.feed("1. Open this link"); ok { + t.Fatal("a prose line released a prompt") + } + if _, ok := p.feed(" https://auth.openai.com/codex/device"); ok { + t.Fatal("the URL alone released a prompt") + } + if _, ok := p.feed("2. Enter this one-time code (expires in 15 minutes)"); ok { + t.Fatal("the TTL line alone released a prompt") + } + got, ok := p.feed(" XER9-NWCA2") + if !ok { + t.Fatal("the code line did not release the prompt") + } + if got.URL != deviceURL || got.Code != deviceCode { + t.Errorf("prompt = %+v, want the parsed URL and code", got) + } + if got.ExpiresIn != 15*time.Minute { + t.Errorf("ExpiresIn = %v, want 15m", got.ExpiresIn) + } + if _, ok := p.feed(" ABCD-EFGH"); ok { + t.Error("a second prompt was released") + } +} + +// TestMatchCode pins the anti-false-positive rule: a hyphenated uppercase token is +// a code only when it is the whole line or the line is explicitly about a code — +// never when it comes out of a URL or an unrelated banner. +func TestMatchCode(t *testing.T) { + tests := []struct { + name string + line string + want string + }{ + {"bare code line", " XER9-NWCA2 ", deviceCode}, + {"labelled code", "Your one-time code: XER9-NWCA2 now", deviceCode}, + {"inside a url", "https://example.com/AAAA-BBBB", ""}, + {"unrelated banner token", "Release NOTES-DRAFT ready", ""}, + {"lowercase", " xer9-nwca2", ""}, + {"no hyphen", " XER9NWCA2", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := matchCode(tt.line) + if tt.want == "" { + if ok { + t.Errorf("matchCode(%q) = %q, want no match", tt.line, got) + } + return + } + if !ok || got != tt.want { + t.Errorf("matchCode(%q) = %q/%v, want %q", tt.line, got, ok, tt.want) + } + }) + } +} + +// TestMatchTTL reads Codex's stated code lifetime in each unit it might use. +func TestMatchTTL(t *testing.T) { + tests := []struct { + line string + want time.Duration + }{ + {"2. Enter this one-time code (expires in 15 minutes)", 15 * time.Minute}, + {"expires in 1 minute", time.Minute}, + {"expires in 90 seconds", 90 * time.Second}, + {"expires in 2 hours", 2 * time.Hour}, + {"no lifetime here", 0}, + {"expires in 0 minutes", 0}, + } + for _, tt := range tests { + t.Run(tt.line, func(t *testing.T) { + if got := matchTTL(tt.line); got != tt.want { + t.Errorf("matchTTL(%q) = %v, want %v", tt.line, got, tt.want) + } + }) + } +} + +// TestIsUnsupportedFlag distinguishes "this CLI cannot do device auth" from any +// other failure that merely mentions the flag. +func TestIsUnsupportedFlag(t *testing.T) { + tests := []struct { + name string + output string + want bool + }{ + {"clap wording", "error: unexpected argument '--device-auth' found", true}, + {"getopt wording", "unrecognized option: --device-auth", true}, + {"unrelated failure", "device-auth polling failed: network unreachable", false}, + {"other flag", "error: unexpected argument '--nope' found", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isUnsupportedFlag(tt.output); got != tt.want { + t.Errorf("isUnsupportedFlag(%q) = %v, want %v", tt.output, got, tt.want) + } + }) + } +} + +// TestSplitLinesBreaksOnCarriageReturn: a line the CLI redraws in place with \r +// must surface immediately instead of waiting for a newline that may never come. +func TestSplitLinesBreaksOnCarriageReturn(t *testing.T) { + advance, token, err := splitLines([]byte("waiting…\rdone\n"), false) + if err != nil { + t.Fatalf("splitLines err = %v", err) + } + if string(token) != "waiting…" { + t.Errorf("token = %q, want %q", token, "waiting…") + } + if advance != len("waiting…")+1 { + t.Errorf("advance = %d, want %d", advance, len("waiting…")+1) + } +} + +// TestLoginIgnoresAStrayURLFromTheOtherStream is the anti-mispairing guard. The +// two output streams are read by separate goroutines, so a shared parser would +// interleave them nondeterministically: an unrelated URL on stderr (an upgrade +// notice, a policy link) could be paired with the REAL code from stdout, and the +// user would be sent to the wrong page holding a code that works. The URL and +// the code must come from the same stream. +func TestLoginIgnoresAStrayURLFromTheOtherStream(t *testing.T) { + bin := fakeCodex(t, "echo 'A new version is available: https://example.com/upgrade' >&2\n"+ + "sleep 0.2\n"+printBanner+"exit 0\n") + + var got Prompt + if err := Login(context.Background(), testConfig(t, bin), func(p Prompt) { got = p }); err != nil { + t.Fatalf("Login = %v, want nil", err) + } + if got.URL != deviceURL { + t.Errorf("URL = %q, want the verification link, not the stray stderr one", got.URL) + } + if got.Code != deviceCode { + t.Errorf("Code = %q, want XER9-NWCA2", got.Code) + } +} + +// TestPromptParserPrefersTheVerificationURL: within one stream, a URL printed +// before the verification link must not win merely by being first. +func TestPromptParserPrefersTheVerificationURL(t *testing.T) { + p := &promptParser{} + p.feed("Docs: https://example.com/help") + p.feed(" https://auth.openai.com/codex/device") + got, ok := p.feed(" XER9-NWCA2") + if !ok { + t.Fatal("no prompt released") + } + if got.URL != deviceURL { + t.Errorf("URL = %q, want the device link to outrank the earlier one", got.URL) + } +} + +// TestPromptParserKeepsALoneUnrecognizedURL: the preference is a ranking, not a +// filter — if Codex ever moves off these domains, the only URL on offer is still +// used rather than the flow breaking outright. +func TestPromptParserKeepsALoneUnrecognizedURL(t *testing.T) { + p := &promptParser{} + p.feed(" https://login.example.test/activate") + got, ok := p.feed(" XER9-NWCA2") + if !ok { + t.Fatal("no prompt released") + } + if got.URL != "https://login.example.test/activate" { + t.Errorf("URL = %q, want the single available URL", got.URL) + } +} + +// TestURLScore pins the ranking. +func TestURLScore(t *testing.T) { + tests := []struct { + url string + want int + }{ + {deviceURL, scoreDevicePath}, + {"https://chatgpt.com/codex/settings", scoreKnownHost}, + {"https://example.com/upgrade", scoreUnknown}, + } + for _, tt := range tests { + t.Run(tt.url, func(t *testing.T) { + if got := urlScore(tt.url); got != tt.want { + t.Errorf("urlScore(%q) = %d, want %d", tt.url, got, tt.want) + } + }) + } +} + +// TestErrorsRedactTheOneTimeCode: a failure carries the CLI's own output, which +// is logged and may be shown in a chat — and that output contains a live +// credential. Anyone who reads the code out of a log can complete the sign-in +// with their own account, so it must never get that far. +func TestErrorsRedactTheOneTimeCode(t *testing.T) { + bin := fakeCodex(t, printBanner+"echo 'polling failed' >&2\nexit 4\n") + + err := Login(context.Background(), testConfig(t, bin), nil) + if err == nil { + t.Fatal("Login = nil, want an error") + } + if strings.Contains(err.Error(), deviceCode) { + t.Errorf("the one-time code leaked into an error: %q", err) + } + if strings.Contains(err.Error(), deviceURL) { + t.Errorf("the verification link leaked into an error: %q", err) + } + if !strings.Contains(err.Error(), "polling failed") { + t.Errorf("error %q lost the CLI's actual diagnosis", err) + } +} + +// TestRedactKeepsTheDiagnosis: redaction must not swallow the message that makes +// the failure debuggable. +func TestRedactKeepsTheDiagnosis(t *testing.T) { + got := redact("visit " + deviceURL + " and enter " + deviceCode + " — connection refused") + if strings.Contains(got, deviceCode) || strings.Contains(got, deviceURL) { + t.Errorf("redact left a secret in %q", got) + } + if !strings.Contains(got, "connection refused") { + t.Errorf("redact(%q) dropped the diagnosis", got) + } +} diff --git a/core/codexauth/logintest/logintest.go b/core/codexauth/logintest/logintest.go new file mode 100644 index 0000000..4e161ad --- /dev/null +++ b/core/codexauth/logintest/logintest.go @@ -0,0 +1,70 @@ +// Package logintest builds a codexauth.Manager with a device login already in +// flight, for tests that need to assert what each kind of destination is allowed +// to see while a one-time code exists. +// +// It is separate from codexauthtest for one reason: this package imports +// codexauth, and codexauth's OWN tests import codexauthtest for the captured CLI +// banner. Putting this helper there would close that loop into an import cycle. +// So codexauthtest holds what everyone needs (the fixtures) and stays import-free +// of codexauth; this package holds what only the adapters need. +package logintest + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/duckbugio/flock/core/codexauth" + "github.com/duckbugio/flock/core/codexauth/codexauthtest" +) + +// noticeBuffer holds the notices a pending login delivers before the code, so a +// slow reader cannot block the manager's broadcast. +const noticeBuffer = 8 + +// Unauthorized returns a Manager for a deployment that has never been signed in, +// and the scratch CODEX_HOME it watches, so a test can complete the sign-in by +// planting auth.json there. +// +// Bin points at a path that does not exist ON PURPOSE. Every caller previously +// set that by hand, and forgetting it in one future case would run the REAL +// `codex login --device-auth` on a developer's machine and leave it polling for +// the whole DeviceCodeTTL. Making it the default removes the chance to forget. +// +// (Here rather than in codexauthtest because this returns a codexauth type, and +// codexauthtest must not import codexauth — see the package doc.) +func Unauthorized(t *testing.T) (*codexauth.Manager, string) { + t.Helper() + home := t.TempDir() + return codexauth.NewManager(codexauth.Config{ + Backend: codexauth.BackendCodex, + AuthMode: codexauth.AuthSubscription, + RequireAuth: true, + Home: home, + Bin: filepath.Join(home, "no-such-codex"), + }), home +} + +// PendingLogin returns a Manager whose device login is under way and whose +// one-time code has already been issued, with a scratch CODEX_HOME holding no +// credential. The login is cancelled when the test ends. +func PendingLogin(t *testing.T) *codexauth.Manager { + t.Helper() + m := codexauth.NewManager(codexauth.Config{ + Backend: codexauth.BackendCodex, + AuthMode: codexauth.AuthSubscription, + RequireAuth: true, + Home: t.TempDir(), + Bin: codexauthtest.WriteCLI(t, codexauthtest.Polling), + Env: []string{"PATH=" + os.Getenv("PATH")}, + }) + t.Cleanup(func() { m.Cancel() }) + + issued := make(chan string, noticeBuffer) + m.Dispatch(context.Background(), "", codexauth.Subscriber{ + ID: "dm", Private: true, Notify: func(text string) { issued <- text }, + }) + codexauthtest.AwaitCode(t, issued) + return m +} diff --git a/core/followup/followup.go b/core/followup/followup.go index deab1d9..b5b51ea 100644 --- a/core/followup/followup.go +++ b/core/followup/followup.go @@ -187,7 +187,14 @@ const tick = 30 * time.Second // Run fires due follow-ups until ctx is cancelled, handing each to fire (in // production a closure over chat.Service.InjectAuto). It returns ctx.Err() on // cancellation. -func Run(ctx context.Context, store *FileStore, now func() time.Time, fire func(Item)) error { +// +// paused reports that firing is impossible right now (in production: the AI +// provider is unauthorized). The WHOLE sweep is skipped while it holds — Due is +// not called at all. That is the point: Due is take-then-fire, removing what it +// returns, so sweeping into a fire that drops the item would delete scheduled +// work permanently. Skipping instead leaves each item in the store, due, to fire +// once the block clears. A nil paused never pauses. +func Run(ctx context.Context, store *FileStore, now func() time.Time, paused func() bool, fire func(Item)) error { if now == nil { now = time.Now } @@ -198,13 +205,25 @@ func Run(ctx context.Context, store *FileStore, now func() time.Time, fire func( case <-ctx.Done(): return ctx.Err() case <-t.C: - for _, it := range store.Due(now()) { - fire(it) - } + sweep(store, now(), paused, fire) } } } +// sweep fires everything due, unless firing is impossible right now. +// +// The pause check comes BEFORE Due deliberately: Due is take-then-fire, removing +// what it returns, so sweeping into a fire that would drop the item deletes +// scheduled work permanently. +func sweep(store *FileStore, now time.Time, paused func() bool, fire func(Item)) { + if paused != nil && paused() { + return + } + for _, it := range store.Due(now) { + fire(it) + } +} + // ParseDelay parses a follow-up filename stem ("15m", "2h", "90s", "1d") into // a delay clamped to [MinDelay, MaxDelay]. ok=false for anything that does not // parse as a positive Go-style duration (with "d" accepted as 24h). diff --git a/core/followup/sweep_test.go b/core/followup/sweep_test.go new file mode 100644 index 0000000..bb52a42 --- /dev/null +++ b/core/followup/sweep_test.go @@ -0,0 +1,62 @@ +//nolint:testpackage // whitebox: the sweep is unexported, and it is the unit under test. +package followup + +import ( + "path/filepath" + "testing" + "time" +) + +// TestSweepSkipsTheStoreWhilePaused is the data-loss regression. Due is +// take-then-fire: it REMOVES what it returns. Sweeping while firing is impossible +// would therefore delete each due follow-up permanently — on an unauthorized +// deployment every matured followup/.md would vanish, and the sign-in +// would not bring it back. Pausing has to skip the sweep, not just the fire. +func TestSweepSkipsTheStoreWhilePaused(t *testing.T) { + store, err := Open(filepath.Join(t.TempDir(), "followups.json")) + if err != nil { + t.Fatalf("open store: %v", err) + } + if _, err := store.Add("1", "check the deploy", time.UnixMilli(1)); err != nil { + t.Fatalf("add item: %v", err) + } + now := time.UnixMilli(1000) + + var fired []Item + sweep(store, now, func() bool { return true }, func(it Item) { fired = append(fired, it) }) + + if len(fired) != 0 { + t.Errorf("fired %+v while paused, want nothing", fired) + } + if got := store.Count("1"); got != 1 { + t.Fatalf("the store holds %d items while paused, want the item kept for later", got) + } + + // Unpaused, the same item fires — it survived the pause. + sweep(store, now, func() bool { return false }, func(it Item) { fired = append(fired, it) }) + + if len(fired) != 1 || fired[0].Prompt != "check the deploy" { + t.Errorf("fired %+v after the pause cleared, want the item that waited", fired) + } + if got := store.Count("1"); got != 0 { + t.Errorf("the store still holds %d items after firing, want them taken", got) + } +} + +// TestSweepWithoutAPausePredicate: a nil predicate never pauses. +func TestSweepWithoutAPausePredicate(t *testing.T) { + store, err := Open(filepath.Join(t.TempDir(), "followups.json")) + if err != nil { + t.Fatalf("open store: %v", err) + } + if _, err := store.Add("1", "go", time.UnixMilli(1)); err != nil { + t.Fatalf("add item: %v", err) + } + + var fired []Item + sweep(store, time.UnixMilli(1000), nil, func(it Item) { fired = append(fired, it) }) + + if len(fired) != 1 { + t.Errorf("fired %+v with a nil predicate, want the due item", fired) + } +} diff --git a/core/schedule/manager.go b/core/schedule/manager.go index 97d2417..d6fd901 100644 --- a/core/schedule/manager.go +++ b/core/schedule/manager.go @@ -30,8 +30,11 @@ type Manager struct { store *Store fire func(chatID, prompt string, userID int64) bool allowed func(userID int64) bool - now func() time.Time - log *slog.Logger + // paused reports that firing is impossible right now, so the whole tick is + // skipped before any minute is recorded. Nil never pauses. + paused func() bool + now func() time.Time + log *slog.Logger } // NewManager builds a Manager. store is the durable job store; fire injects a @@ -41,13 +44,15 @@ type Manager struct { // cost cap, and TickOnce records the matching minute regardless (at-most-one-fire- // attempt per matching minute). allowed re-validates a job's creator against the // live allow-list at fire time, so a de-listed user's stored jobs stop running and -// are pruned; a nil allowed allows every creator (no fire-time check). now supplies +// are pruned; a nil allowed allows every creator (no fire-time check). paused +// skips an entire tick — see TickOnce — and a nil paused never pauses. now supplies // the current time (time.Now in production, a stub in tests); a nil log defaults to // slog.Default(). func NewManager( store *Store, fire func(chatID, prompt string, userID int64) bool, allowed func(userID int64) bool, + paused func() bool, now func() time.Time, log *slog.Logger, ) *Manager { @@ -57,7 +62,7 @@ func NewManager( if log == nil { log = slog.Default() } - return &Manager{store: store, fire: fire, allowed: allowed, now: now, log: log} + return &Manager{store: store, fire: fire, allowed: allowed, paused: paused, now: now, log: log} } // Run drives the scheduler until ctx is cancelled: every tick it reads the LIVE @@ -86,6 +91,16 @@ func (m *Manager) TickOnce() { if m.store == nil || m.fire == nil { return } + // Skipped WHOLE while firing is impossible (in production: the AI provider is + // unauthorized), before anything is marked. TickOnce records a matching minute + // regardless of whether the fire took, so ticking into a fire that must refuse + // would spend the job's occurrence on nothing: a nightly job would lose its + // night and never run it, and the log would blame a busy lane. Skipping leaves + // the minute unrecorded, so the job fires once the block clears. + if m.paused != nil && m.paused() { + m.log.Info("scheduler: paused; the provider is not authorized") + return + } now := m.now() locs := map[string]*time.Location{} // resolve each chat's zone once per tick for _, job := range m.store.ActiveJobs() { diff --git a/core/schedule/manager_test.go b/core/schedule/manager_test.go index 292fcf4..b352eef 100644 --- a/core/schedule/manager_test.go +++ b/core/schedule/manager_test.go @@ -45,7 +45,7 @@ func newManager(t *testing.T) (*schedule.Manager, *schedule.Store, *recordingFir rec := &recordingFire{} clock := time.Date(2026, 6, 18, 9, 0, 0, 0, time.UTC) nowPtr := &clock - mgr := schedule.NewManager(store, rec.fire, nil, func() time.Time { return *nowPtr }, nil) + mgr := schedule.NewManager(store, rec.fire, nil, nil, func() time.Time { return *nowPtr }, nil) return mgr, store, rec, nowPtr } @@ -57,7 +57,7 @@ func newDispatchManager(t *testing.T) (*schedule.Manager, *schedule.Store) { if err != nil { t.Fatalf("Open: %v", err) } - return schedule.NewManager(store, func(string, string, int64) bool { return true }, nil, nil, nil), store + return schedule.NewManager(store, func(string, string, int64) bool { return true }, nil, nil, nil, nil), store } // --- Dispatch tests --- @@ -433,7 +433,7 @@ func TestSchedulerPrunesJobOfDelistedCreator(t *testing.T) { // allowed denies the creator (id 42) of the job below. allowed := func(userID int64) bool { return userID != 42 } clock := time.Date(2026, 6, 18, 9, 0, 0, 0, time.UTC) - mgr := schedule.NewManager(store, rec.fire, allowed, func() time.Time { return clock }, nil) + mgr := schedule.NewManager(store, rec.fire, allowed, nil, func() time.Time { return clock }, nil) // A job whose schedule matches right now (every minute) but whose creator is // de-listed. @@ -470,7 +470,7 @@ func TestSchedulerSkipsBusyFireButMarksMinute(t *testing.T) { } clock := time.Date(2026, 6, 18, 9, 0, 0, 0, time.UTC) now := clock - mgr := schedule.NewManager(store, fire, nil, func() time.Time { return now }, nil) + mgr := schedule.NewManager(store, fire, nil, nil, func() time.Time { return now }, nil) if _, err := store.Add(schedule.Job{ ChatID: "100", Name: "j", Cron: "0 * * * *", Prompt: "go", CreatedBy: 7, Active: true, @@ -512,3 +512,38 @@ func TestRunStopsOnContextCancel(t *testing.T) { t.Fatal("Run did not return within 2s of cancel") } } + +// TestTickOnceSkippedWhilePaused is the lost-occurrence regression. TickOnce +// records a matching minute whether or not the fire took, so ticking into a fire +// that must refuse — the AI provider is unauthorized — would spend the job's +// occurrence on nothing: a nightly job would lose its night and never run it, +// while the log blamed a busy lane. Pausing has to skip the tick. +func TestTickOnceSkippedWhilePaused(t *testing.T) { + store := openStore(t) + if _, err := store.Add(schedule.Job{ + ChatID: "1", Name: "nightly", Cron: "* * * * *", Prompt: "run the nightly", CreatedBy: 7, Active: true, + }); err != nil { + t.Fatalf("add job: %v", err) + } + at := time.Date(2026, 7, 29, 9, 0, 0, 0, time.UTC) + + var fired int + paused := true + mgr := schedule.NewManager(store, func(string, string, int64) bool { + fired++ + return true + }, nil, func() bool { return paused }, func() time.Time { return at }, nil) + + mgr.TickOnce() + if fired != 0 { + t.Fatalf("fired %d times while paused, want 0", fired) + } + + // The minute must NOT have been recorded: the same occurrence still fires once + // the pause clears. + paused = false + mgr.TickOnce() + if fired != 1 { + t.Errorf("fired %d times after the pause cleared, want 1 — the occurrence was consumed", fired) + } +} diff --git a/docs/codex-integration-plan.md b/docs/codex-integration-plan.md index 9cee53e..a1c7201 100644 --- a/docs/codex-integration-plan.md +++ b/docs/codex-integration-plan.md @@ -145,21 +145,25 @@ Accepted credential sources: printf '%s' "$CODEX_ACCESS_TOKEN" | codex login --with-access-token ``` -3. Manual one-time setup with device auth: +3. In-chat device login: an allow-listed user sends `/login` and the bot relays + the verification link and one-time code (`core/codexauth`). This is the + primary path — it needs no shell on the host. +4. Manual one-time setup with device auth, for operators who do have a shell: ```bash docker exec -it codex login --device-auth ``` -4. Manual copy of a local `auth.json` into the persistent `CODEX_HOME` volume. +5. Manual copy of a local `auth.json` into the persistent `CODEX_HOME` volume. Validation rules: - If `CODEX_AUTH_MODE=subscription` and `CODEX_API_KEY` is non-empty, startup must fail with a clear error. This prevents accidental API billing. - If no persisted auth is present and no `CODEX_ACCESS_TOKEN` is supplied, the - bot may start only when `CODEX_REQUIRE_AUTH=false`; otherwise startup fails - with instructions to run `codex login --device-auth`. + bot starts in a "needs login" state rather than failing: runs are blocked with + a notice pointing at `/login`, which is the command that clears the state. + Failing startup here would be a deadlock — the login lives inside the bot. - `CODEX_ACCESS_TOKEN` must be treated as a secret and must never be copied into the per-chat workspace. @@ -496,22 +500,41 @@ Acceptance: current Claude template. - Claude rendering remains byte-compatible where practical. -### Stage 5 - Subscription Auth UX - -Goal: make subscription setup clear and observable for operators. - -Scope: - -- Add startup checks for `CODEX_HOME/auth.json` when subscription mode is - required. -- Add a clear error message with `codex login --device-auth` instructions. -- Optionally add a health/log command that reports auth mode without exposing - secrets. +### Stage 5 - Subscription Auth UX (done) + +Goal: make subscription setup completable and observable WITHOUT host shell +access. + +Delivered: + +- `core/codexauth` drives `codex login --device-auth`, parsing the verification + URL and one-time code off the live (ANSI-colored) output while the CLI is still + polling, and relaying them into the chat. +- `/login` (reserved command, both adapters) starts, re-shows, reports, cancels, + or forces the sign-in. +- Startup no longer fails when only the interactive login is missing + (`airunner.BuildWithPendingLogin`); runs are blocked with an actionable notice + until the login lands, re-checked from `auth.json` on every message so no + restart is needed. +- The block lives in `core/chat` (`RunGate`), at the single point every run + passes through, so the four non-adapter sources — a poller relay, a cron fire, + a workspace follow-up and the restart replay — are gated too instead of + marching into an unauthenticated CLI once per tick. The adapters keep an early + check on the message path so an unauthorized deploy never pays for a voice + transcription or an upload download. A blocked replay keeps its pending marker. +- Where a one-time code may be printed is decided by `codexauth` from the + destination, not by the adapters inspecting the arguments: `/login status` also + re-shows a pending code, so an argument-based guard misses it. A non-private + destination gets a redirect instead of the code, is refused when it tries to + START a sign-in, and is never subscribed to the prompt broadcast. +- `CODEX_REQUIRE_AUTH=false` still opts out of blocking entirely, and startup + logs `authorized` next to `credentials_present` so that opt-out cannot read as + "all good". Acceptance: -- Missing subscription auth fails with actionable instructions. -- Present `auth.json` allows startup. +- Missing subscription auth starts the bot and blocks runs with instructions. +- Present `auth.json` allows runs immediately. - `CODEX_ACCESS_TOKEN` path logs only that access-token auth is configured, never the token. @@ -553,16 +576,45 @@ Acceptance: ### Subscription Setup 1. Deploy with `AI_BACKEND=codex` and `CODEX_AUTH_MODE=subscription`. -2. Start the container once. -3. Run: - - ```bash - docker exec -it codex login --device-auth - ``` - -4. Complete browser/device login. -5. Restart the bot. -6. Send a small prompt in a private allowed chat. +2. Start the container. With no persisted `auth.json` and no `CODEX_ACCESS_TOKEN` + the bot comes up in a "needs login" state: it logs a warning, serves `/login`, + and answers any other message with a notice pointing at it. +3. From an allow-listed **direct message** (not a group — the reply carries a + one-time code that would authorize whoever acts on it first), send `/login`. + The bot replies with a verification link and a one-time code. +4. Open the link, sign in, and enter the code. The bot confirms in the chat. +5. Send a small prompt in a private allowed chat — no restart needed. + +`/login status` reports the auth state, `/login cancel` aborts a pending sign-in, +and `/login force` re-authenticates an already-authorized deployment. + +Two properties an operator should know before running it, because the `.env` +files point here rather than repeating them: + +- **Starting a sign-in only works in a direct message.** The reply carries a + one-time code that authorizes an account for the entire bot, so in a group + every member — allow-listed or not — could read it, and whoever acts on it + first binds their account. `/login status` works anywhere but withholds the + code; `/login cancel` is direct-message only too, since it controls a sign-in + someone else may be completing. +- **Any allow-listed user can finish a sign-in someone else started.** The code is + re-shown in any direct message, and the account that completes it becomes the + deployment's single Codex identity — one Codex account serves the whole + deployment. Restrict `ALLOWED_USERS` accordingly. + +With `CODEX_REQUIRE_AUTH=false` nothing is blocked: the bot runs immediately and +an unfinished sign-in surfaces as a Codex CLI failure instead. + +Why device code and not the shell recipe this plan originally carried +(`docker exec -it codex login --device-auth`): that still works, +but it needs shell access to the host, and the plain `codex login` an operator +reaches for first cannot work in a container at all. Plain `codex login` starts a +loopback OAuth server on `http://localhost:1455` and prints an authorize URL whose +`redirect_uri` points back at that loopback — opened on a laptop, the callback +lands on the laptop's own localhost, so the CLI waits forever for a callback that +never arrives, having printed no code. `codex login --device-auth` instead prints +a verification URL plus a one-time code and polls, which is what `core/codexauth` +drives and relays into the chat. ### Billing Setup diff --git a/internal/airunner/backend.go b/internal/airunner/backend.go index 1460ac6..89b6182 100644 --- a/internal/airunner/backend.go +++ b/internal/airunner/backend.go @@ -4,12 +4,14 @@ package airunner import ( "errors" "fmt" + "log/slog" "os" "strings" "github.com/duckbugio/flock/core/agent" "github.com/duckbugio/flock/core/claude" "github.com/duckbugio/flock/core/codex" + "github.com/duckbugio/flock/core/codexauth" "github.com/duckbugio/flock/core/openaicompat" "github.com/duckbugio/flock/internal/config" ) @@ -84,6 +86,115 @@ func (r Registry) Build(cfg config.Config) (agent.Runner, agent.Options, agent.P }, nil } +// BuildWithPendingLogin is Build, except that the one provider error an END USER +// can clear from chat — Codex subscription mode with no persisted login yet — is +// not fatal. It rebuilds with the auth-presence check relaxed and reports +// pendingLogin=true. +// +// Without this the deployment deadlocks: startup refuses to run until someone has +// logged in, and the /login flow that performs the login only exists inside the +// running bot. Starting in a "needs login" state breaks the cycle — the bot comes +// up, serves /login, and blocks runs (see codexauth.Manager.BlockedNotice) until +// the login lands. Every other validation error is still fatal, so a misconfigured +// deploy (an API key in subscription mode, billing without acknowledgement) fails +// exactly as loudly as before. +func BuildWithPendingLogin(cfg config.Config) ( + agent.Runner, agent.Options, agent.ProviderInfo, bool, error, +) { + runner, opts, info, err := Build(cfg) + if err == nil || !IsPendingLogin(err) { + return runner, opts, info, false, err + } + // An empty CODEX_HOME cannot be fixed by signing in: there is nowhere for the + // credential to be written OR read, so the deployment would boot permanently + // blocked and a "successful" /login would report saving nothing, to a blank + // path. It used to fail at startup, and it still should. + if strings.TrimSpace(cfg.CodexHome) == "" { + return nil, agent.Options{}, agent.ProviderInfo{}, false, err + } + relaxed := cfg + relaxed.CodexRequireAuth = false + runner, opts, info, err = Build(relaxed) + if err != nil { + return nil, agent.Options{}, agent.ProviderInfo{}, false, err + } + return runner, opts, info, true, nil +} + +// IsPendingLogin reports whether err merely means "nobody has completed the +// interactive Codex login yet", as opposed to a real misconfiguration. +func IsPendingLogin(err error) bool { + return errors.Is(err, config.ErrCodexSubscriptionAuthRequired) +} + +// BuildProvider is the whole provider-startup sequence both adapter binaries +// run: resolve the provider (tolerating a pending Codex sign-in), build the +// codexauth.Manager that serves /login and gates runs, and log what came up. It +// lives here rather than in either main so the two cannot drift apart. +// +// The Codex log reports authorized AND credentials_present, because they are +// different questions and only reporting the first would mislead: with +// CODEX_REQUIRE_AUTH=false, Authorized is true by policy even when there is no +// credential at all — precisely the deploy whose first run is about to fail +// inside the CLI, and precisely the log an operator reads first. +// +// The error is logged here; the caller only needs to know to exit. +func BuildProvider(cfg config.Config, logger *slog.Logger) ( + agent.Runner, agent.Options, agent.ProviderInfo, *codexauth.Manager, error, +) { + if logger == nil { + logger = slog.Default() + } + runner, opts, provider, pendingLogin, err := BuildWithPendingLogin(cfg) + if err != nil { + logger.Error("invalid ai provider config", "provider", cfg.AIBackend, "error", err) + return nil, agent.Options{}, agent.ProviderInfo{}, nil, err + } + auth := codexauth.NewManager(CodexAuthConfig(cfg, provider)) + if pendingLogin { + logger.Warn("codex is not authorized yet; runs are blocked until /login completes", + "codex_home", cfg.CodexHome) + } + if provider.Name == config.AIBackendCodex { + logger.Info("codex backend enabled", + "provider", provider.Name, + "display_name", provider.DisplayName, + "auth_mode", cfg.CodexAuthModeName(), + "authorized", auth.Authorized(), + "credentials_present", auth.CredentialsPresent(), + "require_auth", cfg.CodexRequireAuth, + "sandbox", cfg.CodexSandbox, + "approval_policy", cfg.CodexApprovalPolicy, + "codex_home", cfg.CodexHome, + "capabilities", provider.Capabilities, + ) + } else { + logger.Info("ai provider enabled", + "provider", provider.Name, + "display_name", provider.DisplayName, + "model", opts.Model, + "capabilities", provider.Capabilities, + ) + } + return runner, opts, provider, auth, nil +} + +// CodexAuthConfig derives the /login driver's configuration from the deployment +// config and the resolved provider. It passes the SAME child environment the +// runner uses (CodexEnv), so the login writes auth.json into exactly the +// CODEX_HOME later runs read. +func CodexAuthConfig(cfg config.Config, info agent.ProviderInfo) codexauth.Config { + return codexauth.Config{ + Backend: info.Name, + AuthMode: cfg.CodexAuthModeName(), + Bin: cfg.CodexBin, + Home: cfg.CodexHome, + Env: CodexEnv(cfg), + HasAccessToken: strings.TrimSpace(cfg.CodexAccessToken) != "", + RequireAuth: cfg.CodexRequireAuth, + } +} + // Provider resolves a configured provider name or alias. func (r Registry) Provider(name string) (Provider, error) { normalized := normalizeProviderName(name) diff --git a/internal/airunner/backend_test.go b/internal/airunner/backend_test.go index e501a99..3efc0c3 100644 --- a/internal/airunner/backend_test.go +++ b/internal/airunner/backend_test.go @@ -3,8 +3,12 @@ package airunner import ( "errors" + "os" + "path/filepath" + "strings" "testing" + "github.com/duckbugio/flock/core/agent" "github.com/duckbugio/flock/internal/config" ) @@ -136,3 +140,153 @@ func TestBuildOpenAICompatibleProvider(t *testing.T) { t.Fatalf("opts.Model = %q, want qwen-plus", opts.Model) } } + +// TestBuildWithPendingLoginStartsUnauthorized: a Codex subscription deploy that +// has never been signed in must still BOOT, flagged pendingLogin, because the +// /login command that performs the sign-in only exists inside the running bot. +// Failing startup here is what made headless Codex setup impossible. +func TestBuildWithPendingLoginStartsUnauthorized(t *testing.T) { + cfg := config.Config{ + AIBackend: config.AIBackendCodex, + CodexBin: "codex", + CodexAuthMode: config.CodexAuthSubscription, + CodexRequireAuth: true, + CodexHome: t.TempDir(), // no auth.json inside + } + if _, _, _, err := Build(cfg); !errors.Is(err, config.ErrCodexSubscriptionAuthRequired) { + t.Fatalf("Build() error = %v, want ErrCodexSubscriptionAuthRequired", err) + } + + runner, _, info, pendingLogin, err := BuildWithPendingLogin(cfg) + if err != nil { + t.Fatalf("BuildWithPendingLogin() error = %v, want nil", err) + } + if !pendingLogin { + t.Error("pendingLogin = false, want true for a never-signed-in deploy") + } + if runner == nil { + t.Error("runner = nil; the bot must come up to serve /login") + } + if info.Name != config.AIBackendCodex { + t.Errorf("provider = %q, want codex", info.Name) + } +} + +// TestBuildWithPendingLoginKeepsRealErrorsFatal: only the "nobody has logged in +// yet" case is recoverable from chat. A misconfiguration — an API key in +// subscription mode, billing without acknowledgement — must still refuse to boot. +func TestBuildWithPendingLoginKeepsRealErrorsFatal(t *testing.T) { + tests := []struct { + name string + cfg config.Config + want error + }{ + { + name: "api key in subscription mode", + cfg: config.Config{ + AIBackend: config.AIBackendCodex, CodexAuthMode: config.CodexAuthSubscription, + CodexAPIKey: "sk-live", CodexRequireAuth: true, + }, + want: config.ErrCodexSubscriptionWithAPIKey, + }, + { + name: "billing without acknowledgement", + cfg: config.Config{ + AIBackend: config.AIBackendCodex, CodexAuthMode: config.CodexAuthBilling, + CodexAPIKey: "sk-live", CodexBillingAck: false, + }, + want: config.ErrCodexBillingAckRequired, + }, + { + name: "unknown auth mode", + cfg: config.Config{ + AIBackend: config.AIBackendCodex, CodexAuthMode: "whatever", + }, + want: config.ErrCodexUnknownAuthMode, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, _, pendingLogin, err := BuildWithPendingLogin(tt.cfg) + if !errors.Is(err, tt.want) { + t.Fatalf("error = %v, want %v", err, tt.want) + } + if pendingLogin { + t.Error("pendingLogin = true for a misconfiguration") + } + }) + } +} + +// TestBuildWithPendingLoginAuthorizedDeploy: with a persisted login nothing is +// pending and the build is the plain one. +func TestBuildWithPendingLoginAuthorizedDeploy(t *testing.T) { + home := t.TempDir() + if err := os.WriteFile(filepath.Join(home, "auth.json"), []byte("{}"), 0o600); err != nil { + t.Fatalf("write auth.json: %v", err) + } + runner, _, _, pendingLogin, err := BuildWithPendingLogin(config.Config{ + AIBackend: config.AIBackendCodex, CodexAuthMode: config.CodexAuthSubscription, + CodexRequireAuth: true, CodexHome: home, + }) + if err != nil { + t.Fatalf("BuildWithPendingLogin() error = %v", err) + } + if pendingLogin { + t.Error("pendingLogin = true for an authorized deploy") + } + if runner == nil { + t.Error("runner = nil for an authorized deploy") + } +} + +// TestCodexAuthConfigMirrorsTheRunner: the login must run with the SAME child +// environment and CODEX_HOME the runner uses, or it would persist auth.json +// where later runs never look. Subscription mode also keeps CODEX_API_KEY out. +func TestCodexAuthConfigMirrorsTheRunner(t *testing.T) { + t.Setenv("CODEX_API_KEY", "sk-should-not-leak") + home := t.TempDir() + cfg := config.Config{ + AIBackend: config.AIBackendCodex, CodexAuthMode: config.CodexAuthSubscription, + CodexBin: "/usr/local/bin/codex", CodexHome: home, CodexAccessToken: "tok", + } + got := CodexAuthConfig(cfg, agent.ProviderInfo{Name: config.AIBackendCodex}) + + if got.Backend != config.AIBackendCodex || got.AuthMode != config.CodexAuthSubscription { + t.Errorf("backend/auth mode = %q/%q, want codex/subscription", got.Backend, got.AuthMode) + } + if got.Bin != "/usr/local/bin/codex" || got.Home != home { + t.Errorf("bin/home = %q/%q, want the configured pair", got.Bin, got.Home) + } + if !got.HasAccessToken { + t.Error("HasAccessToken = false with CODEX_ACCESS_TOKEN set") + } + for _, kv := range got.Env { + if strings.HasPrefix(kv, "CODEX_API_KEY=") { + t.Fatal("CODEX_API_KEY leaked into the subscription-mode login environment") + } + } +} + +// TestEmptyCodexHomeStaysFatal: a blank CODEX_HOME is not "nobody has signed in +// yet", it is a misconfiguration no sign-in can fix — there is nowhere to write +// the credential and nowhere to read it back. Relaxing it would boot a deployment +// that is blocked forever, whose "successful" /login reports saving nothing to a +// blank path. +func TestEmptyCodexHomeStaysFatal(t *testing.T) { + // CodexHome is left at its zero value on purpose: that is the misconfiguration. + runner, _, _, pendingLogin, err := BuildWithPendingLogin(config.Config{ + AIBackend: config.AIBackendCodex, + CodexAuthMode: config.CodexAuthSubscription, + CodexRequireAuth: true, + }) + if !errors.Is(err, config.ErrCodexSubscriptionAuthRequired) { + t.Fatalf("error = %v, want the auth-required error to stay fatal", err) + } + if pendingLogin { + t.Error("pendingLogin = true for a deployment with nowhere to store a credential") + } + if runner != nil { + t.Error("a runner was built for a deployment that cannot ever authenticate") + } +} diff --git a/internal/autonomy/autonomy.go b/internal/autonomy/autonomy.go index 4a39cf3..161cca8 100644 --- a/internal/autonomy/autonomy.go +++ b/internal/autonomy/autonomy.go @@ -74,7 +74,11 @@ func StartFollowups(ctx context.Context, svc *chat.Service, pr chat.PostRunConfi return } go func() { - if err := followup.Run(ctx, pr.Followups, nil, func(it followup.Item) { + // svc.RunsBlocked pauses the sweep entirely while the provider is + // unauthorized. Without it the sweep would keep TAKING due items from the + // store and handing them to an InjectAuto that refuses them — scheduled work + // deleted for good, and not replayed after the sign-in. + if err := followup.Run(ctx, pr.Followups, nil, svc.RunsBlocked, func(it followup.Item) { svc.InjectAuto(ctx, it.ChatID, chat.FollowupPrompt(it)) }); err != nil && ctx.Err() == nil { logger.Error("follow-up loop stopped", "error", err)