diff --git a/pyproject.toml b/pyproject.toml index 478b6e14..84a7a5bc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,7 +32,7 @@ dependencies = [ "httpx>=0.28", "fastapi>=0.135", "uvicorn>=0.44", - "claude-agent-sdk>=0.1.72", + "claude-agent-sdk>=0.1.77", "Pillow>=10.0", # Federation crypto (sealed_box_v1): X25519, Ed25519, HKDF, XChaCha20-Poly1305. "cryptography>=41.0", @@ -54,7 +54,7 @@ calendar = [ "cryptography>=41.0", ] web = ["camoufox>=0.4", "markdownify>=1.0", "beautifulsoup4>=4.12"] -voice = ["twilio>=9.0", "anthropic>=0.96"] +voice = ["twilio>=9.0", "anthropic>=0.98"] local-embeddings = ["sentence-transformers>=2.0"] all = [ "pinky-ai[telegram,discord,slack,google,calendar,voice]", diff --git a/specs/opencode-session-integration.md b/specs/opencode-session-integration.md new file mode 100644 index 00000000..9031427c --- /dev/null +++ b/specs/opencode-session-integration.md @@ -0,0 +1,659 @@ +# OpencodeSession Integration Design + +Status: draft spec for review +Scope: design only, no implementation in this branch +Primary goal: add `opencode` as a third PinkyBot agent runtime alongside Claude SDK `StreamingSession` and Codex CLI `CodexSession` + +## Context + +PinkyBot currently has two live agent runtimes: + +- `StreamingSession` in `src/pinky_daemon/streaming_session.py`, backed by the Claude Agent SDK. +- `CodexSession` in `src/pinky_daemon/codex_session.py`, backed by `codex exec --json`. + +Both are registered in the broker through `broker.register_streaming(agent_name, session, label=...)` and are expected to expose the same public surface: + +- `connect()` +- `send(prompt, platform="", chat_id="", message_id="", agent_hint="")` +- `disconnect()` +- `force_restart()` +- `idle_sleep()` +- `attempt_reconnect()` where practical +- `is_connected` +- `is_idle_sleeping` or equivalent +- `session_id` +- `id` +- `stats` +- `get_context_info()` + +The current runtime selection is overloaded onto provider config: `_start_streaming_session()` uses `provider_url == "codex_cli"` to select `CodexSession`, otherwise it uses `StreamingSession`. That was workable for one alternate runtime, but `opencode` needs provider selection inside the runtime, so continuing to encode runtime choice in `provider_url` would make configuration ambiguous. + +This spec uses the opencode architecture facts supplied by Barsik, plus a +verification pass against the official server/config docs and a short local +`opencode serve` smoke run. + +Verification notes: + +- `bunx opencode-ai --version` resolved v1.14.32 locally. +- `opencode serve` on `127.0.0.1:4196` with `OPENCODE_SERVER_PASSWORD` returned `/global/health` as `{"healthy":true,"version":"1.14.32"}`. +- `/global/event` emitted `server.connected` SSE events. +- The official server docs say `POST /session/:id/message` waits for completion and returns the response, while `POST /session/:id/message_async` queues a prompt and does not wait for completion. +- The generated SDK/OpenAPI types include `cost` and `tokens` on assistant message and step-finish shapes. +- The config docs support secret substitution from env/file values, and provider config supports `options.apiKey`. + +## Recommendation + +Add `OpencodeSession` as a polymorphic runtime backed by a shared `opencode serve` process. Add an explicit agent `runtime` field with values: + +- `claude_sdk` +- `codex_cli` +- `opencode` + +Keep provider/model selection separate from runtime selection. For opencode agents, PinkyBot stores the selected opencode model string, such as `deepseek/deepseek-v4-pro`, in existing model/provider fields or a normalized provider config, then renders that into generated opencode config. + +Do not auto-migrate existing agents. Opencode should coexist until it proves stable under real workloads. + +## 1. `OpencodeSession` Class Shape + +Create: + +```text +src/pinky_daemon/opencode_session.py +``` + +The class should mirror `StreamingSession` and `CodexSession` closely enough that `MessageBroker`, scheduler, status endpoints, restart endpoints, and web chat keep treating sessions polymorphically. + +Constructor: + +```python +class OpencodeSession: + def __init__( + self, + config: StreamingSessionConfig, + *, + response_callback=None, + conversation_store=None, + cost_callback=None, + stream_event_callback=None, + analytics_store=None, + registry=None, + server_manager=None, + ) -> None: + ... +``` + +Public fields/properties: + +- `agent_name` +- `session_id`: opencode session id persisted via the existing `_on_session_id` callback. +- `created_at` +- `last_active` +- `usage: SessionUsage` +- `account_info = {"apiProvider": "opencode"}` +- `is_connected` +- `is_idle_sleeping` +- `id`: keep `f"{agent_name}-{label}"`, matching existing session ids. +- `stats`: include `connected`, `idle_sleeping`, `processing`, `pending_messages`, `current_activity`, `activity_log`, `cost_usd`, `account`, `thinking_effort`. +- `get_context_info()`: best effort initially, same estimated-token fallback as `CodexSession` unless opencode exposes richer usage. + +Public methods: + +- `connect()`: ensure the shared opencode server is healthy, create or resume the opencode session, start queue worker, attach to the global event stream through the manager, then enqueue the wake prompt. +- `send(prompt, platform="", chat_id="", message_id="", agent_hint="")`: same semantics as `CodexSession.send()`. It must accept `agent_hint`, append it only to the runtime prompt, and store the raw prompt in conversation history. +- `disconnect()`: stop local worker/subscriptions. It should not stop the shared opencode server unless this is daemon shutdown and the manager owns the process. +- `attempt_reconnect()`: reconnect to the server and event stream with the existing bounded backoff shape from `StreamingSession`. +- `force_restart()`: apply the restart guard, clear `session_id`, clear persisted session id via `_on_session_id`, create a fresh opencode session, and enqueue wake context. +- `idle_sleep()`: ask the agent to persist state, disconnect this session, preserve `session_id`, and mark idle sleeping. + +Internal structure: + +- `_message_queue: asyncio.Queue[tuple[str, str, str, str]]` +- `_worker_task` +- `_event_subscription` +- `_processing` +- `_pending_chats` or a per-turn routing map if opencode can run overlapping turns. +- `context_estimator`, using a shared helper rather than copying `CodexSession`'s private `_internal_context_texts` implementation. + +The MVP should process prompts sequentially per PinkyBot session. That matches `CodexSession` and avoids ordering bugs while opencode event semantics are still new. + +Add a shared context helper before or with `OpencodeSession`: + +```python +class ContextTextEstimator: + def record_internal_text(self, text: str) -> None: ... + def estimated_tokens(self, session_id: str, conversation_store) -> int: ... + def context_info(self, session_id: str, conversation_store, max_tokens: int) -> dict: ... +``` + +Then refactor `CodexSession` to use the helper instead of maintaining its own +private implementation. `OpencodeSession` should not become a third copy of the +same estimation code. + +Send semantics are now resolved by docs: use `POST /session/:id/message` for +the sequential worker path because it waits for completion and returns the +response. Use `/global/event` for incremental UI activity and tool/status +streaming. Do not use `/message_async` for the MVP broker path unless we later +need fully detached turns. + +## 2. Process Lifecycle + +Recommendation: shared server manager, not one process per agent. + +However, make it a server pool keyed by opencode runtime root, not a single hard-coded global forever: + +```text +OpencodeServerManager + key: (working_dir/config_root, port/password) + value: running opencode serve process + HTTP client + SSE dispatcher +``` + +For the current PinkyBot deployment, this will normally be one `opencode serve` process shared by all opencode agents, with one opencode `session_id` per PinkyBot streaming session label. + +Why not one process per agent: + +- More memory and process churn. +- More port management. +- Harder daemon startup/shutdown. +- Duplicates opencode's own session persistence. + +Why not one unconditional global: + +- PinkyBot agents can have different `working_dir` values. +- opencode config and file access are likely scoped to the directory where `opencode serve` starts. +- A single global process could accidentally collapse separate project boundaries. + +MVP lifecycle: + +1. `create_api()` initializes `OpencodeServerManager`. +2. On first opencode agent start, manager checks `/global/health`. +3. If no server is reachable and PinkyBot is configured to own it, manager spawns: + + ```text + opencode serve --hostname 127.0.0.1 --port + ``` + +4. Manager waits for `/global/health`. +5. Manager lazily regenerates generated opencode config if the agent/config + fingerprint is dirty. +6. Sessions call `POST /session` or reuse a persisted session id. +7. On daemon shutdown, manager terminates owned processes. + +Operational mode should be configurable: + +- `managed`: PinkyBot starts/stops opencode. Default for local installs. +- `external`: PinkyBot only connects to an existing opencode server. + +Generated config rewrite policy: + +- Store a fingerprint alongside each generated config containing agent runtime, + model, prompt hash, permission mode, materialized MCP config hash, provider + secret refs, and opencode manager version. +- Rewrite lazily on next session start when the fingerprint differs. +- Force-regenerate before `force_restart()`. +- Mark configs dirty on registry writes that affect runtime, model, soul, + boundaries, directives, permission mode, working directory, provider refs, or + skill/materialized MCP state. +- Directive updates should mark dirty through the directive write path, even + though the rewrite still happens lazily unless the agent is explicitly + restarted. +- Permission changes are security-sensitive: for live opencode sessions, + `permission_mode` writes should force-regenerate and require an immediate + session restart to guarantee the new edit/bash policy applies. If the restart + guard blocks, mark the session `restart_required` and surface a warning rather + than silently continuing under stale permissions. + +## 3. Auth Model + +Use both local bind and auth. + +Default: + +- Bind opencode to `127.0.0.1`. +- In managed mode, generate a new random password on every PinkyBot daemon + restart. Do not persist it to disk or DB. +- Pass it to the child process via `OPENCODE_SERVER_PASSWORD`. +- Store it only in memory on `OpencodeServerManager`. +- Use HTTP Basic auth for all REST and SSE requests. +- Expose the current generated password through an auth-gated PinkyBot daemon + diagnostics endpoint for admin/debug curl use cases. It should follow the + same auth posture as existing local admin/status endpoints such as + `/agents/{name}/streaming/status`, and should never be available without + PinkyBot admin auth. + +Do not run unauthenticated by default. Local-only without auth is convenient but brittle: localhost is still reachable by other local processes, browser extensions, and misconfigured reverse proxies. The overhead of random Basic auth is small and avoids a class of avoidable local privilege mistakes. + +For external mode: + +- Read password from a PinkyBot setting or environment variable, such as `PINKY_OPENCODE_SERVER_PASSWORD`. +- Never persist generated passwords to the agent DB. + +Decision trail: Brad chose regenerate-on-restart for managed mode on 2026-05-02. + +## 4. Streaming Protocol Mapping + +opencode exposes `/global/event` as SSE. PinkyBot already exposes its own UI SSE endpoint at: + +```text +GET /agents/{agent_name}/streaming/events +``` + +The mapping should be: + +```text +opencode /global/event + -> OpencodeServerManager single SSE reader + -> dispatch by opencode session_id + -> OpencodeSession._handle_event() + -> stream_event_callback() + -> PinkyBot /agents/{name}/streaming/events subscribers +``` + +The manager should own one SSE connection per server process, not one SSE connection per PinkyBot agent. It should parse events, filter/route by `session_id`, and send heartbeat/liveness updates to registered `OpencodeSession` objects. + +Event categories to normalize: + +- assistant text delta or completed assistant text -> `assistant_delta` +- tool started/completed -> `tool_use` +- command/file activity -> current activity/status labels +- usage/cost if available -> `StreamingTurnResult.model_usage` +- errors -> `turn_error` or `turn_failed` +- session created/resumed -> `_on_session_id(agent_name, session_id)` + +For broker responses, `OpencodeSession` should produce the same final `StreamingTurnResult` used by the current response callback. If opencode's `POST /session/:id/message` returns the completed assistant message, use that as the authoritative final response and use SSE for incremental UI only. If the message endpoint is fire-and-stream, accumulate final text from events and resolve the queued turn on the matching completion event. + +Heartbeat/resurrection fit: + +- PR #339-style resurrection still fits if `OpencodeSession.is_connected` reflects both HTTP health and SSE reader health. +- `attempt_reconnect()` should reconnect the HTTP client/SSE subscription, then verify `/global/health`. +- The watchdog must not restart a deliberately idle-sleeping session, same as `StreamingSession.is_idle_sleeping`. +- If the server process is down and PinkyBot owns it, `attempt_reconnect()` should ask `OpencodeServerManager` to restart the server before reattaching sessions. + +Important edge case: + +- A healthy shared server can coexist with one wedged opencode session. Recovery should first recreate the affected opencode session, not restart the whole server. Restarting the server should be reserved for failed `/global/health` or broken global SSE. + +## 5. Agent Registry Changes + +Recommendation: add an explicit `runtime` column to `agents`. + +Schema: + +```sql +ALTER TABLE agents ADD COLUMN runtime TEXT NOT NULL DEFAULT 'claude_sdk'; +``` + +Allowed values: + +- `claude_sdk` +- `codex_cli` +- `opencode` + +Migration behavior: + +- Existing rows default to `claude_sdk`. +- Run a one-shot boot-time data backfill: if an existing row has + `provider_url = 'codex_cli'`, set `runtime = 'codex_cli'`. +- After that backfill, runtime selection reads only `agents.runtime`. +- Leave `provider_url` stored for compatibility/debugging during the rollout, + but do not continue using it as a runtime selector. +- Remove any legacy `provider_url == 'codex_cli'` runtime shim after the rollout + window, target two weeks after deployment if no rollback is active. + +Update: + +- `Agent` dataclass +- `Agent.to_dict()` +- `_AGENT_COLUMNS` +- `AgentRegistry.register()` +- API create/update payloads +- frontend settings if the UI manages runtime/provider selection + +After the one-shot backfill has run, runtime selection in +`_start_streaming_session()` becomes: + +```python +runtime = agent.runtime or "claude_sdk" +if runtime == "codex_cli": + SessionClass = CodexSession +elif runtime == "opencode": + SessionClass = OpencodeSession +else: + SessionClass = StreamingSession +``` + +During the short rollout window only, a compatibility shim can protect DBs that +have not yet run the one-shot backfill: + +```python +def runtime_from_legacy_provider(agent): + if agent.provider_url == "codex_cli": + return "codex_cli" + return "claude_sdk" +``` + +Do not introduce opencode as another sentinel `provider_url` value. +`provider_url` should mean provider endpoint, not runtime. + +Rollback path: + +- UI: expose a runtime selector that can switch an agent from `opencode` back to + `claude_sdk`, then restart the streaming session. +- API/SQL fallback: + + ```sql + UPDATE agents SET runtime='claude_sdk', updated_at=? WHERE name=?; + ``` + +- On rollback, disconnect the live opencode session, leave the opencode + `session_id` persisted only for audit/debugging if needed, and start a fresh + Claude SDK session with normal wake context. + +## 6. Config Injection + +Use PinkyBot's existing system prompt builder as the source of truth. + +For an opencode agent, generate an opencode config block from PinkyBot agent state: + +```json +{ + "agent": { + "barsik": { + "description": "Barsik - Personal AI Sidekick", + "mode": "primary", + "model": "deepseek/deepseek-v4-pro", + "prompt": "", + "permission": { + "edit": "allow", + "bash": "allow" + } + } + } +} +``` + +Mapping: + +- `soul`, boundaries, users, active directives, skill catalog, and generated messaging instructions still flow through `agents.build_system_prompt(agent_name, skill_store=skills)`. +- `wake_context` is not baked into opencode config. It should be sent as the first prompt on `connect()`, matching `StreamingSession` and `CodexSession`. +- Permission mapping should be conservative: + - Pinky `bypassPermissions` or `auto` -> opencode `allow` for edit/bash. + - Pinky `default`, `acceptEdits`, or unknown -> opencode `ask` initially. + - Pinky `plan` -> opencode `deny` for edit/bash. +- Runtime permission changes require config regeneration and session restart, as + described in the lifecycle section. Do not assume an in-place config reload is + sufficient for active opencode turns until verified in implementation. + +MCP config: + +- Prefer opencode native MCP support where possible. +- Generate MCP entries from the same `skills.materialize_for_agent()` result used today. +- In shared MCP mode, pass Pinky shared MCP HTTP endpoints with `X-Agent-Name`. +- Technical verification: generated opencode types for remote MCP config include + `headers`, so Pinky shared MCP identity headers can be represented in config. +- Even with header support, the first implementation should prefer generated + config roots over mutating a user's hand-written `opencode.json`. + +Config file ownership: + +- PinkyBot should generate runtime config into a Pinky-owned cache path, not mutate the user's hand-written `opencode.json` directly. +- Suggested path: + + ```text + data/opencode/{server_key}/opencode.json + ``` + +- Include a header/comment-equivalent in adjacent metadata that says it is generated. + +Config regeneration triggers are intentionally centralized in +`OpencodeServerManager`. Individual registry write paths should mark agent config +dirty; the manager decides whether to rewrite immediately or lazily. This avoids +duplicating opencode config rendering across agent CRUD, directive writes, skill +assignment, and session startup. + +## 7. Provider/Model Config Flow + +User-facing model selection should stay in PinkyBot. + +Recommended fields: + +- `agents.runtime = 'opencode'` +- `agents.model = 'deepseek/deepseek-v4-pro'` or a selected row from `models` +- `providers` table can still store display/provider presets, but for opencode the effective output is an opencode model id string. + +Do not make users edit generated `opencode.json`. + +Flow: + +1. User selects runtime: `opencode`. +2. UI lists opencode-compatible provider/model ids. This can initially be manually seeded in PinkyBot's `models` table, then later refreshed from opencode/models.dev if desired. +3. User selects `DeepSeek V4-Pro`. +4. Pinky stores the chosen model id in `agent.model` or `provider_model`. +5. `_start_streaming_session()` passes `effective_model` to `OpencodeSession`. +6. `OpencodeServerManager` regenerates config if agent prompt/model/permissions/MCP config changed. + +Secrets: + +- Phase 1 provider secrets must use scoped environment variables only. +- Technical verification: opencode config supports secret substitution from env + and file values, and provider config supports `options.apiKey`. +- Generated opencode config should reference environment variables rather than + embedding raw provider keys or file substitutions. +- The manager should launch opencode with a scoped environment containing only + provider keys required by opencode agents attached to that server root. +- opencode also exposes `/auth/{providerID}` for setting auth credentials; keep + that as a flagged future path, not a Phase 1 path. +- Do not write provider keys into generated config files in Phase 1. + +Decision trail: Brad chose scoped env vars for Phase 1 on 2026-05-02. + +## 8. Opencode Runtime Dependency + +Treat opencode as an optional runtime dependency, not a hard PinkyBot install +requirement for agents that do not use the opencode runtime. + +Install detection: + +- `opencode --version` +- `PINKY_OPENCODE_CMD` can override the executable path for dev/test. + +Mac Mini / production deploy path: + +- Install with npm, pinned to a deliberate version: + + ```text + npm install -g opencode-ai@ + ``` + +- Pin the version in PinkyBot deploy config alongside other runtime/SDK pins. +- Bump deliberately through `update_and_restart`/deploy changes, not silently. +- Expected result: `opencode` is on `PATH`. +- `PINKY_OPENCODE_CMD` remains available for development and test overrides. + +Linux/Pi/Mini: + +- Use the same npm-pinned install path where Node/npm is available. +- If npm install is unavailable on a target, keep opencode disabled with a clear + health error until Brad approves an alternate path: + + ```text + Opencode runtime unavailable: opencode executable not found. Install opencode or configure PINKY_OPENCODE_CMD. + ``` + +Non-Phase-1 alternatives: + +- Homebrew is not the Mac Mini path because the available formula was stale + during review. +- `bunx` is useful for local verification but not the production deploy path; + Brad does not want Bun as a platform dependency when Node is already present. +- curl-bash install paths are out for Phase 1 because they fight reproducibility + and version pinning. + +Configuration: + +- `PINKY_OPENCODE_CMD`: override executable path. +- `PINKY_OPENCODE_MODE=managed|external`. +- `PINKY_OPENCODE_URL`: external server URL. +- `PINKY_OPENCODE_SERVER_PASSWORD`: external password. + +Rollout should not break existing Claude/Codex installs when opencode is missing. + +Decision trail: Brad chose npm global install with pinned version for the Mac +Mini deployment path on 2026-05-02. + +## 9. Test Plan + +Unit tests: + +- REST client builds Basic auth headers. +- REST client handles `/global/health` healthy/unhealthy. +- Session creation persists returned `session_id` through `_on_session_id`. +- `send()` accepts `agent_hint` and appends it only to the runtime prompt. +- `send()` uses the wait-for-completion message endpoint for MVP. +- `send()` stores raw prompt in `ConversationStore`. +- SSE dispatcher routes events by opencode `session_id`. +- Event normalization maps assistant/tool/error/completion events into PinkyBot stream events. +- Cost/tokens are extracted from assistant message or step-finish data when present. +- `force_restart()` clears persisted session id and creates a new opencode session. +- `idle_sleep()` preserves session id and sets idle sleeping. +- `attempt_reconnect()` restarts/reconnects via manager when health/SSE fails. +- Runtime selection chooses `OpencodeSession` only when `agent.runtime == "opencode"`. +- Boot-time backfill maps legacy `provider_url == "codex_cli"` to `runtime = "codex_cli"`. + +Integration tests: + +- Fixture starts `opencode serve` on a random localhost port with a random password. +- Test creates a temporary agent config and session. +- Test sends a trivial prompt to `/session/:id/message`. +- Test consumes `/global/event` until assistant completion or timeout. +- Test verifies Pinky `StreamingTurnResult` callback receives final response. +- Test verifies `/agents/{name}/streaming/events` emits normalized events. + +Smoke test: + +```text +1. Start PinkyBot with PINKY_OPENCODE_MODE=managed. +2. Create agent `deepseek-test` with runtime=opencode and model=deepseek/deepseek-v4-pro. +3. Send "Reply with exactly pong." +4. Verify Telegram/web response is "pong" or provider-equivalent. +5. Kill opencode serve. +6. Verify watchdog recovery restarts/reconnects without losing the PinkyBot process. +``` + +CI gating: + +- Unit tests always run. +- Main CI should include an opencode integration job rather than silently + skipping forever. The job should install the pinned opencode version with npm + or use a Docker fixture that contains that same pinned version, then launch + `opencode serve` on a random localhost port. +- Local developer runs may skip integration tests if opencode or npm is missing, + but `main` should exercise the real server path. +- If the team chooses not to run opencode integration in CI, document that as an + accepted risk in the PR and keep a manual smoke checklist mandatory before + enabling `PINKY_ENABLE_OPENCODE` in production. + +## 10. Migration And Rollout + +Phase 0: spec and review. + +Phase 1: hidden backend runtime. + +- Add `runtime` column. +- Add `OpencodeServerManager`. +- Add REST client. +- Add `OpencodeSession`. +- Add runtime selection behind feature flag: + + ```text + PINKY_ENABLE_OPENCODE=1 + ``` + +- No UI by default. + +Phase 2: local dogfood. + +- Enable one test agent. +- Keep Claude/Codex agents unchanged. +- Track reliability: + - send latency + - turn completion rate + - SSE reconnects + - watchdog recoveries + - tool-call success + - reported cost/tokens coverage + +Circuit breaker: + +- Track opencode runtime errors per agent and globally: + - failed sends + - reconnect attempts + - `force_restart()` calls + - watchdog recoveries + - global SSE disconnects +- If an opencode agent exceeds 3 forced restarts in 1 hour, disable new sends to + that opencode session, mark it unhealthy, and alert Brad through the existing + owner notification path. +- If all opencode agents together exceed 10 forced restarts or watchdog + recoveries in 1 hour, automatically flip the process-level opencode feature + gate to disabled for new session starts until manual intervention. +- Circuit breaker rollback should not rewrite agent runtime fields. It should + stop starting new opencode sessions and tell the operator to switch affected + agents back to `claude_sdk` or `codex_cli`. + +Cost visibility: + +- Verified opencode types expose cost/tokens on assistant message and + step-finish data, so the implementation should wire those into + `StreamingTurnResult.total_cost_usd`, `model_usage`, and analytics. +- If a provider/event path does not return cost, do not silently report `$0`. + Mark cost as unknown in session stats/analytics and surface a warning in + diagnostics. `$0` is acceptable only when opencode explicitly reports zero. + +Phase 3: UI support. + +- Add runtime selector. +- Add opencode model selector. +- Show opencode server health in diagnostics. +- Make install/runtime errors actionable. + +Phase 4: optional migration. + +- Do not auto-migrate agents. +- Offer manual clone/migrate: + + ```text + Clone agent as opencode runtime + ``` + +- Keep `codex_session.py` and `streaming_session.py` indefinitely until real usage shows opencode can replace them. + +## Open Questions + +1. What is the exact generated config root strategy for multiple PinkyBot + `working_dir` values: one opencode server per root, or a shared server with + per-session directory metadata? The spec assumes a manager pool keyed by + runtime root until implementation verifies opencode's directory isolation. +2. Does opencode apply regenerated agent config to an existing session in place, + or is a fresh opencode session always required? The spec assumes restart for + permission changes until proven otherwise. +3. Which exact opencode event names should be treated as terminal for a + completed assistant turn? Generated types show `session.idle` and + message/part events, but implementation should confirm the most reliable + completion signal from `/doc` or a fixture trace. +4. How should opencode slash commands map to PinkyBot admin actions, if at all? Initial implementation should keep them internal to opencode. + +## Non-Goals + +- Replacing existing Claude SDK or Codex runtime in the first implementation. +- Reworking PinkyBot's memory model. +- Moving provider secrets into generated opencode config unless unavoidable. +- UI polish beyond basic runtime/model selection during initial backend integration. +- Any implementation in this spec branch. + +## References + +- Official server docs: +- Official config docs: +- Official agents docs: +- Generated SDK types checked during verification: + diff --git a/src/pinky_daemon/agent_registry.py b/src/pinky_daemon/agent_registry.py index 95a44880..f2dbd006 100644 --- a/src/pinky_daemon/agent_registry.py +++ b/src/pinky_daemon/agent_registry.py @@ -185,6 +185,7 @@ class Agent: working_status: str = "idle" # idle, working, offline working_status_updated_at: float = 0.0 # When working_status last changed last_seen_at: float = 0.0 # Server-side presence: updated on delivery/turn completion + runtime: str = "claude_sdk" # Agent runtime: claude_sdk, codex_cli, opencode provider_url: str = "" # e.g. "http://localhost:11434" for Ollama, empty = Anthropic default provider_key: str = "" # API key override, empty = use ANTHROPIC_API_KEY env var provider_model: str = "" # model name override (e.g. "llama3.2"), empty = use agent.model @@ -243,6 +244,7 @@ def to_dict(self) -> dict: "working_status": self.working_status, "working_status_updated_at": self.working_status_updated_at, "last_seen_at": self.last_seen_at, + "runtime": self.runtime, "provider_url": self.provider_url, "provider_key": self.provider_key, "provider_model": self.provider_model, @@ -475,6 +477,7 @@ def _init_tables(self) -> None: heartbeat_interval INTEGER NOT NULL DEFAULT 0, plain_text_fallback INTEGER NOT NULL DEFAULT 0, role TEXT NOT NULL DEFAULT '', + runtime TEXT NOT NULL DEFAULT 'claude_sdk', created_at REAL NOT NULL, updated_at REAL NOT NULL ); @@ -729,11 +732,14 @@ def _migrate(self) -> None: ("thinking_effort", "TEXT NOT NULL DEFAULT 'medium'"), ("watchdog_config", "TEXT NOT NULL DEFAULT '{}'"), ("last_seen_at", "REAL NOT NULL DEFAULT 0"), + ("runtime", "TEXT NOT NULL DEFAULT 'claude_sdk'"), ] for col, typedef in migrations: if col not in existing: self._db.execute(f"ALTER TABLE agents ADD COLUMN {col} {typedef}") _log(f"agent_registry: migrated — added column {col}") + self._db.commit() + self._backfill_runtime_from_provider_url() # Migrate agent_schedules table sched_existing = { @@ -804,6 +810,25 @@ def _migrate(self) -> None: # Seed default models self._seed_models() + def _backfill_runtime_from_provider_url(self) -> None: + """One-shot migration from legacy provider_url runtime selection.""" + marker = "migration:agents_runtime_codex_cli_backfill" + if self.get_setting(marker) == "1": + return + + cursor = self._db.execute( + "UPDATE agents SET runtime='codex_cli' " + "WHERE provider_url='codex_cli' AND runtime='claude_sdk'" + ) + self._db.execute( + "INSERT INTO system_settings (key, value) VALUES (?, '1') " + "ON CONFLICT(key) DO UPDATE SET value='1'", + (marker,), + ) + self._db.commit() + if cursor.rowcount: + _log(f"agent_registry: backfilled runtime=codex_cli for {cursor.rowcount} agent(s)") + # ── Workspace Init ───────────────────────────────────── @staticmethod @@ -932,7 +957,7 @@ def register(self, name: str, **kwargs) -> Agent: "clock_aligned", "auto_sleep_hours", "plain_text_fallback", "voice_config", "role", "dream_enabled", "dream_schedule", "dream_timezone", "dream_model", "dream_notify", "librarian_enabled", "librarian_schedule", - "provider_url", "provider_key", "provider_model", "provider_ref", + "runtime", "provider_url", "provider_key", "provider_model", "provider_ref", "thinking_effort"): if key in kwargs: updates[key] = kwargs[key] @@ -1014,6 +1039,12 @@ def register(self, name: str, **kwargs) -> Agent: dream_notify=kwargs.get("dream_notify", True), librarian_enabled=kwargs.get("librarian_enabled", False), librarian_schedule=kwargs.get("librarian_schedule", "0 4 * * *"), + runtime=kwargs.get("runtime", "claude_sdk"), + provider_url=kwargs.get("provider_url", ""), + provider_key=kwargs.get("provider_key", ""), + provider_model=kwargs.get("provider_model", ""), + provider_ref=kwargs.get("provider_ref", ""), + thinking_effort=kwargs.get("thinking_effort", "medium"), watchdog_config=kwargs.get("watchdog_config", {}), created_at=now, updated_at=now, @@ -1027,9 +1058,11 @@ def register(self, name: str, **kwargs) -> Agent: max_sessions, enabled, auto_start, heartbeat_interval, plain_text_fallback, wake_interval, clock_aligned, auto_sleep_hours, voice_config, role, dream_enabled, dream_schedule, dream_timezone, dream_model, dream_notify, - librarian_enabled, librarian_schedule, watchdog_config, + librarian_enabled, librarian_schedule, + runtime, provider_url, provider_key, provider_model, provider_ref, + thinking_effort, watchdog_config, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""", + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""", (agent.name, agent.display_name, agent.model, agent.soul, agent.users, agent.boundaries, agent.system_prompt, agent.working_dir, agent.permission_mode, @@ -1042,7 +1075,9 @@ def register(self, name: str, **kwargs) -> Agent: json.dumps(agent.voice_config), agent.role, int(agent.dream_enabled), agent.dream_schedule, agent.dream_timezone, agent.dream_model, int(agent.dream_notify), int(agent.librarian_enabled), agent.librarian_schedule, - json.dumps(agent.watchdog_config), + agent.runtime, agent.provider_url, agent.provider_key, + agent.provider_model, agent.provider_ref, + agent.thinking_effort, json.dumps(agent.watchdog_config), agent.created_at, agent.updated_at), ) self._db.commit() @@ -1060,7 +1095,7 @@ def register(self, name: str, **kwargs) -> Agent: "dream_enabled, dream_schedule, dream_timezone, dream_model, dream_notify, " "librarian_enabled, librarian_schedule, " "working_status, working_status_updated_at, " - "provider_url, provider_key, provider_model, provider_ref, " + "runtime, provider_url, provider_key, provider_model, provider_ref, " "disallowed_tools, thinking_effort, watchdog_config, last_seen_at" ) @@ -2584,14 +2619,15 @@ def _row_to_agent(self, row: tuple) -> Agent: librarian_schedule=row[36] if len(row) > 36 and row[36] else "0 4 * * *", working_status=row[37] if len(row) > 37 and row[37] else "idle", working_status_updated_at=row[38] if len(row) > 38 else 0.0, - provider_url=row[39] if len(row) > 39 and row[39] else "", - provider_key=row[40] if len(row) > 40 and row[40] else "", - provider_model=row[41] if len(row) > 41 and row[41] else "", - provider_ref=row[42] if len(row) > 42 and row[42] else "", - disallowed_tools=json.loads(row[43]) if len(row) > 43 and row[43] else [], - thinking_effort=row[44] if len(row) > 44 and row[44] else "medium", - watchdog_config=json.loads(row[45]) if len(row) > 45 and row[45] else {}, - last_seen_at=row[46] if len(row) > 46 else 0.0, + runtime=row[39] if len(row) > 39 and row[39] else "claude_sdk", + provider_url=row[40] if len(row) > 40 and row[40] else "", + provider_key=row[41] if len(row) > 41 and row[41] else "", + provider_model=row[42] if len(row) > 42 and row[42] else "", + provider_ref=row[43] if len(row) > 43 and row[43] else "", + disallowed_tools=json.loads(row[44]) if len(row) > 44 and row[44] else [], + thinking_effort=row[45] if len(row) > 45 and row[45] else "medium", + watchdog_config=json.loads(row[46]) if len(row) > 46 and row[46] else {}, + last_seen_at=row[47] if len(row) > 47 else 0.0, ) # ── Cost Tracking ────────────────────────────────────── diff --git a/src/pinky_daemon/api.py b/src/pinky_daemon/api.py index d5535d8c..f76d6ba9 100644 --- a/src/pinky_daemon/api.py +++ b/src/pinky_daemon/api.py @@ -447,6 +447,11 @@ class RegisterAgentRequest(BaseModel): auto_start: bool = False role: str = "" heartbeat_interval: int = 0 + runtime: str = "claude_sdk" + provider_url: str = "" # ANTHROPIC_BASE_URL override (e.g. Ollama endpoint) + provider_key: str = "" # ANTHROPIC_API_KEY override + provider_model: str = "" # Model name override for this provider + provider_ref: str = "" # ID of a global provider from the providers table thinking_effort: str = "medium" # low/medium/high/xhigh/max watchdog_config: dict | None = None # Per-agent watchdog overrides @@ -480,9 +485,11 @@ class UpdateAgentRequest(BaseModel): dream_timezone: str | None = None # IANA timezone for dream schedule dream_model: str | None = None # Model override for dream runs (empty = agent's model) dream_notify: bool | None = None # Inject dream summary into morning wake context + runtime: str | None = None # Runtime selector: claude_sdk, codex_cli, opencode provider_url: str | None = None # ANTHROPIC_BASE_URL override (e.g. Ollama endpoint) provider_key: str | None = None # ANTHROPIC_API_KEY override provider_model: str | None = None # Model name override for this provider + provider_ref: str | None = None # ID of a global provider from the providers table thinking_effort: str | None = None # low/medium/high/xhigh/max watchdog_config: dict | None = None # Per-agent watchdog overrides @@ -1303,6 +1310,17 @@ async def security_headers_middleware(request: Request, call_next): _comms_db = db_path.replace(".db", "_agent_comms.db") comms = AgentComms(db_path=_comms_db) + # Auth-failure tracker — counts SDK auth errors across all streaming + # sessions on this host and decides when to alert the operator. The + # callback wiring is created in _build_auth_alert_callbacks() below + # once _broker_send is available; the tracker itself is host-global. + from pinky_daemon.auth_alerts import ( + AuthFailureTracker, + format_alert_message, + resolve_operator_chat, + ) + auth_tracker = AuthFailureTracker() + # Message broker — routes platform messages through approval checks to agent sessions _platform_adapters: dict[tuple[str, str], object] = {} @@ -1616,6 +1634,10 @@ async def _broker_send( silent=silent, ), ) + # Once an outbound message lands, the typing indicator becomes noise — + # Telegram has no "stop typing" API, but cancelling our 4s sendChatAction + # loop prevents the indicator from popping back up after the message arrives. + broker._stop_typing(agent_name, chat_id) return { "sent": True, "agent": agent_name, @@ -1814,11 +1836,14 @@ def _generate_and_send(): try: loop = asyncio.get_running_loop() - return await loop.run_in_executor(None, _generate_and_send) + result = await loop.run_in_executor(None, _generate_and_send) except HTTPException: raise except Exception as e: raise HTTPException(500, f"Voice note failed: {e}") + # Stop the typing loop now that the voice message has landed. + broker._stop_typing(agent_name, chat_id) + return result activity = ActivityStore(db_path=db_path.replace(".db", "_activity.db")) @@ -2273,6 +2298,86 @@ async def _on_stream_event(event: dict): return _on_stream_event + async def _on_auth_failure(agent_name: str, error: str) -> None: + """Streaming-session hook: record an auth failure and alert the operator. + + Called from StreamingSession's reader loop whenever the SDK reports + an authentication-class error — either AssistantMessage.error == + "authentication_failed" (see streaming_session._is_auth_error_assistant) + or ResultMessage.api_error_status in {401, 403} (see + streaming_session._is_auth_error_result). The AuthFailureTracker + decides whether this failure crosses the alert threshold and whether + the cooldown has elapsed; if both, we DM the operator via + _broker_send using the affected agent's own bot. + """ + try: + decision = auth_tracker.record_failure(agent_name, error) + except Exception as e: + _log(f"auth_alerts: tracker.record_failure raised: {e}") + return + + if not decision.get("should_alert"): + return + + try: + chat_id, platform = resolve_operator_chat( + get_setting=agents.get_setting, + list_all_approved_users=agents.list_all_approved_users, + ) + except Exception as e: + _log(f"auth_alerts: resolve_operator_chat raised: {e}") + return + + if not chat_id: + _log( + "auth_alerts: auth failure detected but no operator_chat_id " + "configured and no approved_users to fall back on — " + "alert suppressed (set system_settings.operator_chat_id)" + ) + return + + try: + host_label = agents.get_setting("host_label", "") or "" + except Exception: + host_label = "" + + body = format_alert_message( + agent_name=agent_name, + decision=decision, + error=error, + host_label=host_label, + ) + + try: + await _broker_send(agent_name, platform, chat_id, body) + except Exception as e: + # Delivery failed — do NOT commit the alert. The next failure + # crossing threshold will retry instead of being silenced for + # the cooldown window. + _log( + f"auth_alerts: failed to send operator alert via " + f"{agent_name} bot to {platform}:{chat_id}: {e} " + f"(cooldown not advanced — will retry on next failure)" + ) + return + + try: + auth_tracker.commit_alert() + except Exception as e: + _log(f"auth_alerts: tracker.commit_alert raised: {e}") + _log( + f"auth_alerts: operator alert sent to {platform}:{chat_id} " + f"for {agent_name} (reason={decision.get('reason')}, " + f"agents_failing={decision.get('agents_failing')})" + ) + + def _on_auth_success(agent_name: str) -> None: + """Clear an agent's auth-failure record after a successful turn.""" + try: + auth_tracker.record_success(agent_name) + except Exception as e: + _log(f"auth_alerts: tracker.record_success raised: {e}") + async def _make_streaming_session_id_callback(agent_name: str, label: str): """Persist a streaming session ID when captured from the SDK. @@ -2447,12 +2552,44 @@ async def _start_streaming_session( # Deduplicate effective_disallowed = sorted(set(effective_disallowed)) + def runtime_from_legacy_provider(agent_config) -> str: + """Deprecated temporary shim for the runtime rollout. + + Runtime selection now belongs to agents.runtime. This fallback exists + only for legacy rows that have not passed the one-shot codex_cli + provider_url backfill yet, and should be removed after rollout. + """ + runtime = (getattr(agent_config, "runtime", "") or "").strip() + if runtime: + return runtime + if (getattr(agent_config, "provider_url", "") or "").strip() == "codex_cli": + return "codex_cli" + return "claude_sdk" + + runtime = runtime_from_legacy_provider(agent) + if runtime == "opencode": + if os.environ.get("PINKY_ENABLE_OPENCODE", "0") == "1": + msg = "opencode runtime is enabled but OpencodeSession is not implemented yet" + status = 501 + else: + msg = "opencode runtime is disabled; set PINKY_ENABLE_OPENCODE=1 after implementation lands" + status = 503 + _log(f"api: refusing to start {agent_name}: {msg}") + raise HTTPException(status, msg) + if runtime not in {"claude_sdk", "codex_cli"}: + msg = f"unknown runtime '{runtime}' for agent '{agent_name}'" + _log(f"api: {msg}") + raise HTTPException(400, msg) + + is_codex = runtime == "codex_cli" resolved_provider_url, resolved_provider_key, resolved_provider_model = _resolve_agent_provider(agent) + if is_codex: + resolved_provider_url = "codex_cli" effective_model = resolved_provider_model or agent.model # Build MCP server config for Codex agents (injected via -c flags) codex_mcp_servers = {} - if resolved_provider_url == "codex_cli" and SHARED_MCP_ENABLED: + if is_codex and SHARED_MCP_ENABLED: shared_base = f"http://{SHARED_MCP_HOST}:{SHARED_MCP_PORT}" agent_headers = {"X-Agent-Name": agent_name} for srv_name in ("self", "memory", "messaging"): @@ -2488,8 +2625,7 @@ async def _start_streaming_session( callback = await _make_streaming_response_callback() sid_callback = await _make_streaming_session_id_callback(agent_name, label) - # Select session class based on provider type - is_codex = resolved_provider_url == "codex_cli" + # Select session class based on the persisted runtime. SessionClass = CodexSession if is_codex else StreamingSession # noqa: N806 init_kwargs = { @@ -2499,6 +2635,13 @@ async def _start_streaming_session( "analytics_store": analytics, "registry": agents, } + # Auth-failure detection is currently only wired for the SDK-based + # StreamingSession. Codex sessions don't surface an equivalent + # "authentication_failed" signal — they rely on a separate provider + # token lifecycle — so we skip the hooks there. + if not is_codex: + init_kwargs["auth_alert_callback"] = _on_auth_failure + init_kwargs["auth_success_callback"] = _on_auth_success if is_codex: init_kwargs["stream_event_callback"] = await _make_streaming_event_callback(agent_name, label) @@ -2514,7 +2657,7 @@ async def _start_streaming_session( session_id=ss.id, agent_name=agent_name, session_label=label, - provider=resolved_provider_url or "default", + provider=runtime if is_codex else (resolved_provider_url or "default"), model=effective_model or "", ) analytics.log_activity( @@ -5244,6 +5387,11 @@ async def register_agent(req: RegisterAgentRequest): auto_start=req.auto_start, role=req.role, heartbeat_interval=req.heartbeat_interval, + runtime=req.runtime, + provider_url=req.provider_url, + provider_key=req.provider_key, + provider_model=req.provider_model, + provider_ref=req.provider_ref, thinking_effort=req.thinking_effort, watchdog_config=req.watchdog_config or {}, ) @@ -5921,6 +6069,44 @@ async def set_agent_token(name: str, platform: str, req: SetAgentTokenRequest): except Exception as e: _log(f"api: failed to start telegram poller for {name}: {e}") + # Dynamically start/restart a broker poller for Discord tokens + if platform == "discord" and raw_token: + try: + from pinky_daemon.pollers import BrokerDiscordPoller + from pinky_outreach.discord import DiscordAdapter + + # Stop any existing poller for this agent + for p in list(_broker_pollers): + if ( + isinstance(p, BrokerDiscordPoller) + and p._agent_name == name + ): + p.stop() + _broker_pollers.remove(p) + _log(f"api: stopped old discord poller for {name}") + break + + # Optional per-token overrides come through token settings. + settings = req.settings or {} + poll_interval = float(settings.get("poll_interval_sec", 1.0)) + watched = settings.get("watched_channels") or None + + adapter = DiscordAdapter(raw_token) + poller = BrokerDiscordPoller( + adapter, name, broker, registry=agents, + poll_interval=poll_interval, + watched_channels=watched, + ) + _broker_pollers.append(poller) + asyncio.create_task(poller.start()) + _log( + f"api: started discord poller for {name} " + f"(poll_interval={poll_interval}s, " + f"watched={'auto' if not watched else len(watched)})" + ) + except Exception as e: + _log(f"api: failed to start discord poller for {name}: {e}") + return token.to_dict() @app.get("/agents/{name}/tokens") @@ -5950,6 +6136,19 @@ async def remove_agent_token(name: str, platform: str): _log(f"api: stopped telegram poller for {name}") break + # Stop broker poller if removing a Discord token + if platform == "discord": + from pinky_daemon.pollers import BrokerDiscordPoller + for p in list(_broker_pollers): + if ( + isinstance(p, BrokerDiscordPoller) + and p._agent_name == name + ): + p.stop() + _broker_pollers.remove(p) + _log(f"api: stopped discord poller for {name}") + break + return {"deleted": True, "agent": name, "platform": platform} # ── MCP Servers ──────────────────────────────────────── @@ -6472,6 +6671,7 @@ async def broker_send_photo(req: dict): loop = asyncio.get_running_loop() msg = await loop.run_in_executor(None, lambda: _send_file_message(agent_name, platform, chat_id, file_path, caption=caption, reply_to=reply_to, kind="photo")) result = {"sent": True, "message_id": msg.message_id, "platform": platform, "chat_id": chat_id} + broker._stop_typing(agent_name, chat_id) _record_outbound_message( agent_name, platform=platform, @@ -6503,6 +6703,7 @@ async def broker_send_document(req: dict): loop = asyncio.get_running_loop() msg = await loop.run_in_executor(None, lambda: _send_file_message(agent_name, platform, chat_id, file_path, caption=caption, reply_to=reply_to, kind="document")) result = {"sent": True, "message_id": msg.message_id, "platform": platform, "chat_id": chat_id} + broker._stop_typing(agent_name, chat_id) _record_outbound_message( agent_name, platform=platform, @@ -6601,6 +6802,7 @@ def _download_and_send(): try: msg = await loop.run_in_executor(None, _download_and_send) result = {"sent": True, "message_id": msg.message_id, "query": query, "platform": platform, "chat_id": chat_id} + broker._stop_typing(agent_name_req, chat_id) _record_outbound_message( agent_name_req, platform=platform, @@ -6632,6 +6834,7 @@ async def broker_send_animation(req: dict): loop = asyncio.get_running_loop() msg = await loop.run_in_executor(None, lambda: _send_file_message(agent_name, platform, chat_id, file_path, caption=caption, reply_to=reply_to, kind="animation")) result = {"sent": True, "message_id": msg.message_id, "platform": platform, "chat_id": chat_id} + broker._stop_typing(agent_name, chat_id) _record_outbound_message( agent_name, platform=platform, @@ -7853,7 +8056,12 @@ def _resolve_memory_db(agent_name: str) -> str: auto_start_agents = agents.list_auto_start_agents() # Start broker pollers and streaming sessions for all enabled agents. - from pinky_daemon.pollers import BrokeriMessagePoller, BrokerTelegramPoller + from pinky_daemon.pollers import ( + BrokerDiscordPoller, + BrokeriMessagePoller, + BrokerTelegramPoller, + ) + from pinky_outreach.discord import DiscordAdapter from pinky_outreach.telegram import TelegramAdapter all_agents = agents.list(enabled_only=True) streaming_count = 0 @@ -7885,6 +8093,42 @@ def _resolve_memory_db(agent_name: str) -> str: asyncio.create_task(poller.start()) _log(f"startup: broker poller started for {agent.name}") + # Discord poller — REST polling (Gateway/WebSocket is a future v0.2) + discord_token = agents.get_raw_token(agent.name, "discord") + if discord_token: + existing = any( + isinstance(p, BrokerDiscordPoller) and p._agent_name == agent.name + for p in _broker_pollers + ) + if existing: + _log(f"startup: discord poller already exists for {agent.name}, skipping") + else: + try: + # Read per-token settings (poll_interval_sec, watched_channels) + settings = {} + for tok in agents.list_tokens(agent.name): + if tok.platform == "discord": + settings = tok.settings or {} + break + poll_interval = float(settings.get("poll_interval_sec", 1.0)) + watched = settings.get("watched_channels") or None + + d_adapter = DiscordAdapter(discord_token) + d_poller = BrokerDiscordPoller( + d_adapter, agent.name, broker, registry=agents, + poll_interval=poll_interval, + watched_channels=watched, + ) + _broker_pollers.append(d_poller) + asyncio.create_task(d_poller.start()) + _log( + f"startup: discord poller started for {agent.name} " + f"(interval={poll_interval}s, " + f"channels={'auto' if not watched else len(watched)})" + ) + except Exception as e: + _log(f"startup: discord poller failed for {agent.name}: {e}") + # iMessage poller — check if agent has imessage enabled in DB if agents.get_raw_token(agent.name, "imessage"): try: @@ -8035,8 +8279,22 @@ async def on_shutdown(): @app.get("/admin/watchdog") async def get_watchdog_status(): - """Return session watchdog status and tracked agents.""" - return watchdog.status() + """Return session watchdog status, tracked agents, and Claude auth health. + + Three top-level keys for external monitors: + running / check_interval / agents — session-stuck watchdog state. + auth_status — host-wide Claude auth health (see auth_alerts). + + ``auth_status.status`` is one of ``ok | degraded | broken``; tying a + Prometheus alert to ``broken`` catches credential outages quickly. + """ + out = watchdog.status() + try: + out["auth_status"] = auth_tracker.status() + except Exception as e: + _log(f"admin/watchdog: auth_tracker.status raised: {e}") + out["auth_status"] = {"status": "unknown", "error": str(e)} + return out # ── Admin: Shared MCP Status ───────────────────────── @@ -8093,7 +8351,12 @@ async def set_release_channel(channel: str = "stable"): # ── Admin: Update & Restart ─────────────────────────── @app.post("/admin/update") - async def admin_update(branch: str = "", dry_run: bool = False): + async def admin_update( + branch: str = "", + dry_run: bool = False, + force: bool = False, + force_deps: bool = False, + ): """Pull latest code, rebuild if needed, and restart the daemon. The process manager (launchctl/systemd) must be installed for @@ -8103,9 +8366,20 @@ async def admin_update(branch: str = "", dry_run: bool = False): Beta channel: pulls HEAD of the beta branch. Branch defaults to PINKYBOT_CHANNEL env var ("stable" -> release tags, "beta" -> beta branch), falling back to "stable" if unset. + + force=True discards local modifications to TRACKED files + (`git checkout -- .`) before pulling, to recover from a dirty working + tree (e.g. stale build artifacts blocking the update). Untracked files + are preserved — no `git clean`. Ignored when dry_run=True. + + force_deps=True: reinstall dependencies even when git HEAD didn't + change. Use this when installed package versions have drifted from + pyproject.toml (e.g., manual pip install bypassed, or the daemon was + seeded from a stale image). """ import shutil import subprocess as sp + import sys if not branch: channel = os.environ.get("PINKYBOT_CHANNEL", "stable") @@ -8209,6 +8483,29 @@ async def admin_update(branch: str = "", dry_run: bool = False): except Exception as e: _log(f"admin: branch checkout warning: {e}") + # Force mode: discard local mods to TRACKED files before pulling. + # Untracked files (e.g. .env, local notes) are NOT touched. + forced_reset = False + forced_files: list[str] = [] + if force: + try: + dirty = sp.check_output( + ["git", "diff", "--name-only", "HEAD"], + cwd=repo_dir, stderr=sp.DEVNULL, timeout=10, + ).decode().strip() + if dirty: + forced_files = [f for f in dirty.splitlines() if f.strip()] + sp.check_output( + ["git", "checkout", "--", "."], + cwd=repo_dir, stderr=sp.STDOUT, timeout=30, + ) + forced_reset = True + _log(f"admin: force=True reset {len(forced_files)} tracked file(s): {forced_files[:10]}") + except sp.CalledProcessError as e: + return {"error": f"force reset failed: {e.output.decode()[:500]}"} + except Exception as e: + _log(f"admin: force reset warning: {e}") + try: sp.check_output( ["git", "pull", "origin", branch], @@ -8234,23 +8531,45 @@ async def admin_update(branch: str = "", dry_run: bool = False): except Exception: summary = "" - # Detect dependency changes + # Detect dependency changes — rebuild whenever pyproject.toml or uv.lock + # changed in the pull, or when force_deps=True is passed (escape hatch + # for installed-vs-pinned drift that git diff can't see). deps_rebuilt = False + deps_error = "" try: - changed = sp.check_output( - ["git", "diff", "--name-only", before_hash, after_hash, "--", "pyproject.toml"], - cwd=repo_dir, stderr=sp.DEVNULL, timeout=10, - ).decode().strip() - if changed: - venv_pip = str(Path(repo_dir) / ".venv" / "bin" / "pip") - if Path(venv_pip).exists(): - sp.check_output( - [venv_pip, "install", "-e", ".[all]", "--quiet"], - cwd=repo_dir, stderr=sp.STDOUT, timeout=120, - ) - deps_rebuilt = True - except Exception: - pass + if before_hash != after_hash: + changed = sp.check_output( + ["git", "diff", "--name-only", before_hash, after_hash, "--", + "pyproject.toml", "uv.lock"], + cwd=repo_dir, stderr=sp.DEVNULL, timeout=10, + ).decode().strip() + else: + changed = "" + + if changed or force_deps: + # Prefer project venv pip if present, else use the running daemon's + # interpreter (sys.executable). This works for both venv and + # system-python deployments — the prior `.venv/bin/pip`-only path + # silently skipped rebuilds on system-python hosts. + venv_pip = Path(repo_dir) / ".venv" / "bin" / "pip" + if venv_pip.exists(): + pip_cmd = [str(venv_pip), "install", "-e", ".[all]", "--quiet"] + else: + # PEP 668: system pythons (Homebrew, Debian) mark themselves + # externally-managed. --break-system-packages lets us install + # into the same env the daemon imports from. + pip_cmd = [ + sys.executable, "-m", "pip", "install", + "-e", ".[all]", "--quiet", "--break-system-packages", + ] + sp.check_output(pip_cmd, cwd=repo_dir, stderr=sp.STDOUT, timeout=180) + deps_rebuilt = True + except sp.CalledProcessError as e: + deps_error = f"pip install failed: {e.output.decode()[:500] if e.output else e}" + _log(f"admin: {deps_error}") + except Exception as e: + deps_error = f"deps rebuild failed: {e}" + _log(f"admin: {deps_error}") # Always rebuild frontend on update to keep compiled assets fresh frontend_rebuilt = False @@ -8281,8 +8600,11 @@ async def admin_update(branch: str = "", dry_run: bool = False): "release": target_tag if use_release_tags else None, "commits": summary.splitlines() if summary else [], "deps_rebuilt": deps_rebuilt, + "deps_error": deps_error or None, "frontend_rebuilt": frontend_rebuilt, "frontend_error": frontend_error or None, + "forced_reset": forced_reset, + "forced_files": forced_files, "restarting": before_hash != after_hash or deps_rebuilt, } diff --git a/src/pinky_daemon/auth_alerts.py b/src/pinky_daemon/auth_alerts.py new file mode 100644 index 00000000..10e27638 --- /dev/null +++ b/src/pinky_daemon/auth_alerts.py @@ -0,0 +1,316 @@ +"""Auth-failure tracking and operator alerts. + +When the Claude SDK returns ``error='authentication_failed'`` for an agent's +streaming session, the user-facing chat sees nothing — the daemon suppresses +the response so raw error JSON never reaches end users. The downside is the +operator (the person who can re-auth) has no idea Claude credentials are +broken until someone complains. + +This module provides: + +- ``AuthFailureTracker`` — in-memory counter that records auth failures across + all agents on a host, decides when an alert should fire, and enforces a + cooldown so we don't spam the operator. +- ``resolve_operator_chat()`` — best-effort lookup of the operator's + chat_id/platform. Reads ``operator_chat_id`` / ``operator_platform`` from + system_settings if set; otherwise picks the chat_id appearing across the + most ``approved_users`` rows (heuristic: the person with broadest access + is the operator). + +The streaming session calls into the tracker on each auth failure; when the +tracker says "alert", the daemon's alert wiring sends a Telegram DM via the +existing broker. Both are documented inline so the next Brad wedge debug is +faster. +""" +from __future__ import annotations + +import logging +import time +from collections import Counter, defaultdict +from dataclasses import dataclass, field +from typing import Iterable + +_log = logging.getLogger("pinky.auth_alerts").info +_warn = logging.getLogger("pinky.auth_alerts").warning + + +# ── Defaults ──────────────────────────────────────────────────────── + +DEFAULT_FAIL_WINDOW_SECONDS = 300 # 5 min +DEFAULT_FAIL_THRESHOLD = 3 # within window → alert +DEFAULT_ALERT_COOLDOWN_SECONDS = 1800 # 30 min between alerts + + +@dataclass +class _AgentFailures: + """Sliding-window record of recent auth failures for one agent.""" + + timestamps: list[float] = field(default_factory=list) + last_error: str = "" + + def record(self, now: float, error: str, window: float) -> int: + """Append a failure timestamp, prune older ones, return current count.""" + self.timestamps.append(now) + self.last_error = error or "" + cutoff = now - window + self.timestamps = [t for t in self.timestamps if t >= cutoff] + return len(self.timestamps) + + def reset(self) -> None: + self.timestamps = [] + self.last_error = "" + + +class AuthFailureTracker: + """Per-host auth failure detector with cooldown. + + Concurrency: callers must share a single event loop (the streaming-session + reader loop). No internal locking is needed — every method is synchronous + and mutation happens before return. Methods are O(N) over recent + timestamps which is fine — N is bounded by the threshold in practice. + + Two-phase alerting: ``record_failure()`` decides whether an alert *should* + fire but never advances the cooldown by itself. The caller must invoke + ``commit_alert()`` only after the operator notification was actually + delivered. If delivery raises (broker down, bot token revoked, etc.), the + cooldown is preserved so the next failure can retry — that's the whole + reason this PR exists, so a single failed-send doesn't silence the alert + system for 30 minutes. + """ + + def __init__( + self, + *, + fail_window_seconds: int = DEFAULT_FAIL_WINDOW_SECONDS, + fail_threshold: int = DEFAULT_FAIL_THRESHOLD, + alert_cooldown_seconds: int = DEFAULT_ALERT_COOLDOWN_SECONDS, + clock=time.time, + ) -> None: + self._window = fail_window_seconds + self._threshold = fail_threshold + self._cooldown = alert_cooldown_seconds + self._clock = clock + self._agents: dict[str, _AgentFailures] = defaultdict(_AgentFailures) + self._last_alert_at: float = 0.0 + self._first_failure_at: float = 0.0 # When current outage started + self._alert_count: int = 0 # Alerts fired since last clear + + def record_failure(self, agent_name: str, error: str = "") -> dict: + """Record one auth failure. Returns a decision dict. + + Returned keys: + should_alert (bool): True if this failure crossed the threshold + AND cooldown has elapsed. + reason (str): Short message explaining the decision. + count (int): Current failures-in-window for this agent. + agents_failing (int): Count of distinct agents currently failing. + """ + now = self._clock() + if self._first_failure_at == 0.0: + self._first_failure_at = now + + record = self._agents[agent_name] + count = record.record(now, error, self._window) + agents_failing = sum(1 for r in self._agents.values() if r.timestamps) + + # Multi-agent host outage: if ≥ threshold agents failing simultaneously, + # alert immediately on the very first qualifying failure (don't wait for + # any single agent to hit `threshold` failures alone). + threshold_hit = ( + count >= self._threshold or agents_failing >= self._threshold + ) + + if not threshold_hit: + return { + "should_alert": False, + "reason": "below_threshold", + "count": count, + "agents_failing": agents_failing, + } + + if self._last_alert_at and now - self._last_alert_at < self._cooldown: + return { + "should_alert": False, + "reason": "cooldown", + "count": count, + "agents_failing": agents_failing, + "cooldown_remaining": int( + self._cooldown - (now - self._last_alert_at) + ), + } + + # NOTE: do NOT advance _last_alert_at / _alert_count here. Caller must + # invoke commit_alert() after successful delivery; that way a broker + # failure preserves the cooldown so the alert can retry on the next + # failure instead of being silenced for the cooldown window. + return { + "should_alert": True, + "reason": ( + "host_wide" if agents_failing >= self._threshold else "agent_repeated" + ), + "count": count, + "agents_failing": agents_failing, + } + + def commit_alert(self) -> None: + """Mark an alert as delivered: advance cooldown and bump the counter. + + Call this only after the operator notification was actually sent. If + delivery fails, do NOT call this — the next record_failure() that + crosses threshold will return should_alert=True so the alert retries + instead of being lost for the cooldown window. + """ + self._last_alert_at = self._clock() + self._alert_count += 1 + + def record_success(self, agent_name: str) -> None: + """Clear failure tracking for an agent that successfully called Claude. + + If all agents are clear, reset the outage state so the next outage + emits a fresh "outage started" alert. + """ + if agent_name in self._agents: + self._agents[agent_name].reset() + if not any(r.timestamps for r in self._agents.values()): + self._first_failure_at = 0.0 + # Keep _last_alert_at in place — cooldown still respected even + # across a brief recovery → re-failure to avoid alert flapping. + + def status(self) -> dict: + """Snapshot for /admin/watchdog and diagnostics. + + Status semantics: + "ok" — no agent has failures in window. + "degraded" — some failures but below threshold. + "broken" — at or above threshold (alert is or will fire). + """ + now = self._clock() + agents_failing: list[dict] = [] + for name, rec in self._agents.items(): + # Prune stale timestamps lazily here too so /admin/watchdog + # doesn't show ghosts after the window has elapsed. + cutoff = now - self._window + rec.timestamps = [t for t in rec.timestamps if t >= cutoff] + if not rec.timestamps: + continue + agents_failing.append( + { + "agent": name, + "failures_in_window": len(rec.timestamps), + "last_failure_age_s": int(now - rec.timestamps[-1]), + "last_error": rec.last_error[:120], + } + ) + + if not agents_failing: + status = "ok" + elif any( + a["failures_in_window"] >= self._threshold for a in agents_failing + ) or len(agents_failing) >= self._threshold: + status = "broken" + else: + status = "degraded" + + return { + "status": status, + "fail_window_seconds": self._window, + "fail_threshold": self._threshold, + "alert_cooldown_seconds": self._cooldown, + "outage_started_at": self._first_failure_at or None, + "outage_age_seconds": ( + int(now - self._first_failure_at) + if self._first_failure_at + else None + ), + "alerts_sent": self._alert_count, + "last_alert_age_seconds": ( + int(now - self._last_alert_at) if self._last_alert_at else None + ), + "agents_failing": agents_failing, + } + + +# ── Operator chat resolution ──────────────────────────────────────── + + +def resolve_operator_chat( + *, + get_setting, + list_all_approved_users, +) -> tuple[str, str]: + """Pick the (chat_id, platform) that should receive operator alerts. + + Resolution order: + 1. ``operator_chat_id`` + ``operator_platform`` in system_settings. + 2. Fallback: the chat_id appearing across the most approved_users rows + (heuristic: the person with broadest access is the operator). + 3. Empty strings if nothing matches — caller must handle "no operator". + + Returns ("", "") when unresolved. Never raises. + """ + try: + chat_id = (get_setting("operator_chat_id", "") or "").strip() + platform = (get_setting("operator_platform", "") or "").strip() or "telegram" + if chat_id: + return chat_id, platform + except Exception as e: + _warn("auth_alerts: failed to read operator_chat_id setting: %s", e) + + try: + users: Iterable[dict] = list_all_approved_users() or [] + except Exception as e: + _warn("auth_alerts: failed to list approved users: %s", e) + return "", "" + + counter: Counter[str] = Counter() + for u in users: + cid = (u.get("chat_id") or "").strip() if isinstance(u, dict) else "" + if cid: + counter[cid] += 1 + if not counter: + return "", "" + + chat_id, _ = counter.most_common(1)[0] + return chat_id, "telegram" + + +# ── Alert formatting ─────────────────────────────────────────────── + + +def format_alert_message( + *, + agent_name: str, + decision: dict, + error: str, + host_label: str = "", +) -> str: + """Render the operator-alert message body. + + Plain text — _broker_send defaults to no parse_mode, so anything wrapped + in ``*`` or `` ` `` would render literally on Telegram. Keep it readable + without markdown affordances. + + Kept human-readable and short; includes enough detail for the operator + to know which host to log into and re-auth. + """ + reason = decision.get("reason", "") + agents_failing = decision.get("agents_failing", 0) + count = decision.get("count", 0) + + if reason == "host_wide": + headline = ( + f"🚨 Claude auth broken on {host_label or 'this host'}: " + f"{agents_failing} agent(s) failing" + ) + else: + headline = ( + f"🚨 Claude auth broken for {agent_name}: " + f"{count} failures in window" + ) + + detail = error.strip()[:200] or "no error detail" + instructions = ( + "Re-auth needed: SSH to the host, run 'claude' to log back in, " + "then 'sudo systemctl restart pinkybot' (or equivalent)." + ) + return f"{headline}\n\nLast error: {detail}\n\n{instructions}" diff --git a/src/pinky_daemon/broker.py b/src/pinky_daemon/broker.py index 94b9ae5c..7480c9e4 100644 --- a/src/pinky_daemon/broker.py +++ b/src/pinky_daemon/broker.py @@ -237,12 +237,15 @@ async def _start_typing(self, agent_name: str, platform: str, chat_id: str, stre _log(f"broker: typing indicator started for {agent_name}/{chat_id}") def _stop_typing(self, agent_name: str, chat_id: str) -> None: - """Stop the native typing indicator loop.""" + """Stop the native typing indicator loop. + + Safe to call defensively — no-op (and silent) when no task is active. + """ key = (agent_name, chat_id) task = self._typing_tasks.pop(key, None) if task and not task.done(): task.cancel() - _log(f"broker: typing indicator stopped for {agent_name}/{chat_id}") + _log(f"broker: typing indicator stopped for {agent_name}/{chat_id}") def _stop_all_typing(self, agent_name: str) -> None: """Stop ALL typing indicator loops for an agent (used on disconnect/stop).""" diff --git a/src/pinky_daemon/codex_session.py b/src/pinky_daemon/codex_session.py index 1a0e84e6..f1b771c0 100644 --- a/src/pinky_daemon/codex_session.py +++ b/src/pinky_daemon/codex_session.py @@ -23,7 +23,8 @@ import time from dataclasses import dataclass, field -from pinky_daemon.sessions import CHARS_PER_TOKEN, MODEL_CONTEXT_SIZES, SessionUsage +from pinky_daemon.context_estimator import ContextTextEstimator +from pinky_daemon.sessions import MODEL_CONTEXT_SIZES, SessionUsage from pinky_daemon.streaming_session import ( StreamingSessionConfig, StreamingTurnResult, @@ -96,7 +97,7 @@ def __init__( self.account_info: dict = {"apiProvider": "codex_cli"} self._on_session_id = None # async fn(agent_name, session_id) self._pending_session_id_update = "" # Set by sync _handle_event, consumed by async worker - self._internal_context_texts: list[str] = [] + self._context_estimator = ContextTextEstimator() self._current_turn_seq = 0 self._last_user_message = "" # For analytics keyword classification @@ -877,21 +878,9 @@ def max_tokens(self) -> int: @property def estimated_tokens(self) -> int: """Estimate current context size from persisted chat plus internal prompts.""" - total = sum( - max(1, len(text) // CHARS_PER_TOKEN) - for text in self._internal_context_texts - if text - ) - if not self._conversation_store: - return total - try: - history = self._conversation_store.get_history(self.id, limit=1000) - except Exception: - return total - return total + sum( - max(1, len(msg.content) // CHARS_PER_TOKEN) - for msg in history - if msg.content + return self._context_estimator.estimated_tokens( + session_id=self.id, + conversation_store=self._conversation_store, ) @property @@ -902,16 +891,11 @@ def context_used_pct(self) -> float: def get_context_info(self) -> dict: """Best-effort context info for APIs that expect session context details.""" - total = self.estimated_tokens - max_tokens = self.max_tokens - pct = round(total / max_tokens * 100, 1) if max_tokens > 0 else 0.0 - return { - "total_tokens": total, - "max_tokens": max_tokens, - "percentage": pct, - "categories": [], - "mcp_tools": [], - } + return self._context_estimator.context_info( + session_id=self.id, + conversation_store=self._conversation_store, + max_tokens=self.max_tokens, + ) @property def stats(self) -> dict: @@ -934,8 +918,7 @@ def id(self) -> str: def _record_internal_context_text(self, text: str) -> None: """Track prompts/responses that do not appear in the conversation store.""" - if text: - self._internal_context_texts.append(text) + self._context_estimator.record_internal_text(text) def _analytics_session_started(self) -> None: if not self._analytics_store: diff --git a/src/pinky_daemon/context_estimator.py b/src/pinky_daemon/context_estimator.py new file mode 100644 index 00000000..62803000 --- /dev/null +++ b/src/pinky_daemon/context_estimator.py @@ -0,0 +1,70 @@ +"""Shared context-size estimation helpers for non-SDK runtimes.""" + +from __future__ import annotations + +from pinky_daemon.sessions import CHARS_PER_TOKEN + + +class ContextTextEstimator: + """Estimate context from internal prompts plus persisted conversation text. + + Runtimes that do not expose authoritative context usage can share this + best-effort estimator instead of duplicating token-counting fallback code. + """ + + def __init__(self, *, chars_per_token: int = CHARS_PER_TOKEN) -> None: + self._chars_per_token = max(1, chars_per_token) + self._internal_context_texts: list[str] = [] + + def record_internal_text(self, text: str) -> None: + """Track prompt/response text that is not in ConversationStore.""" + if text: + self._internal_context_texts.append(text) + + def estimated_tokens( + self, + *, + session_id: str, + conversation_store=None, + history_limit: int = 1000, + ) -> int: + """Estimate tokens from internal text and optional conversation history.""" + total = sum(self._estimate_text_tokens(text) for text in self._internal_context_texts) + if not conversation_store: + return total + try: + history = conversation_store.get_history(session_id, limit=history_limit) + except Exception: + return total + return total + sum( + self._estimate_text_tokens(getattr(msg, "content", "")) + for msg in history + ) + + def context_info( + self, + *, + session_id: str, + max_tokens: int, + conversation_store=None, + history_limit: int = 1000, + ) -> dict: + """Return the context-info shape expected by streaming status APIs.""" + total = self.estimated_tokens( + session_id=session_id, + conversation_store=conversation_store, + history_limit=history_limit, + ) + pct = round(total / max_tokens * 100, 1) if max_tokens > 0 else 0.0 + return { + "total_tokens": total, + "max_tokens": max_tokens, + "percentage": pct, + "categories": [], + "mcp_tools": [], + } + + def _estimate_text_tokens(self, text: str) -> int: + if not text: + return 0 + return max(1, len(text) // self._chars_per_token) diff --git a/src/pinky_daemon/pollers.py b/src/pinky_daemon/pollers.py index 222a63ca..ee8de6ab 100644 --- a/src/pinky_daemon/pollers.py +++ b/src/pinky_daemon/pollers.py @@ -9,10 +9,22 @@ import asyncio import sys +from datetime import datetime, timezone +from typing import TYPE_CHECKING from pinky_daemon.message_handler import InboundMessage, MessageHandler +from pinky_outreach.discord import DiscordAdapter, DiscordError, DiscordRateLimited from pinky_outreach.telegram import TelegramAdapter, TelegramError +# Threshold below which a "most recent message" found during channel-priming +# is treated as a real first-test inbound rather than as a replay-prevention +# floor. Discord first-contact UX: if a user sends a test message within this +# window before discovery sweeps, we still want to deliver it. +_PRIME_FRESH_WINDOW_SECONDS = 30.0 + +if TYPE_CHECKING: + from pinky_outreach.types import Chat + class TelegramPoller: """Polls Telegram Bot API for new messages. @@ -386,5 +398,358 @@ def is_running(self) -> bool: return self._running +class BrokerDiscordPoller: + """Polls Discord REST API for a specific agent's bot token, routes through MessageBroker. + + This is the Discord analogue of BrokerTelegramPoller, but using REST polling + rather than long-polling — the Discord REST API has no equivalent to + Telegram's getUpdates. A future version may add a Gateway/WebSocket poller + for true push delivery; this class provides a working inbound channel today. + + Behavior: + - Discovers watchable channels at startup (settings.watched_channels if set, + otherwise auto-discovered text channels across all the bot's guilds). + - Polls each channel every `poll_interval` seconds, fetching only messages + with id > last seen via the `?after=` parameter. + - On first sight of a channel, primes `last_id` from the most recent + message so we don't replay history. + - Skips messages authored by the bot itself (no self-reply loop). + - Honors 429 rate limits via DiscordRateLimited (sleeps retry_after). + - Re-discovers channels every `discovery_interval` seconds so newly-joined + guilds become reachable without a daemon restart. + """ + + def __init__( + self, + adapter: DiscordAdapter, + agent_name: str, + broker, # MessageBroker + registry=None, # AgentRegistry — reserved for future guild tracking + *, + poll_interval: float = 1.0, + discovery_interval: float = 60.0, + watched_channels: list[str] | None = None, + event_callback=None, + ) -> None: + from pinky_daemon.broker import BrokerMessage, MessageBroker + self._BrokerMessage = BrokerMessage + + self._adapter = adapter + self._agent_name = agent_name + self._broker: MessageBroker = broker + self._registry = registry + self._poll_interval = max(0.25, float(poll_interval)) + self._discovery_interval = max(10.0, float(discovery_interval)) + self._configured_channels = list(watched_channels or []) + self._event_callback = event_callback + self._running = False + self._poll_count = 0 + self._bot_user_id = "" + self._bot_username = "" + # Per-channel state + self._channels: list[str] = [] + self._last_id: dict[str, str] = {} + self._last_discovery: float = 0.0 + # Channel metadata cache — invalidated each discovery cycle (~60s by + # default). Avoids fanning out get_channel() calls across every message + # in a burst on the same channel. + self._channel_info_cache: dict[str, Chat] = {} + + @property + def agent_name(self) -> str: + return self._agent_name + + @property + def poll_count(self) -> int: + return self._poll_count + + @property + def is_running(self) -> bool: + return self._running + + @property + def watched_channels(self) -> list[str]: + return list(self._channels) + + async def start(self) -> None: + """Start the polling loop.""" + self._running = True + _log(f"discord-poller[{self._agent_name}]: starting") + + # Verify bot connection + try: + me = await asyncio.get_running_loop().run_in_executor( + None, self._adapter.get_me, + ) + self._bot_user_id = me.get("id", "") + self._bot_username = me.get("username", "?") + _log( + f"discord-poller[{self._agent_name}]: connected as " + f"{self._bot_username} (id={self._bot_user_id})" + ) + except DiscordError as e: + _log(f"discord-poller[{self._agent_name}]: failed to connect: {e}") + self._running = False + return + + # Initial channel discovery + last_id priming + await self._refresh_channels(verbose=True) + + while self._running: + try: + # Periodic re-discovery (cheap — one /users/@me/guilds + one + # /guilds/{id}/channels per guild per discovery_interval). + import time as _time + if _time.monotonic() - self._last_discovery >= self._discovery_interval: + await self._refresh_channels(verbose=False) + + await self._poll_once() + except DiscordRateLimited as e: + _log( + f"discord-poller[{self._agent_name}]: rate limited, " + f"sleeping {e.retry_after:.2f}s" + ) + await asyncio.sleep(min(e.retry_after, 30.0)) + except DiscordError as e: + _log(f"discord-poller[{self._agent_name}]: error: {e}") + await asyncio.sleep(5) + except Exception as e: + _log(f"discord-poller[{self._agent_name}]: unexpected error: {e}") + await asyncio.sleep(5) + + await asyncio.sleep(self._poll_interval) + + async def _refresh_channels(self, *, verbose: bool) -> None: + """Refresh the watched-channel set and prime last_id for newcomers. + + `verbose=True` produces a log line on every call (used at startup); + otherwise we only log on add/remove changes. + """ + import time as _time + loop = asyncio.get_running_loop() + + if self._configured_channels: + new_set = list(self._configured_channels) + else: + try: + new_set = await loop.run_in_executor( + None, self._adapter.discover_text_channels, + ) + except DiscordError as e: + _log( + f"discord-poller[{self._agent_name}]: " + f"channel discovery failed: {e}" + ) + # Keep existing set on transient discovery failure. + self._last_discovery = _time.monotonic() + return + + added = [c for c in new_set if c not in self._last_id] + removed = [c for c in self._channels if c not in new_set] + + # Prime last_id for newly-discovered channels: fetch the most recent + # message and treat its id as the baseline so we don't replay history. + # Exception: if that message is a fresh (< _PRIME_FRESH_WINDOW_SECONDS) + # non-bot message, treat it as a real first-test inbound — back the + # floor up by 1 snowflake so the next poll picks it up and delivers it. + # This avoids the "user sends test, discovery silently swallows it" + # first-contact UX trap. (Discord IDs are monotonic snowflakes; + # `?after=` resolves to "messages with id > id-1", i.e. this + # message itself.) + for ch in added: + try: + recent = await loop.run_in_executor( + None, + lambda c=ch: self._adapter.get_messages(c, limit=1), + ) + except DiscordError: + # Channel might be unreadable; skip it for now, retry next discovery + continue + if not recent: + # Empty channel — start from "0", which sorts before any real snowflake + self._last_id[ch] = "0" + _log( + f"discord-poller[{self._agent_name}]: primed channel {ch} " + f"(empty — first message will be delivered)" + ) + continue + + # get_messages returns newest-first + msg = recent[0] + now = datetime.now(timezone.utc) + try: + age_sec = (now - msg.timestamp).total_seconds() + except (TypeError, ValueError): + age_sec = float("inf") + is_bot = bool((msg.metadata or {}).get("is_bot", False)) + + if age_sec < _PRIME_FRESH_WINDOW_SECONDS and not is_bot: + # Fresh first-contact message — back the floor up by 1 so + # the next poll fetches and delivers it. + try: + backed_off = str(int(msg.message_id) - 1) + except (TypeError, ValueError): + # Non-numeric id (shouldn't happen on Discord) — fall back + # to using the message itself as the floor. + backed_off = msg.message_id + self._last_id[ch] = backed_off + _log( + f"discord-poller[{self._agent_name}]: primed channel {ch} " + f"with floor {backed_off} (fresh msg {msg.message_id} from " + f"{msg.sender}, age {age_sec:.0f}s — will deliver on next poll)" + ) + else: + self._last_id[ch] = msg.message_id + _log( + f"discord-poller[{self._agent_name}]: primed channel {ch} " + f"with floor {msg.message_id} (most-recent msg age " + f"{age_sec:.0f}s, is_bot={is_bot} — first message AFTER " + f"this will be delivered)" + ) + + for ch in removed: + self._last_id.pop(ch, None) + + self._channels = new_set + self._last_discovery = _time.monotonic() + + # Drop cached channel metadata so renames / type changes get picked up + # on the next inbound. Cheap (≤ N discovery_interval-old entries). + self._channel_info_cache.clear() + + if added or removed or verbose: + _log( + f"discord-poller[{self._agent_name}]: watching " + f"{len(self._channels)} channels (+{len(added)} -{len(removed)})" + ) + + async def _resolve_channel_info(self, channel_id: str): + """Return cached channel metadata or fetch + cache it on miss.""" + cached = self._channel_info_cache.get(channel_id) + if cached is not None: + return cached + try: + chat_info = await asyncio.get_running_loop().run_in_executor( + None, lambda c=channel_id: self._adapter.get_channel(c), + ) + except DiscordError: + return None + self._channel_info_cache[channel_id] = chat_info + return chat_info + + def _attach_broker_failure_logger(self, task: "asyncio.Task") -> None: + """Surface unhandled broker delivery exceptions as poller log lines.""" + def _log_failure(t: "asyncio.Task") -> None: + if t.cancelled(): + return + exc = t.exception() + if exc is None: + return + _log( + f"discord-poller[{self._agent_name}]: " + f"broker delivery failed: {exc!r}" + ) + task.add_done_callback(_log_failure) + + async def _poll_once(self) -> None: + """Single sweep across all watched channels.""" + loop = asyncio.get_running_loop() + self._poll_count += 1 + + for channel_id in list(self._channels): + after = self._last_id.get(channel_id, "") + if after == "0": + # Empty-channel sentinel — drop the after filter so the first real + # message is picked up regardless of snowflake ordering. + after = "" + + try: + messages = await loop.run_in_executor( + None, + lambda c=channel_id, a=after: self._adapter.get_messages( + c, limit=50, after=a, + ), + ) + except DiscordRateLimited: + # Bubble up so the outer loop can sleep retry_after + raise + except DiscordError as e: + _log( + f"discord-poller[{self._agent_name}]: " + f"channel {channel_id} fetch failed: {e}" + ) + continue + + if not messages: + continue + + # Discord returns newest-first; replay in chronological order so + # the broker sees them as they happened. + messages = list(reversed(messages)) + + # Channel metadata is constant per channel per poll cycle (and + # essentially constant across the whole discovery_interval — + # renames are rare). Resolve once per channel per sweep instead + # of fanning out N get_channel calls in the per-message loop. + chat_info = await self._resolve_channel_info(channel_id) + chat_title = chat_info.title if chat_info and chat_info.title else "" + is_group = bool(chat_info and chat_info.chat_type != "dm") + + for msg in messages: + # Track high-water mark even for skipped messages so we don't + # re-fetch them next tick. + self._last_id[channel_id] = msg.message_id + + meta = msg.metadata or {} + if meta.get("is_bot"): + continue + if meta.get("author_id") and meta["author_id"] == self._bot_user_id: + continue + + broker_msg = self._BrokerMessage( + platform="discord", + chat_id=channel_id, + sender_name=msg.sender, + sender_id=meta.get("author_id", ""), + content=msg.content, + agent_name=self._agent_name, + message_id=msg.message_id, + chat_title=chat_title, + is_group=is_group, + reply_to=msg.reply_to, + metadata=meta, + attachments=meta.get("attachments", []), + ) + + _log( + f"discord-poller[{self._agent_name}]: message from " + f"{msg.sender} in {channel_id}: {msg.content[:50]}..." + ) + + delivery = asyncio.create_task( + self._broker.handle_inbound(broker_msg) + ) + self._attach_broker_failure_logger(delivery) + + if self._event_callback: + try: + await self._event_callback( + platform="discord", + chat_id=str(channel_id), + sender=msg.sender, + content=msg.content, + ) + except Exception as e: + _log( + f"discord-poller[{self._agent_name}]: " + f"event callback error: {e}" + ) + + def stop(self) -> None: + """Stop the polling loop.""" + self._running = False + _log(f"discord-poller[{self._agent_name}]: stopping") + + def _log(msg: str) -> None: print(msg, file=sys.stderr, flush=True) diff --git a/src/pinky_daemon/scheduler.py b/src/pinky_daemon/scheduler.py index c9c0bff6..e5603fb1 100644 --- a/src/pinky_daemon/scheduler.py +++ b/src/pinky_daemon/scheduler.py @@ -35,6 +35,9 @@ def _log(msg: str) -> None: def _is_claude_code_agent(agent, registry: AgentRegistry) -> bool: """Return True if this agent runs on Claude Code (not Codex or other provider).""" + runtime = (getattr(agent, "runtime", "") or "").strip() + if runtime: + return runtime == "claude_sdk" try: from pinky_daemon.api import resolve_provider_config url, _, _ = resolve_provider_config( diff --git a/src/pinky_daemon/streaming_session.py b/src/pinky_daemon/streaming_session.py index fc6bfabd..e0d238ba 100644 --- a/src/pinky_daemon/streaming_session.py +++ b/src/pinky_daemon/streaming_session.py @@ -112,6 +112,40 @@ def _is_outreach_tool(tool_name: str) -> bool: return _tool_basename(tool_name) in _OUTREACH_TOOL_NAMES +# Auth-failure detection — uses native SDK types (claude-agent-sdk >= 0.1.76). +# +# Two paths surface credential failures, and we check both: +# +# 1. ``AssistantMessage.error`` is an ``AssistantMessageError`` Literal whose +# only credential-failure value is "authentication_failed". The other +# Literal members (billing_error, rate_limit, invalid_request, +# server_error, unknown) are NOT auth issues and must NOT trip the +# operator alert — billing errors and rate limits in particular would +# spam the operator on perfectly authenticated sessions. +# +# 2. ``ResultMessage.api_error_status`` is the raw HTTP status of a failing +# API call (added in SDK 0.1.76, emitted by CLI >= 2.1.110). 401 and 403 +# are credential failures; 429 and 5xx are transient/operational and +# handled separately. +# +# This replaces an older substring-tuple match against ``msg.error`` that +# pre-dated the Literal type and over-matched (e.g. "permission_error" is +# never a value the SDK can produce). +_AUTH_ASSISTANT_ERROR = "authentication_failed" +_AUTH_HTTP_STATUSES = frozenset({401, 403}) + + +def _is_auth_error_assistant(msg) -> bool: + """True if an ``AssistantMessage`` indicates a credential failure.""" + return getattr(msg, "error", None) == _AUTH_ASSISTANT_ERROR + + +def _is_auth_error_result(result) -> bool: + """True if a ``ResultMessage``'s HTTP status indicates a credential failure.""" + status = getattr(result, "api_error_status", None) + return status in _AUTH_HTTP_STATUSES + + def _describe_tool_use(tool_name: str, tool_input: dict) -> str: """Build a human-readable description of a tool invocation.""" name = _tool_basename(tool_name) @@ -174,6 +208,8 @@ def __init__( cost_callback=None, # fn(agent_name, cost_usd, input_tokens, output_tokens, session_id) analytics_store=None, registry=None, # AgentRegistry — for server-side presence stamping + auth_alert_callback=None, # async fn(agent_name, error_str) — fires on auth_failed + auth_success_callback=None, # fn(agent_name) — fires on a successful turn (clears auth fail state) ) -> None: self._config = config self._response_callback = response_callback @@ -181,6 +217,11 @@ def __init__( self._conversation_store = conversation_store self._analytics_store = analytics_store self._registry = registry + # Auth-failure detection plumbing — called from the reader loop when + # the SDK reports an authentication error so the daemon can alert + # the operator. See pinky_daemon/auth_alerts.py for the tracker. + self._auth_alert_callback = auth_alert_callback + self._auth_success_callback = auth_success_callback self._client = None self._reader_task: asyncio.Task | None = None self._connected = False @@ -412,6 +453,7 @@ async def _reader_loop(self) -> None: """Background loop that reads responses and fires callbacks.""" from claude_agent_sdk.types import ( AssistantMessage, + AssistantMessageError, ResultMessage, TextBlock, ThinkingBlock, @@ -419,9 +461,30 @@ async def _reader_loop(self) -> None: ToolUseBlock, ) + # Defensive invariant: ``_is_auth_error_assistant`` does an exact + # match against ``AssistantMessageError`` for "authentication_failed". + # If a future SDK release renames that Literal value, exact-match + # silently stops detecting credential failures — re-creating the + # exact regression mode #400 was built to catch. Fail loud at session + # start instead. (CI also asserts this in test_auth_alerts.py, so + # bumps caught in PR; this guard catches local-dev SDK upgrades.) + assert _AUTH_ASSISTANT_ERROR in AssistantMessageError.__args__, ( + f"claude-agent-sdk renamed AssistantMessageError Literal — " + f"_AUTH_ASSISTANT_ERROR={_AUTH_ASSISTANT_ERROR!r} no longer in " + f"{AssistantMessageError.__args__}. Update the constant and tests." + ) + _log(f"streaming[{self.agent_name}]: reader loop running") turn_tool_uses = [] # Track tool uses per turn turn_thinking: list[str] = [] # Track thinking blocks per turn + # Per-turn dedupe for auth-failure callbacks. A single failed turn can + # surface auth errors on BOTH paths: an AssistantMessage with + # error="authentication_failed", followed by the terminal ResultMessage + # with api_error_status=401. Without dedupe, AuthFailureTracker would + # increment twice for one real failure — tripping the operator-alert + # threshold early and skewing the multi-agent baseline. Reset at the + # end of ResultMessage handling (turn boundary). + auth_reported_this_turn = False try: async for msg in self._client.receive_messages(): @@ -439,6 +502,27 @@ async def _reader_loop(self) -> None: f"streaming[{self.agent_name}]: assistant error={msg.error!r}" f" stop_reason={msg.stop_reason!r} — suppressing content" ) + # Detect auth failures and notify the operator. The SDK + # types ``msg.error`` as the AssistantMessageError + # Literal; only "authentication_failed" is a credential + # issue. Other Literal values (billing_error, rate_limit, + # invalid_request, server_error, unknown) are NOT auth + # failures and must not trip the operator alert. + if _is_auth_error_assistant(msg) and self._auth_alert_callback: + # Set the dedupe flag BEFORE invoking the callback + # so a callback exception can't cause the + # ResultMessage path to double-fire for the same + # turn. + auth_reported_this_turn = True + try: + await self._auth_alert_callback( + self.agent_name, str(msg.error) + ) + except Exception as exc: + _log( + f"streaming[{self.agent_name}]: " + f"auth_alert_callback raised: {exc}" + ) # Don't touch _last_response; fall through to usage/session_id capture. else: # Extract text and tool uses from content blocks @@ -534,8 +618,42 @@ async def _reader_loop(self) -> None: _log( f"streaming[{self.agent_name}]: error result" f" stop_reason={msg.stop_reason!r}" + f" api_error_status={getattr(msg, 'api_error_status', None)!r}" f" errors={msg.errors!r} — suppressing forwarded response" ) + # Detect credential failures on the result path too. The + # AssistantMessage path catches errors the SDK surfaces + # mid-turn; this path catches errors that only land at + # turn completion (api_error_status added in 0.1.76: + # 401/403 = bad creds; 429/5xx = transient, handled + # below as a generic error result). + # + # Skip if the AssistantMessage path already reported + # auth for this turn — both paths firing for one real + # failure double-counts in AuthFailureTracker. + if ( + _is_auth_error_result(msg) + and self._auth_alert_callback + and not auth_reported_this_turn + ): + auth_reported_this_turn = True + # Surface msg.errors into the callback string when + # present — operators get richer triage context + # than the raw status code alone (the SDK started + # returning actionable messages in 0.1.77). + err_detail = f"api_error_status={msg.api_error_status}" + if msg.errors: + err_detail += f" errors={msg.errors!r}" + try: + await self._auth_alert_callback( + self.agent_name, + err_detail, + ) + except Exception as exc: + _log( + f"streaming[{self.agent_name}]: " + f"auth_alert_callback (result) raised: {exc}" + ) # Analytics: still record errored turns _u = msg.usage or {} self._analytics_log_turn_usage( @@ -570,6 +688,8 @@ async def _reader_loop(self) -> None: turn_thinking = [] self._stats["turns"] += 1 self.last_active = time.time() + # Reset per-turn auth dedupe — turn boundary + auth_reported_this_turn = False continue # Turn complete — fire response callback @@ -596,6 +716,15 @@ async def _reader_loop(self) -> None: except Exception as e: _log(f"streaming[{self.agent_name}]: callback error: {e}") + # A successful (non-errored) turn proves Claude auth is + # working again — clear any auth-fail tracking for this + # agent so the next outage emits a fresh alert. + if self._auth_success_callback: + try: + self._auth_success_callback(self.agent_name) + except Exception: + pass + # Track usage from result if msg.total_cost_usd: self.usage.total_cost_usd += msg.total_cost_usd @@ -712,6 +841,10 @@ async def _reader_loop(self) -> None: turn_thinking = [] # Reset for next turn self._stats["turns"] += 1 self.last_active = time.time() + # Reset per-turn auth dedupe — turn boundary. Successful + # turns rarely set this flag (no auth error fired), but + # reset unconditionally to keep the invariant simple. + auth_reported_this_turn = False _log(f"streaming[{self.agent_name}]: turn complete (total: {self._stats['turns']})") diff --git a/src/pinky_outreach/discord.py b/src/pinky_outreach/discord.py index 1d73d6df..ff4cdb1e 100644 --- a/src/pinky_outreach/discord.py +++ b/src/pinky_outreach/discord.py @@ -11,12 +11,18 @@ from __future__ import annotations import os +import time from datetime import datetime import httpx from pinky_outreach.types import Chat, Message, Platform +# Discord requires a User-Agent that follows the convention; default urllib/httpx +# UAs trigger Cloudflare 403s on some endpoints. See +# https://discord.com/developers/docs/reference#user-agent +_DEFAULT_USER_AGENT = "DiscordBot (https://github.com/olegbrok/PinkyBot, 0.1.0)" + class DiscordError(Exception): """Discord API error.""" @@ -26,39 +32,98 @@ def __init__(self, message: str, status_code: int = 0): super().__init__(f"Discord API error {status_code}: {message}") +class DiscordRateLimited(DiscordError): # noqa: N818 — name reads naturally at call sites + """Discord 429 — caller may retry after `retry_after` seconds.""" + + def __init__(self, retry_after: float, message: str = "rate limited"): + self.retry_after = retry_after + super().__init__(message, status_code=429) + + class DiscordAdapter: - """Discord REST API adapter using httpx.""" + """Discord REST API adapter using httpx. + + Synchronous: uses `httpx.Client` and `time.sleep` for 429 backoff. Callers + in async contexts must wrap calls in `loop.run_in_executor` to avoid + blocking the event loop. The bundled `BrokerDiscordPoller` does this + correctly; ad-hoc async callers must too. A future v0.2 may swap to + `httpx.AsyncClient` + `asyncio.sleep` to remove the requirement. + """ BASE_URL = "https://discord.com/api/v10" - def __init__(self, bot_token: str, *, timeout: float = 30.0) -> None: + def __init__( + self, + bot_token: str, + *, + timeout: float = 30.0, + user_agent: str = _DEFAULT_USER_AGENT, + max_429_retries: int = 1, + ) -> None: self._token = bot_token + self._max_429_retries = max_429_retries self._client = httpx.Client( base_url=self.BASE_URL, headers={ "Authorization": f"Bot {bot_token}", + "User-Agent": user_agent, }, timeout=timeout, ) self._bot_user: dict | None = None + # Cache user_id → DM channel_id resolutions so we only call + # POST /users/@me/channels once per recipient. + self._dm_channel_cache: dict[str, str] = {} def close(self) -> None: self._client.close() def _request(self, method: str, path: str, **kwargs) -> dict | list: - """Make a Discord API request.""" - resp = self._client.request(method, path, **kwargs) + """Make a Discord API request, with bounded 429 retry. - if resp.status_code == 204: - return {} - - data = resp.json() - - if resp.status_code >= 400: - msg = data.get("message", str(data)) - raise DiscordError(msg, resp.status_code) - - return data + Synchronous: blocks via `time.sleep` on 429 backoff. Async callers + must wrap in `run_in_executor` (see class docstring). + """ + attempts = 0 + while True: + resp = self._client.request(method, path, **kwargs) + + if resp.status_code == 429: + # Discord returns retry_after either as a JSON body field (seconds) + # or in the X-RateLimit-Reset-After header. Prefer the body. + retry_after = 1.0 + try: + body = resp.json() + retry_after = float(body.get("retry_after", retry_after)) + except Exception: + header = resp.headers.get("X-RateLimit-Reset-After") + if header: + try: + retry_after = float(header) + except ValueError: + pass + + if attempts >= self._max_429_retries: + raise DiscordRateLimited(retry_after) + attempts += 1 + time.sleep(min(retry_after, 5.0)) + continue + + if resp.status_code == 204: + return {} + + try: + data = resp.json() + except Exception: + if resp.status_code >= 400: + raise DiscordError(resp.text[:200], resp.status_code) + return {} + + if resp.status_code >= 400: + msg = data.get("message", str(data)) if isinstance(data, dict) else str(data) + raise DiscordError(msg, resp.status_code) + + return data # ── Actions ────────────────────────────────────────────── @@ -68,6 +133,24 @@ def send_typing(self, channel_id: str) -> None: # ── Sending ────────────────────────────────────────────── + def open_dm_channel(self, user_id: str) -> str: + """Open (or look up cached) a DM channel with a user. Returns channel_id. + + Discord requires explicitly opening a DM via POST /users/@me/channels + with `{recipient_id: user_id}`. The result is a channel_id that can + then be used with send_message / etc. The mapping is stable per bot, + so we cache it. + """ + cached = self._dm_channel_cache.get(user_id) + if cached: + return cached + result = self._request( + "POST", "/users/@me/channels", json={"recipient_id": user_id}, + ) + channel_id = result["id"] + self._dm_channel_cache[user_id] = channel_id + return channel_id + def send_message( self, channel_id: str, @@ -75,12 +158,33 @@ def send_message( *, reply_to: str = "", ) -> Message: - """Send a message to a Discord channel.""" + """Send a message to a Discord channel. + + If `channel_id` looks like a user_id (Discord returns 404 Unknown + Channel), transparently opens a DM channel with that user and + retries — this lets callers pass user_ids for owner-notify flows + where they only know the recipient's user_id. + """ payload: dict = {"content": content} if reply_to: payload["message_reference"] = {"message_id": reply_to} - result = self._request("POST", f"/channels/{channel_id}/messages", json=payload) + try: + result = self._request( + "POST", f"/channels/{channel_id}/messages", json=payload, + ) + except DiscordError as e: + if e.status_code == 404 and "Unknown Channel" in str(e): + # Treat channel_id as a user_id and try DMing them. + # Bounded — open_dm_channel will surface its own error + # if user_id is also invalid; we don't loop again. + dm_channel_id = self.open_dm_channel(channel_id) + result = self._request( + "POST", f"/channels/{dm_channel_id}/messages", json=payload, + ) + channel_id = dm_channel_id + else: + raise return Message( platform=Platform.discord, @@ -249,6 +353,38 @@ def get_guild_channels(self, guild_id: str) -> list[Chat]: for ch in results ] + def get_my_guilds(self) -> list[dict]: + """List guilds (servers) the bot is currently a member of. + + Returns minimal guild dicts: {id, name, owner, permissions, ...}. + Used by the inbound poller to auto-discover channels to watch. + """ + result = self._request("GET", "/users/@me/guilds") + return result if isinstance(result, list) else [] + + def discover_text_channels(self) -> list[str]: + """Auto-discover all text channels (type 0) the bot can access across all guilds. + + Returns a list of channel IDs. Used as a default watch-set when no + explicit channel list is configured in the bot token settings. + """ + # Discord channel types: 0=GUILD_TEXT, 5=GUILD_ANNOUNCEMENT. + # We treat both as message-bearing for inbound polling purposes. + watchable_types = {0, 5} + channel_ids: list[str] = [] + for guild in self.get_my_guilds(): + try: + raw = self._request("GET", f"/guilds/{guild['id']}/channels") + except DiscordError: + # Skip guilds where listing fails (e.g., transient permission issues). + continue + if not isinstance(raw, list): + continue + for ch in raw: + if ch.get("type") in watchable_types: + channel_ids.append(ch["id"]) + return channel_ids + # ── Reactions ──────────────────────────────────────────── def add_reaction( diff --git a/src/pinky_self/server.py b/src/pinky_self/server.py index 216aa2e2..897465aa 100644 --- a/src/pinky_self/server.py +++ b/src/pinky_self/server.py @@ -1933,11 +1933,32 @@ def check_for_updates() -> str: return "\n".join(parts) @mcp.tool() - def update_and_restart(branch: str = "") -> str: - """Pull latest code, rebuild if needed, and restart the daemon.""" + def update_and_restart( + branch: str = "", + force: bool = False, + force_deps: bool = False, + ) -> str: + """Pull latest code, rebuild if needed, and restart the daemon. + + force=True discards local mods to TRACKED files before pulling. + Use to recover from a dirty working tree (e.g. stale build artifacts). + Untracked files are preserved. + + force_deps=True reinstalls dependencies even when git HEAD didn't + move. Useful when installed package versions have drifted from + pyproject.toml (e.g., daemon seeded from a stale image, or a + previous deploy skipped the pip install step). + """ url = "/admin/update" + params = [] if branch: - url += f"?branch={branch}" + params.append(f"branch={branch}") + if force: + params.append("force=true") + if force_deps: + params.append("force_deps=true") + if params: + url += "?" + "&".join(params) result = _api("POST", url) if "error" in result: return f"Update failed: {result['error']}" @@ -1951,6 +1972,14 @@ def update_and_restart(branch: str = "") -> str: else: parts = [f"Update applied: {result.get('before_hash', '?')} -> {result.get('after_hash', '?')}"] + if result.get("forced_reset"): + forced_files = result.get("forced_files", []) + parts.append(f"\nForce reset: discarded local mods to {len(forced_files)} tracked file(s).") + for f in forced_files[:5]: + parts.append(f" - {f}") + if len(forced_files) > 5: + parts.append(f" ... and {len(forced_files) - 5} more") + commits = result.get("commits", []) if commits: parts.append(f"\nChanges ({len(commits)} commit(s)):") @@ -1959,6 +1988,8 @@ def update_and_restart(branch: str = "") -> str: if result.get("deps_rebuilt"): parts.append("\nDependencies rebuilt.") + if result.get("deps_error"): + parts.append(f"\nDeps rebuild error: {result['deps_error']}") if result.get("frontend_rebuilt"): parts.append("Frontend rebuilt.") diff --git a/tests/test_admin_update.py b/tests/test_admin_update.py new file mode 100644 index 00000000..f34362a0 --- /dev/null +++ b/tests/test_admin_update.py @@ -0,0 +1,349 @@ +"""Tests for POST /admin/update — covers force flag behavior.""" + +from __future__ import annotations + +import os +import subprocess as sp +import tempfile +from unittest.mock import patch + +from fastapi.testclient import TestClient + + +def _make_client(): + from pinky_daemon.api import create_api + fd, path = tempfile.mkstemp(suffix=".db") + os.close(fd) + app = create_api(max_sessions=10, default_working_dir="/tmp", db_path=path) + return TestClient(app) + + +class _GitMock: + """Programmable subprocess.check_output side_effect for git commands. + + Records every call so tests can assert on order/presence. + """ + + def __init__( + self, + *, + dirty_files: list[str] | None = None, + before_hash: str = "abc1234", + after_hash: str = "def5678", + branch: str = "beta", + ): + self.calls: list[list[str]] = [] + self.dirty_files = dirty_files or [] + self.before_hash = before_hash + self.after_hash = after_hash + self.branch = branch + self._hash_call = 0 + + def __call__(self, cmd, **kwargs): + self.calls.append(list(cmd)) + + # git rev-parse --short HEAD — first call returns before, second returns after + if cmd[:3] == ["git", "rev-parse", "--short"]: + self._hash_call += 1 + val = self.before_hash if self._hash_call == 1 else self.after_hash + return (val + "\n").encode() + + # git describe --tags --exact-match HEAD — pretend untagged + if cmd[:2] == ["git", "describe"]: + raise sp.CalledProcessError(128, cmd, output=b"") + + # git fetch origin --tags + if cmd[:3] == ["git", "fetch", "origin"]: + return b"" + + # git tag --sort=... -l ... (only for main / use_release_tags) + if cmd[:2] == ["git", "tag"]: + return b"" + + # git rev-parse --is-shallow-repository + if cmd[:3] == ["git", "rev-parse", "--is-shallow-repository"]: + return b"false\n" + + # git rev-parse --abbrev-ref HEAD + if cmd[:3] == ["git", "rev-parse", "--abbrev-ref"]: + return (self.branch + "\n").encode() + + # git diff --name-only HEAD (used by force-mode dirty check) + if cmd[:4] == ["git", "diff", "--name-only", "HEAD"]: + joined = "\n".join(self.dirty_files) + if joined: + joined += "\n" + return joined.encode() + + # git checkout -- . (force reset of tracked files) + if cmd[:3] == ["git", "checkout", "--"]: + return b"" + + # git checkout (used when on detached HEAD / wrong branch) + if cmd[:2] == ["git", "checkout"]: + return b"" + + # git pull origin + if cmd[:3] == ["git", "pull", "origin"]: + return b"" + + # git log --oneline ... (commit summary or dry-run pending list) + if cmd[:3] == ["git", "log", "--oneline"]: + return b"abc1234 feat: example\n" + + # git diff --name-only -- pyproject.toml + if cmd[:3] == ["git", "diff", "--name-only"]: + return b"" + + return b"" + + def did_force_reset(self) -> bool: + return any(c[:4] == ["git", "checkout", "--", "."] for c in self.calls) + + def did_pull(self) -> bool: + return any(c[:3] == ["git", "pull", "origin"] for c in self.calls) + + def did_clean(self) -> bool: + return any(c[:2] == ["git", "clean"] for c in self.calls) + + +class TestAdminUpdateForce: + """Verify the force=True flag from task #77.""" + + def test_force_false_is_default(self): + """No force param → no `git checkout -- .` is invoked.""" + gm = _GitMock(dirty_files=["frontend-dist/index.html"]) + with ( + patch("subprocess.check_output", side_effect=gm), + patch("shutil.which", return_value=None), + patch("os.kill"), + ): + client = _make_client() + r = client.post("/admin/update?branch=beta") + assert r.status_code == 200 + body = r.json() + assert body.get("updated") is True + assert body.get("forced_reset") is False + assert body.get("forced_files") == [] + assert not gm.did_force_reset() + assert gm.did_pull() + + def test_force_true_resets_dirty_tracked_files(self): + """force=True with a dirty tree → checkout -- . runs before pull.""" + dirty = ["frontend-dist/index.html", "frontend-dist/assets/app.js"] + gm = _GitMock(dirty_files=dirty) + with ( + patch("subprocess.check_output", side_effect=gm), + patch("shutil.which", return_value=None), + patch("os.kill"), + ): + client = _make_client() + r = client.post("/admin/update?branch=beta&force=true") + assert r.status_code == 200 + body = r.json() + assert body.get("forced_reset") is True + assert body.get("forced_files") == dirty + assert gm.did_force_reset() + + # Critical ordering: reset must happen BEFORE the pull + checkout_idx = next( + i for i, c in enumerate(gm.calls) if c[:4] == ["git", "checkout", "--", "."] + ) + pull_idx = next( + i for i, c in enumerate(gm.calls) if c[:3] == ["git", "pull", "origin"] + ) + assert checkout_idx < pull_idx, "force reset must precede git pull" + + def test_force_true_clean_tree_no_reset(self): + """force=True but tree is clean → no checkout invoked, forced_reset=False.""" + gm = _GitMock(dirty_files=[]) + with ( + patch("subprocess.check_output", side_effect=gm), + patch("shutil.which", return_value=None), + patch("os.kill"), + ): + client = _make_client() + r = client.post("/admin/update?branch=beta&force=true") + assert r.status_code == 200 + body = r.json() + assert body.get("forced_reset") is False + assert body.get("forced_files") == [] + assert not gm.did_force_reset() + assert gm.did_pull() + + def test_force_never_runs_git_clean(self): + """Untracked files must be preserved — never invoke `git clean`.""" + gm = _GitMock(dirty_files=["some/file.py"]) + with ( + patch("subprocess.check_output", side_effect=gm), + patch("shutil.which", return_value=None), + patch("os.kill"), + ): + client = _make_client() + client.post("/admin/update?branch=beta&force=true") + assert not gm.did_clean(), "force must not delete untracked files" + + def test_force_ignored_in_dry_run(self): + """dry_run=True returns before the destructive force block runs.""" + gm = _GitMock(dirty_files=["frontend-dist/index.html"]) + with ( + patch("subprocess.check_output", side_effect=gm), + patch("shutil.which", return_value=None), + ): + client = _make_client() + r = client.post("/admin/update?branch=beta&force=true&dry_run=true") + assert r.status_code == 200 + body = r.json() + assert body.get("dry_run") is True + # No destructive ops in dry_run, regardless of force + assert not gm.did_force_reset() + assert not gm.did_pull() + + +class TestAdminUpdateBaseline: + """Sanity coverage for the existing endpoint paths.""" + + def test_dry_run_reports_pending_commits(self): + gm = _GitMock(dirty_files=[]) + with ( + patch("subprocess.check_output", side_effect=gm), + patch("shutil.which", return_value=None), + ): + client = _make_client() + r = client.post("/admin/update?branch=beta&dry_run=true") + assert r.status_code == 200 + body = r.json() + assert body.get("dry_run") is True + assert body.get("branch") == "beta" + # _GitMock returns one canned "feat: example" commit for git log + assert body.get("pending_commits") == 1 + assert body.get("up_to_date") is False + + def test_pull_failure_returns_error(self): + """If git pull errors, endpoint returns {'error': ...}.""" + + def fail_on_pull(cmd, **kwargs): + if cmd[:3] == ["git", "pull", "origin"]: + raise sp.CalledProcessError(1, cmd, output=b"merge conflict in foo") + # Delegate everything else to a clean mock + return _GitMock(dirty_files=[])(cmd, **kwargs) + + with ( + patch("subprocess.check_output", side_effect=fail_on_pull), + patch("shutil.which", return_value=None), + patch("os.kill"), + ): + client = _make_client() + r = client.post("/admin/update?branch=beta") + assert r.status_code == 200 + body = r.json() + assert "error" in body + assert "git pull failed" in body["error"] + + +class TestAdminUpdateForceDepsIntegration: + """force_deps (PR #323) and force (PR #390) are orthogonal — verify they cooperate.""" + + def _track_pip_calls(self, gm): + """Wrap _GitMock so we also log any pip-install invocation.""" + gm.pip_calls: list[list[str]] = [] + original_call = gm.__call__ + + def wrapped(cmd, **kwargs): + cmd_list = list(cmd) + # Detect pip install: either `/path/to/pip install ...` or + # `python -m pip install ...`. The first arg is the executable. + is_pip = ( + "install" in cmd_list + and ( + cmd_list[0].endswith("/pip") + or cmd_list[0] == "pip" + or "pip" in cmd_list # for `python -m pip ...` form + ) + ) + if is_pip: + gm.pip_calls.append(cmd_list) + return b"" + return original_call(cmd, **kwargs) + + return wrapped + + def test_force_deps_triggers_pip_install_even_on_clean_pull(self): + """force_deps=True → pip install runs even when nothing changed in git.""" + gm = _GitMock(dirty_files=[], before_hash="same1", after_hash="same1") + wrapped = self._track_pip_calls(gm) + with ( + patch("subprocess.check_output", side_effect=wrapped), + patch("shutil.which", return_value=None), + patch("os.kill"), + ): + client = _make_client() + r = client.post("/admin/update?branch=beta&force_deps=true") + assert r.status_code == 200 + body = r.json() + assert body.get("deps_rebuilt") is True + assert body.get("deps_error") is None + assert len(gm.pip_calls) == 1, f"expected exactly 1 pip install call, got {gm.pip_calls}" + # Should target the editable install + assert "-e" in gm.pip_calls[0] + assert ".[all]" in gm.pip_calls[0] + + def test_force_and_force_deps_combine(self): + """Both flags together: dirty tree gets reset AND deps get rebuilt.""" + gm = _GitMock(dirty_files=["frontend-dist/index.html"]) + wrapped = self._track_pip_calls(gm) + with ( + patch("subprocess.check_output", side_effect=wrapped), + patch("shutil.which", return_value=None), + patch("os.kill"), + ): + client = _make_client() + r = client.post("/admin/update?branch=beta&force=true&force_deps=true") + assert r.status_code == 200 + body = r.json() + assert body.get("forced_reset") is True + assert body.get("forced_files") == ["frontend-dist/index.html"] + assert body.get("deps_rebuilt") is True + assert gm.did_force_reset() + assert len(gm.pip_calls) == 1 + + def test_no_force_deps_skips_pip_when_pyproject_unchanged(self): + """Default behavior: don't reinstall when pyproject.toml didn't change.""" + gm = _GitMock(dirty_files=[]) + wrapped = self._track_pip_calls(gm) + with ( + patch("subprocess.check_output", side_effect=wrapped), + patch("shutil.which", return_value=None), + patch("os.kill"), + ): + client = _make_client() + r = client.post("/admin/update?branch=beta") + body = r.json() + assert body.get("deps_rebuilt") is False + assert gm.pip_calls == [] + + def test_deps_error_surfaced_when_pip_fails(self): + """If pip install errors out, deps_error is populated and surfaced.""" + gm = _GitMock(dirty_files=[], before_hash="same1", after_hash="same1") + + def fail_on_pip(cmd, **kwargs): + cmd_list = list(cmd) + is_pip = ( + "install" in cmd_list + and (cmd_list[0].endswith("/pip") or cmd_list[0] == "pip" or "pip" in cmd_list) + ) + if is_pip: + raise sp.CalledProcessError(1, cmd, output=b"ERROR: package not found") + return gm(cmd, **kwargs) + + with ( + patch("subprocess.check_output", side_effect=fail_on_pip), + patch("shutil.which", return_value=None), + patch("os.kill"), + ): + client = _make_client() + r = client.post("/admin/update?branch=beta&force_deps=true") + body = r.json() + assert body.get("deps_rebuilt") is False + assert body.get("deps_error") + assert "pip install failed" in body["deps_error"] diff --git a/tests/test_agent_registry.py b/tests/test_agent_registry.py index 286d7f43..3250d45f 100644 --- a/tests/test_agent_registry.py +++ b/tests/test_agent_registry.py @@ -26,6 +26,7 @@ def test_register(self, registry): assert agent.name == "oleg" assert agent.display_name == "Oleg" assert agent.model == "opus" + assert agent.runtime == "claude_sdk" assert agent.enabled is True assert agent.created_at > 0 @@ -44,6 +45,7 @@ def test_register_with_full_config(self, registry, tmp_path): parent="oleg", groups=["butter-team"], max_sessions=3, + runtime="codex_cli", ) assert agent.model == "sonnet" assert agent.soul == "# Leo the Worker" @@ -51,6 +53,7 @@ def test_register_with_full_config(self, registry, tmp_path): assert agent.parent == "oleg" assert agent.groups == ["butter-team"] assert agent.max_sessions == 3 + assert agent.runtime == "codex_cli" def test_register_update(self, registry): registry.register("oleg", model="sonnet") @@ -126,8 +129,74 @@ def test_to_dict(self, registry): assert d["name"] == "test" assert d["display_name"] == "Test Agent" assert d["model"] == "opus" + assert d["runtime"] == "claude_sdk" assert d["enabled"] is True + def test_runtime_column_exists_with_default(self, registry): + columns = { + row[1]: row + for row in registry._db.execute("PRAGMA table_info(agents)").fetchall() + } + assert "runtime" in columns + assert columns["runtime"][4] == "'claude_sdk'" + + agent = registry.register("runtime-default") + assert agent.runtime == "claude_sdk" + + def test_first_register_with_all_kwargs_persists_every_field(self, registry): + # Regression: pre-#358 INSERT omitted provider_url/key/model/ref, thinking_effort, runtime. + # A single register() call (no follow-up UPDATE) must persist all of them. + agent = registry.register( + "full-kwargs-agent", + provider_url="https://api.openai.com/v1", + provider_key="sk-test-key", + provider_model="gpt-5", + provider_ref="some-provider-id", + thinking_effort="high", + runtime="codex_cli", + ) + # Verify via the returned object (built from INSERT path) + assert agent.provider_url == "https://api.openai.com/v1" + assert agent.provider_key == "sk-test-key" + assert agent.provider_model == "gpt-5" + assert agent.provider_ref == "some-provider-id" + assert agent.thinking_effort == "high" + assert agent.runtime == "codex_cli" + + # Verify via a fresh get() to confirm DB round-trip, not just in-memory object + fetched = registry.get("full-kwargs-agent") + assert fetched.provider_url == "https://api.openai.com/v1" + assert fetched.provider_key == "sk-test-key" + assert fetched.provider_model == "gpt-5" + assert fetched.provider_ref == "some-provider-id" + assert fetched.thinking_effort == "high" + assert fetched.runtime == "codex_cli" + + def test_runtime_codex_cli_backfill_is_one_shot_and_idempotent(self, registry): + registry.register("legacy-codex", provider_url="codex_cli", runtime="claude_sdk") + registry.register("explicit-claude", provider_url="codex_cli", runtime="claude_sdk") + registry.register("already-codex", provider_url="codex_cli", runtime="codex_cli") + registry.register("opencode-agent", provider_url="codex_cli", runtime="opencode") + + marker = "migration:agents_runtime_codex_cli_backfill" + registry.set_setting(marker, "") + registry._backfill_runtime_from_provider_url() + assert registry.get("legacy-codex").runtime == "codex_cli" + assert registry.get("explicit-claude").runtime == "codex_cli" + assert registry.get("already-codex").runtime == "codex_cli" + assert registry.get("opencode-agent").runtime == "opencode" + assert registry.get_setting(marker) == "1" + + registry.set_setting(marker, "") + registry._backfill_runtime_from_provider_url() + assert registry.get("legacy-codex").runtime == "codex_cli" + assert registry.get("already-codex").runtime == "codex_cli" + assert registry.get("opencode-agent").runtime == "opencode" + + registry.register("explicit-claude", runtime="claude_sdk") + registry._backfill_runtime_from_provider_url() + assert registry.get("explicit-claude").runtime == "claude_sdk" + def test_stamp_last_seen_updates_column(self, registry): registry.register("seen") agent = registry.get("seen") diff --git a/tests/test_api.py b/tests/test_api.py index e07e9051..66c4ced7 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -233,6 +233,17 @@ def __init__(self, thinking=""): fake_types.ToolResultBlock = ToolResultBlock fake_types.AssistantMessage = AssistantMessage fake_types.ResultMessage = ResultMessage + # Stub for the SDK Literal — reader_loop's import-time invariant + # check (PR #404) reads __args__ to defend against SDK rename. + from typing import Literal as _Literal + fake_types.AssistantMessageError = _Literal[ + "authentication_failed", + "billing_error", + "rate_limit", + "invalid_request", + "server_error", + "unknown", + ] old_sdk_types = sys.modules.get("claude_agent_sdk.types") sys.modules["claude_agent_sdk.types"] = fake_types @@ -600,6 +611,87 @@ async def fake_send(self, prompt: str, platform: str = "", chat_id: str = ""): assert app.state.broker._streaming["test-agent"]["main"].is_connected is True assert sent_prompts[-1][1] == "Wake up" + def test_wake_uses_streaming_session_for_claude_runtime(self): + async def fake_connect(self): + self._connected = True + + async def fake_send(self, prompt: str, platform: str = "", chat_id: str = ""): + del prompt, platform, chat_id + + with tempfile.TemporaryDirectory() as tmpdir, \ + patch("pinky_daemon.streaming_session.StreamingSession.connect", new=fake_connect), \ + patch("pinky_daemon.streaming_session.StreamingSession.send", new=fake_send): + db_path = os.path.join(tmpdir, "test.db") + app = self._make_app(db_path) + with TestClient(app) as client: + client.post("/agents", json={"name": "claude-agent", "model": "sonnet", "runtime": "claude_sdk"}) + + resp = client.post("/agents/claude-agent/wake?prompt=Wake") + assert resp.status_code == 200 + + session = app.state.broker._streaming["claude-agent"]["main"] + assert session.__class__.__name__ == "StreamingSession" + + def test_wake_uses_codex_session_for_codex_runtime(self): + async def fake_connect(self): + self._connected = True + self.session_id = self.session_id or f"{self.agent_name}-codex" + if self._on_session_id: + await self._on_session_id(self.agent_name, self.session_id) + + async def fake_send(self, prompt: str, platform: str = "", chat_id: str = "", message_id: str = "", agent_hint: str = ""): + del prompt, platform, chat_id, message_id, agent_hint + + with tempfile.TemporaryDirectory() as tmpdir, \ + patch("pinky_daemon.codex_session.CodexSession.connect", new=fake_connect), \ + patch("pinky_daemon.codex_session.CodexSession.send", new=fake_send): + db_path = os.path.join(tmpdir, "test.db") + app = self._make_app(db_path) + with TestClient(app) as client: + now = time.time() + app.state.agents._db.execute( + "INSERT INTO providers " + "(id, name, preset, provider_url, provider_key, provider_model, created_at, updated_at) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?)", + ( + "codex-openai", + "Codex OpenAI", + "", + "https://api.openai.com/v1", + "global-openai-key", + "gpt-5-codex", + now, + now, + ), + ) + app.state.agents._db.commit() + client.post("/agents", json={ + "name": "codex-agent", + "model": "fallback-model", + "runtime": "codex_cli", + "provider_ref": "codex-openai", + }) + + resp = client.post("/agents/codex-agent/wake?prompt=Wake") + assert resp.status_code == 200 + + session = app.state.broker._streaming["codex-agent"]["main"] + assert session.__class__.__name__ == "CodexSession" + assert session._config.provider_url == "codex_cli" + assert session._config.provider_key == "global-openai-key" + assert session._config.model == "gpt-5-codex" + + def test_wake_rejects_opencode_runtime_until_session_exists(self): + with tempfile.TemporaryDirectory() as tmpdir: + db_path = os.path.join(tmpdir, "test.db") + app = self._make_app(db_path) + with TestClient(app) as client: + client.post("/agents", json={"name": "opencode-agent", "model": "deepseek-v4", "runtime": "opencode"}) + + resp = client.post("/agents/opencode-agent/wake?prompt=Wake") + assert resp.status_code == 503 + assert "opencode runtime is disabled" in resp.text + def test_streaming_restart_requires_explicit_current_session_save(self): with tempfile.TemporaryDirectory() as tmpdir: db_path = os.path.join(tmpdir, "test.db") @@ -1382,11 +1474,28 @@ def _make_client(self): def test_register_agent(self): client = self._make_client() - resp = client.post("/agents", json={"name": "alice", "model": "sonnet"}) + resp = client.post("/agents", json={ + "name": "alice", + "model": "sonnet", + "runtime": "codex_cli", + "provider_url": "codex_cli", + "provider_model": "gpt-5-codex", + }) assert resp.status_code == 200 data = resp.json() assert data["name"] == "alice" assert data["model"] == "sonnet" + assert data["runtime"] == "codex_cli" + assert data["provider_url"] == "codex_cli" + assert data["provider_model"] == "gpt-5-codex" + + def test_update_agent_runtime(self): + client = self._make_client() + client.post("/agents", json={"name": "alice", "model": "sonnet"}) + + resp = client.put("/agents/alice", json={"runtime": "codex_cli"}) + assert resp.status_code == 200 + assert resp.json()["runtime"] == "codex_cli" def test_register_agent_with_soul(self): client = self._make_client() diff --git a/tests/test_auth_alerts.py b/tests/test_auth_alerts.py new file mode 100644 index 00000000..4f999cc7 --- /dev/null +++ b/tests/test_auth_alerts.py @@ -0,0 +1,406 @@ +"""Tests for the auth-failure tracker and operator-alert wiring.""" +from __future__ import annotations + +from types import SimpleNamespace + +from pinky_daemon.auth_alerts import ( + AuthFailureTracker, + format_alert_message, + resolve_operator_chat, +) +from pinky_daemon.streaming_session import ( + _AUTH_ASSISTANT_ERROR, + _is_auth_error_assistant, + _is_auth_error_result, +) + +# ── SDK Literal invariant ──────────────────────────────────────── +# +# ``_is_auth_error_assistant`` does an exact string match against +# AssistantMessageError. If a future SDK rename of the Literal value +# happens unnoticed, exact-match silently stops detecting credential +# failures — re-creating the exact regression mode #400 was built to +# catch. This test fails loud at CI time on any SDK bump that drops +# or renames "authentication_failed". + + +def test_auth_assistant_error_literal_present_in_sdk_type(): + """``_AUTH_ASSISTANT_ERROR`` must remain a member of + ``AssistantMessageError.__args__``. If the SDK renames the Literal, + update both the constant and ``_is_auth_error_assistant``.""" + from claude_agent_sdk.types import AssistantMessageError + + assert _AUTH_ASSISTANT_ERROR in AssistantMessageError.__args__, ( + f"claude-agent-sdk renamed the AssistantMessageError Literal — " + f"{_AUTH_ASSISTANT_ERROR!r} no longer in {AssistantMessageError.__args__}. " + f"Update _AUTH_ASSISTANT_ERROR in streaming_session.py." + ) + +# ── _is_auth_error_assistant ────────────────────────────────────── +# +# AssistantMessage.error is the AssistantMessageError Literal: +# authentication_failed, billing_error, rate_limit, invalid_request, +# server_error, unknown +# Only "authentication_failed" should trip the operator alert. + + +def _msg(error=None): + """Lightweight AssistantMessage stand-in (only the .error attr matters).""" + return SimpleNamespace(error=error) + + +def test_is_auth_error_assistant_true_only_for_authentication_failed(): + assert _is_auth_error_assistant(_msg("authentication_failed")) is True + + +def test_is_auth_error_assistant_false_for_other_literal_values(): + # Other AssistantMessageError values are real errors but NOT credential + # failures. Alerting the operator on these would be noise (or actively + # wrong, in the case of billing/rate_limit issues that have nothing to + # do with credentials). + for not_auth in ( + "billing_error", + "rate_limit", + "invalid_request", + "server_error", + "unknown", + ): + assert _is_auth_error_assistant(_msg(not_auth)) is False, not_auth + + +def test_is_auth_error_assistant_false_for_none_and_missing(): + assert _is_auth_error_assistant(_msg(None)) is False + # Missing attribute (e.g. unrelated message type) must not raise. + assert _is_auth_error_assistant(SimpleNamespace()) is False + + +# ── _is_auth_error_result ───────────────────────────────────────── +# +# ResultMessage.api_error_status is int|None — raw HTTP status from a +# failing API call. 401/403 are credential failures; 429 and 5xx are +# transient/operational and must not trip the auth alert. + + +def _result(api_error_status=None): + """Lightweight ResultMessage stand-in.""" + return SimpleNamespace(api_error_status=api_error_status) + + +def test_is_auth_error_result_true_for_401_and_403(): + assert _is_auth_error_result(_result(401)) is True + assert _is_auth_error_result(_result(403)) is True + + +def test_is_auth_error_result_false_for_transient_statuses(): + # Rate limit (429) and any 5xx must NOT alert as a credential failure. + for not_auth_status in (429, 500, 502, 503, 504, 529): + assert _is_auth_error_result(_result(not_auth_status)) is False, not_auth_status + + +def test_is_auth_error_result_false_for_none_and_success(): + assert _is_auth_error_result(_result(None)) is False + assert _is_auth_error_result(_result(200)) is False + # Missing attribute (older SDK / non-result objects) must not raise. + assert _is_auth_error_result(SimpleNamespace()) is False + + +# ── AuthFailureTracker ──────────────────────────────────────────── + + +class _FakeClock: + def __init__(self, start: float = 1_000_000.0) -> None: + self.now = start + + def __call__(self) -> float: + return self.now + + def tick(self, seconds: float) -> None: + self.now += seconds + + +def _make_tracker(**kw): + clock = _FakeClock() + tracker = AuthFailureTracker( + fail_window_seconds=kw.get("window", 300), + fail_threshold=kw.get("threshold", 3), + alert_cooldown_seconds=kw.get("cooldown", 1800), + clock=clock, + ) + return tracker, clock + + +def test_below_threshold_does_not_alert(): + tracker, _ = _make_tracker() + d = tracker.record_failure("sasha", "authentication_failed") + assert d["should_alert"] is False + assert d["count"] == 1 + assert d["reason"] == "below_threshold" + + +def test_threshold_for_single_agent_fires_alert(): + tracker, clock = _make_tracker(threshold=3) + for _ in range(2): + clock.tick(10) + d = tracker.record_failure("sasha", "authentication_failed") + assert d["should_alert"] is False + clock.tick(10) + d = tracker.record_failure("sasha", "authentication_failed") + assert d["should_alert"] is True + assert d["reason"] == "agent_repeated" + assert d["count"] == 3 + # Decision alone must not commit cooldown — caller hasn't delivered yet. + assert tracker._last_alert_at == 0.0 + assert tracker._alert_count == 0 + + +def test_host_wide_outage_fires_immediately_at_threshold_agents(): + """If 3 different agents fail, alert on the 3rd — don't wait for any + single agent to hit the per-agent threshold.""" + tracker, _ = _make_tracker(threshold=3) + assert tracker.record_failure("sasha", "auth")["should_alert"] is False + assert tracker.record_failure("ivan", "auth")["should_alert"] is False + d = tracker.record_failure("kuzya", "auth") + assert d["should_alert"] is True + assert d["reason"] == "host_wide" + assert d["agents_failing"] == 3 + + +def test_cooldown_blocks_repeat_alerts(): + tracker, clock = _make_tracker(threshold=3, cooldown=600) + # Drive past threshold once and simulate successful delivery. + for _ in range(3): + tracker.record_failure("sasha", "auth") + tracker.commit_alert() + # Next failure within cooldown — must not realert. + clock.tick(60) + d = tracker.record_failure("sasha", "auth") + assert d["should_alert"] is False + assert d["reason"] == "cooldown" + assert d["cooldown_remaining"] > 0 + + +def test_cooldown_elapses_then_alert_fires_again(): + """During a continuous outage we want a re-alert each cooldown period. + + Use a window long enough that failures stay live across the cooldown gap + — that's the realistic outage shape (agent retries every minute or so). + """ + tracker, clock = _make_tracker(threshold=3, window=3600, cooldown=600) + for _ in range(3): + tracker.record_failure("sasha", "auth") # first alert decision here + tracker.commit_alert() # simulate first delivery succeeded + clock.tick(601) # past cooldown, still inside window + d = tracker.record_failure("sasha", "auth") + assert d["should_alert"] is True + assert d["count"] >= 3 + + +def test_failed_delivery_preserves_cooldown_and_retries_next_failure(): + """Regression: a broker_send raise must NOT advance the cooldown. + + Pushok review on PR #400: the original code mutated _last_alert_at + *before* the await, so a network blip on the very first send-attempt + silenced the alert system for 30 minutes — the exact regression this + feature exists to prevent (Sasha-down-9-12h). + """ + tracker, clock = _make_tracker(threshold=3, cooldown=600) + # First three failures cross threshold; caller "tries to deliver" but + # the broker raises, so commit_alert() is NEVER called. + for _ in range(3): + d = tracker.record_failure("sasha", "auth") + assert d["should_alert"] is True + # No commit_alert() — simulating broker_send failure. + assert tracker._last_alert_at == 0.0 # cooldown untouched + + # Next failure (1 second later) must still be should_alert: True so the + # caller can retry delivery. + clock.tick(1) + d = tracker.record_failure("sasha", "auth") + assert d["should_alert"] is True + assert d["reason"] == "agent_repeated" + + +def test_commit_alert_advances_cooldown_and_counter(): + tracker, clock = _make_tracker(threshold=3, cooldown=600) + for _ in range(3): + tracker.record_failure("sasha", "auth") + assert tracker._alert_count == 0 + tracker.commit_alert() + assert tracker._alert_count == 1 + assert tracker._last_alert_at == clock.now + + +def test_window_eviction_drops_old_failures(): + tracker, clock = _make_tracker(window=60, threshold=3) + tracker.record_failure("sasha", "auth") + clock.tick(61) # first entry now stale + tracker.record_failure("sasha", "auth") + clock.tick(1) + d = tracker.record_failure("sasha", "auth") + # Only 2 fresh entries within window — below threshold. + assert d["should_alert"] is False + assert d["count"] == 2 + + +def test_record_success_clears_agent_failures(): + tracker, _ = _make_tracker(threshold=3) + tracker.record_failure("sasha", "auth") + tracker.record_failure("sasha", "auth") + tracker.record_success("sasha") + d = tracker.record_failure("sasha", "auth") + assert d["count"] == 1 + assert d["should_alert"] is False + + +def test_status_reports_ok_when_no_failures(): + tracker, _ = _make_tracker() + s = tracker.status() + assert s["status"] == "ok" + assert s["agents_failing"] == [] + assert s["alerts_sent"] == 0 + + +def test_status_reports_broken_at_or_above_threshold(): + tracker, _ = _make_tracker(threshold=3) + for _ in range(3): + tracker.record_failure("sasha", "auth") + tracker.commit_alert() # simulate delivery succeeded + s = tracker.status() + assert s["status"] == "broken" + assert s["alerts_sent"] == 1 + assert any(a["agent"] == "sasha" for a in s["agents_failing"]) + + +def test_status_reports_degraded_below_threshold(): + tracker, _ = _make_tracker(threshold=3) + tracker.record_failure("sasha", "auth") + s = tracker.status() + assert s["status"] == "degraded" + + +# ── resolve_operator_chat ───────────────────────────────────────── + + +def test_resolve_operator_chat_prefers_setting(): + settings = {"operator_chat_id": "12345", "operator_platform": "telegram"} + chat_id, platform = resolve_operator_chat( + get_setting=lambda k, default="": settings.get(k, default), + list_all_approved_users=lambda: [], + ) + assert chat_id == "12345" + assert platform == "telegram" + + +def test_resolve_operator_chat_falls_back_to_most_common_approved_user(): + users = [ + {"chat_id": "111", "agent_name": "a"}, + {"chat_id": "222", "agent_name": "a"}, + {"chat_id": "111", "agent_name": "b"}, + {"chat_id": "111", "agent_name": "c"}, + ] + chat_id, platform = resolve_operator_chat( + get_setting=lambda k, default="": "", + list_all_approved_users=lambda: users, + ) + assert chat_id == "111" # appears 3× → operator + assert platform == "telegram" + + +def test_resolve_operator_chat_returns_empty_when_nothing_known(): + chat_id, platform = resolve_operator_chat( + get_setting=lambda k, default="": "", + list_all_approved_users=lambda: [], + ) + assert chat_id == "" + assert platform == "" + + +def test_resolve_operator_chat_tolerates_setting_errors(): + def bad_setting(*_a, **_kw): + raise RuntimeError("db down") + + chat_id, platform = resolve_operator_chat( + get_setting=bad_setting, + list_all_approved_users=lambda: [{"chat_id": "999"}], + ) + assert chat_id == "999" # fell through to fallback + assert platform == "telegram" + + +# ── format_alert_message ────────────────────────────────────────── + + +def test_format_alert_includes_agent_when_repeated(): + msg = format_alert_message( + agent_name="sasha", + decision={"reason": "agent_repeated", "count": 3, "agents_failing": 1}, + error="authentication_failed", + host_label="rpi5", + ) + assert "sasha" in msg + assert "authentication_failed" in msg + assert "Re-auth" in msg + + +def test_format_alert_says_host_wide_when_multiple_agents_fail(): + msg = format_alert_message( + agent_name="sasha", + decision={"reason": "host_wide", "count": 1, "agents_failing": 3}, + error="invalid_api_key", + host_label="rpi5", + ) + assert "rpi5" in msg + assert "3 agent" in msg + + +def test_format_alert_truncates_long_error_strings(): + long = "x" * 5000 + msg = format_alert_message( + agent_name="sasha", + decision={"reason": "agent_repeated", "count": 3, "agents_failing": 1}, + error=long, + host_label="", + ) + # Last error block is capped at 200 chars. + assert "x" * 5000 not in msg + + +def test_format_alert_uses_no_markdown_so_plain_text_renders_clean(): + """Pushok review: _broker_send defaults to no parse_mode, so any markdown + in the message renders literally on Telegram (operator sees `*sasha*`). + Keep the message plain text. + """ + msg = format_alert_message( + agent_name="sasha", + decision={"reason": "agent_repeated", "count": 3, "agents_failing": 1}, + error="authentication_failed", + host_label="rpi5", + ) + assert "*" not in msg + assert "`" not in msg + # Sanity: still actually informative. + assert "sasha" in msg + assert "authentication_failed" in msg + + +# ── /admin/watchdog integration ─────────────────────────────────── + + +def test_admin_watchdog_exposes_auth_status(tmp_path): + """End-to-end: /admin/watchdog includes auth_status with the right shape.""" + from fastapi.testclient import TestClient + + from pinky_daemon.api import create_api + + api = create_api(db_path=str(tmp_path / "memory.db")) + with TestClient(api) as client: + resp = client.get("/admin/watchdog") + assert resp.status_code == 200 + body = resp.json() + assert "auth_status" in body + auth = body["auth_status"] + assert auth["status"] == "ok" # no failures yet + assert auth["fail_threshold"] >= 1 + assert auth["agents_failing"] == [] + assert "fail_window_seconds" in auth + assert "alert_cooldown_seconds" in auth diff --git a/tests/test_broker.py b/tests/test_broker.py index 910ce97c..277a86bb 100644 --- a/tests/test_broker.py +++ b/tests/test_broker.py @@ -126,6 +126,76 @@ async def test_inject_agent_message_does_not_stamp_when_not_connected(self): finally: tmpdir.cleanup() + @pytest.mark.asyncio + async def test_stop_typing_cancels_active_task(self): + """_stop_typing must cancel a running typing-loop task.""" + import asyncio + + tmpdir, _, broker, _, _ = self._make_broker() + try: + async def _fake_loop(): + await asyncio.sleep(60) + + task = asyncio.create_task(_fake_loop()) + # Yield once so the task actually starts before we cancel it. + await asyncio.sleep(0) + broker._typing_tasks[("barsik", "6770805286")] = task + + broker._stop_typing("barsik", "6770805286") + # Yield until cancellation has propagated to the task. + try: + await task + except asyncio.CancelledError: + pass + + assert ("barsik", "6770805286") not in broker._typing_tasks + assert task.cancelled() + finally: + tmpdir.cleanup() + + def test_stop_typing_is_silent_noop_when_no_task(self): + """Defensive _stop_typing calls (e.g. after every outreach send) must be no-op.""" + tmpdir, _, broker, _, _ = self._make_broker() + try: + # No task registered for this chat — should not raise, should not log. + broker._stop_typing("barsik", "never-typed-here") + assert ("barsik", "never-typed-here") not in broker._typing_tasks + finally: + tmpdir.cleanup() + + @pytest.mark.asyncio + async def test_stop_typing_scoped_per_chat(self): + """Stopping typing for one chat must not affect other chats for the same agent.""" + import asyncio + + tmpdir, _, broker, _, _ = self._make_broker() + try: + async def _fake_loop(): + await asyncio.sleep(60) + + task_a = asyncio.create_task(_fake_loop()) + task_b = asyncio.create_task(_fake_loop()) + broker._typing_tasks[("barsik", "chat-A")] = task_a + broker._typing_tasks[("barsik", "chat-B")] = task_b + + broker._stop_typing("barsik", "chat-A") + # Yield once so cancellation propagates. + await asyncio.sleep(0) + + assert ("barsik", "chat-A") not in broker._typing_tasks + assert ("barsik", "chat-B") in broker._typing_tasks + assert task_a.cancelled() or task_a.done() + assert not task_b.done() + + # Cleanup + task_b.cancel() + try: + await task_b + except asyncio.CancelledError: + pass + finally: + tmpdir.cleanup() + def test_remember_message_context_tracks_voice_and_reply_metadata(self): tmpdir, _, broker, _, _ = self._make_broker() try: diff --git a/tests/test_context_estimator.py b/tests/test_context_estimator.py new file mode 100644 index 00000000..8286d403 --- /dev/null +++ b/tests/test_context_estimator.py @@ -0,0 +1,72 @@ +"""Tests for shared context estimation helpers.""" + +from __future__ import annotations + +from types import SimpleNamespace + +from pinky_daemon.context_estimator import ContextTextEstimator + + +class _Store: + def __init__(self, *contents: str) -> None: + self.contents = contents + self.calls: list[tuple[str, int]] = [] + + def get_history(self, session_id: str, limit: int = 1000): + self.calls.append((session_id, limit)) + return [SimpleNamespace(content=content) for content in self.contents] + + +class _BrokenStore: + def get_history(self, session_id: str, limit: int = 1000): + raise RuntimeError("db unavailable") + + +def test_estimates_internal_texts(): + estimator = ContextTextEstimator(chars_per_token=4) + + estimator.record_internal_text("") + estimator.record_internal_text("abcd") + estimator.record_internal_text("abcdefghi") + + assert estimator.estimated_tokens(session_id="s") == 3 + + +def test_estimates_store_history_with_internal_texts(): + estimator = ContextTextEstimator(chars_per_token=4) + estimator.record_internal_text("abcdefgh") + store = _Store("abcd", "", "abcdefghijkl") + + total = estimator.estimated_tokens( + session_id="agent-main", + conversation_store=store, + history_limit=50, + ) + + assert total == 6 + assert store.calls == [("agent-main", 50)] + + +def test_returns_internal_estimate_when_store_read_fails(): + estimator = ContextTextEstimator(chars_per_token=4) + estimator.record_internal_text("abcdefgh") + + assert estimator.estimated_tokens( + session_id="agent-main", + conversation_store=_BrokenStore(), + ) == 2 + + +def test_context_info_shape_and_percentage(): + estimator = ContextTextEstimator(chars_per_token=4) + estimator.record_internal_text("abcdefgh") + + info = estimator.context_info(session_id="agent-main", max_tokens=10) + + assert info == { + "total_tokens": 2, + "max_tokens": 10, + "percentage": 20.0, + "categories": [], + "mcp_tools": [], + } diff --git a/tests/test_discord.py b/tests/test_discord.py index 4de93eb2..b78a7729 100644 --- a/tests/test_discord.py +++ b/tests/test_discord.py @@ -7,7 +7,7 @@ import httpx import pytest -from pinky_outreach.discord import DiscordAdapter, DiscordError +from pinky_outreach.discord import DiscordAdapter, DiscordError, DiscordRateLimited from pinky_outreach.types import Platform @@ -84,6 +84,114 @@ def test_send_message_error(self): assert exc.value.status_code == 403 adapter.close() + def test_open_dm_channel_caches(self): + """open_dm_channel calls /users/@me/channels once per recipient and caches.""" + adapter = self._make_adapter() + resp = MagicMock() + resp.status_code = 200 + resp.json.return_value = {"id": "dm-channel-abc", "type": 1} + adapter._client.request = MagicMock(return_value=resp) + + ch1 = adapter.open_dm_channel("user-754") + ch2 = adapter.open_dm_channel("user-754") + + assert ch1 == "dm-channel-abc" + assert ch2 == "dm-channel-abc" + # Only one HTTP call thanks to the cache + assert adapter._client.request.call_count == 1 + # Verify it hit the right endpoint with the right body + call = adapter._client.request.call_args + assert call.args[0] == "POST" + assert call.args[1] == "/users/@me/channels" + assert call.kwargs["json"] == {"recipient_id": "user-754"} + adapter.close() + + def test_send_message_falls_back_to_dm_on_404_unknown_channel(self): + """If channel_id is actually a user_id, send_message opens a DM and retries.""" + adapter = self._make_adapter() + + send_404 = MagicMock() + send_404.status_code = 404 + send_404.json.return_value = {"message": "Unknown Channel", "code": 10003} + + open_dm_ok = MagicMock() + open_dm_ok.status_code = 200 + open_dm_ok.json.return_value = {"id": "dm-channel-xyz", "type": 1} + + send_ok = MagicMock() + send_ok.status_code = 200 + send_ok.json.return_value = { + "id": "msg-9999", + "channel_id": "dm-channel-xyz", + "content": "hi owner", + "timestamp": "2026-05-05T19:00:00+00:00", + } + + adapter._client.request = MagicMock(side_effect=[send_404, open_dm_ok, send_ok]) + + msg = adapter.send_message("754027672526389310", "hi owner") + assert msg.message_id == "msg-9999" + # chat_id should reflect the resolved DM channel, not the user_id + assert msg.chat_id == "dm-channel-xyz" + + # Three calls: failed POST → open DM → retry POST + assert adapter._client.request.call_count == 3 + calls = adapter._client.request.call_args_list + assert calls[0].args == ("POST", "/channels/754027672526389310/messages") + assert calls[1].args == ("POST", "/users/@me/channels") + assert calls[1].kwargs["json"] == {"recipient_id": "754027672526389310"} + assert calls[2].args == ("POST", "/channels/dm-channel-xyz/messages") + + # Cache populated for next time + assert adapter._dm_channel_cache["754027672526389310"] == "dm-channel-xyz" + adapter.close() + + def test_send_message_404_other_message_does_not_fallback(self): + """404 with a different message (e.g. Unknown Message) should NOT trigger DM open.""" + adapter = self._make_adapter() + + resp = MagicMock() + resp.status_code = 404 + resp.json.return_value = {"message": "Unknown Message", "code": 10008} + adapter._client.request = MagicMock(return_value=resp) + + with pytest.raises(DiscordError) as exc: + adapter.send_message("99999", "hi", reply_to="missing-msg-id") + assert exc.value.status_code == 404 + assert "Unknown Message" in str(exc.value) + # No DM resolution attempted + assert adapter._client.request.call_count == 1 + adapter.close() + + def test_send_message_dm_fallback_uses_cached_channel(self): + """Second send to the same user_id reuses the cached DM channel — no /users/@me/channels call.""" + adapter = self._make_adapter() + # Pre-populate the cache as if a previous send had resolved the DM + adapter._dm_channel_cache["user-754"] = "dm-channel-abc" + + send_404 = MagicMock() + send_404.status_code = 404 + send_404.json.return_value = {"message": "Unknown Channel", "code": 10003} + + send_ok = MagicMock() + send_ok.status_code = 200 + send_ok.json.return_value = { + "id": "msg-2", + "channel_id": "dm-channel-abc", + "content": "second msg", + "timestamp": "2026-05-05T19:01:00+00:00", + } + + adapter._client.request = MagicMock(side_effect=[send_404, send_ok]) + + msg = adapter.send_message("user-754", "second msg") + assert msg.message_id == "msg-2" + # Only the failed initial POST + the retry — no /users/@me/channels lookup + assert adapter._client.request.call_count == 2 + calls = adapter._client.request.call_args_list + assert "/users/@me/channels" not in (calls[0].args[1], calls[1].args[1]) + adapter.close() + def test_send_file_success_uses_httpx_multipart_header(self, tmp_path): """Regression: passing Content-Type=None makes httpx raise before sending.""" adapter = self._make_adapter() @@ -283,6 +391,122 @@ def test_get_guild_channels(self): assert channels[1].title == "voice" adapter.close() + def test_user_agent_header_sent(self): + """Discord 403s requests with the default httpx UA on some endpoints.""" + adapter = self._make_adapter() + ua = adapter._client.headers["User-Agent"] + # Must follow Discord's "DiscordBot (..., version)" convention + assert ua.startswith("DiscordBot (") + adapter.close() + + def test_429_retry_then_success(self, monkeypatch): + """A single 429 should sleep `retry_after` then retry transparently.""" + adapter = self._make_adapter() + slept: list[float] = [] + monkeypatch.setattr( + "pinky_outreach.discord.time.sleep", + lambda s: slept.append(s), + ) + + rate_limited = MagicMock() + rate_limited.status_code = 429 + rate_limited.json.return_value = {"retry_after": 0.5, "message": "rate limited"} + + success = MagicMock() + success.status_code = 200 + success.json.return_value = { + "id": "1234567890", + "channel_id": "99999", + "content": "ok", + "timestamp": "2026-03-27T12:00:00+00:00", + } + adapter._client.request = MagicMock(side_effect=[rate_limited, success]) + + msg = adapter.send_message("99999", "ok") + assert msg.message_id == "1234567890" + assert slept and slept[0] == pytest.approx(0.5) + adapter.close() + + def test_429_retry_exhausted_raises_rate_limited(self, monkeypatch): + """After `max_429_retries` attempts a 429 must surface as DiscordRateLimited.""" + adapter = DiscordAdapter("fake-discord-token", max_429_retries=1) + monkeypatch.setattr("pinky_outreach.discord.time.sleep", lambda s: None) + + rate_limited = MagicMock() + rate_limited.status_code = 429 + rate_limited.json.return_value = {"retry_after": 2.5} + + adapter._client.request = MagicMock(return_value=rate_limited) + with pytest.raises(DiscordRateLimited) as exc: + adapter.send_message("99999", "ok") + assert exc.value.retry_after == pytest.approx(2.5) + adapter.close() + + def test_get_my_guilds(self): + adapter = self._make_adapter() + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = [ + {"id": "g1", "name": "Server One"}, + {"id": "g2", "name": "Server Two"}, + ] + adapter._client.request = MagicMock(return_value=mock_response) + + guilds = adapter.get_my_guilds() + assert len(guilds) == 2 + assert guilds[0]["id"] == "g1" + adapter.close() + + def test_discover_text_channels_filters_voice_and_categories(self): + """Channel discovery should include text(0) + announcement(5), drop the rest.""" + adapter = self._make_adapter() + + guilds_resp = MagicMock() + guilds_resp.status_code = 200 + guilds_resp.json.return_value = [{"id": "g1", "name": "Server One"}] + + channels_resp = MagicMock() + channels_resp.status_code = 200 + channels_resp.json.return_value = [ + {"id": "ch-text", "name": "general", "type": 0}, + {"id": "ch-voice", "name": "Lounge", "type": 2}, + {"id": "ch-cat", "name": "Category", "type": 4}, + {"id": "ch-announce", "name": "news", "type": 5}, + ] + adapter._client.request = MagicMock(side_effect=[guilds_resp, channels_resp]) + + ids = adapter.discover_text_channels() + assert ids == ["ch-text", "ch-announce"] + adapter.close() + + def test_discover_text_channels_skips_unreadable_guilds(self): + """If listing one guild's channels fails, others should still be returned.""" + adapter = self._make_adapter() + + guilds_resp = MagicMock() + guilds_resp.status_code = 200 + guilds_resp.json.return_value = [ + {"id": "g1", "name": "Server One"}, + {"id": "g2", "name": "Server Two"}, + ] + + forbidden_resp = MagicMock() + forbidden_resp.status_code = 403 + forbidden_resp.json.return_value = {"message": "Missing Access"} + + ok_channels_resp = MagicMock() + ok_channels_resp.status_code = 200 + ok_channels_resp.json.return_value = [ + {"id": "ch-ok", "name": "general", "type": 0}, + ] + adapter._client.request = MagicMock( + side_effect=[guilds_resp, forbidden_resp, ok_channels_resp], + ) + + ids = adapter.discover_text_channels() + assert ids == ["ch-ok"] + adapter.close() + class TestOutreachServerDiscord: """Test the MCP server with Discord support.""" diff --git a/tests/test_discord_poller.py b/tests/test_discord_poller.py new file mode 100644 index 00000000..723b4450 --- /dev/null +++ b/tests/test_discord_poller.py @@ -0,0 +1,469 @@ +"""Tests for BrokerDiscordPoller (REST-polling Discord inbound).""" + +from __future__ import annotations + +import asyncio +from datetime import datetime +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from pinky_outreach.discord import DiscordError, DiscordRateLimited +from pinky_outreach.types import Chat, Message, Platform + + +@pytest.fixture +def mock_adapter(): + """A mock DiscordAdapter with sensible defaults.""" + adapter = MagicMock() + adapter.get_me.return_value = {"id": "bot-001", "username": "TestBot"} + adapter.discover_text_channels.return_value = ["chan-A"] + adapter.get_messages.return_value = [] + adapter.get_channel.return_value = Chat( + platform=Platform.discord, + chat_id="chan-A", + title="general", + chat_type="text", + ) + return adapter + + +@pytest.fixture +def mock_broker(): + broker = MagicMock() + broker.handle_inbound = AsyncMock() + return broker + + +def _msg( + *, + msg_id: str, + content: str, + author_id: str = "user-1", + sender: str = "brad", + is_bot: bool = False, + reply_to: str = "", +) -> Message: + return Message( + platform=Platform.discord, + chat_id="chan-A", + sender=sender, + content=content, + timestamp=datetime.fromisoformat("2026-05-05T12:00:00+00:00"), + message_id=msg_id, + reply_to=reply_to, + is_outbound=is_bot, + metadata={"author_id": author_id, "is_bot": is_bot}, + ) + + +class TestBrokerDiscordPoller: + """Mocked-adapter tests for the inbound Discord poller.""" + + def _make_poller(self, mock_adapter, mock_broker, **kwargs): + from pinky_daemon.pollers import BrokerDiscordPoller + return BrokerDiscordPoller( + mock_adapter, "barsik", mock_broker, registry=None, + **kwargs, + ) + + @pytest.mark.asyncio + async def test_priming_skips_history_replay(self, mock_adapter, mock_broker): + """First sweep should treat existing latest message as the baseline (no replay).""" + existing = _msg(msg_id="100", content="ancient") + # Adapter returns the most recent message during priming, then nothing on poll + mock_adapter.get_messages.side_effect = [ + [existing], # priming call (limit=1) + [], # _poll_once call + ] + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + await poller._poll_once() + + assert poller.watched_channels == ["chan-A"] + assert poller._last_id["chan-A"] == "100" + mock_broker.handle_inbound.assert_not_called() + + @pytest.mark.asyncio + async def test_new_message_routed_through_broker(self, mock_adapter, mock_broker): + """Real inbound user message should produce a BrokerMessage with platform=discord.""" + baseline = _msg(msg_id="100", content="baseline") + new_msg = _msg(msg_id="101", content="hello", author_id="user-9", sender="ryan") + mock_adapter.get_messages.side_effect = [ + [baseline], # priming + [new_msg], # poll + ] + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + await poller._poll_once() + + # Allow the fire-and-forget task to settle + await asyncio.sleep(0) + + assert mock_broker.handle_inbound.await_count == 1 + delivered = mock_broker.handle_inbound.await_args.args[0] + assert delivered.platform == "discord" + assert delivered.chat_id == "chan-A" + assert delivered.message_id == "101" + assert delivered.content == "hello" + assert delivered.sender_name == "ryan" + assert delivered.sender_id == "user-9" + assert delivered.agent_name == "barsik" + assert poller._last_id["chan-A"] == "101" + + @pytest.mark.asyncio + async def test_bot_messages_skipped(self, mock_adapter, mock_broker): + """is_bot=True or author_id matching the bot's own id must be filtered out.""" + baseline = _msg(msg_id="100", content="baseline") + bot_self = _msg( + msg_id="101", content="self echo", + author_id="bot-001", sender="TestBot", is_bot=False, + ) + bot_other = _msg( + msg_id="102", content="other bot", + author_id="bot-other", sender="OtherBot", is_bot=True, + ) + real_user = _msg( + msg_id="103", content="real", + author_id="user-9", sender="ryan", + ) + # Discord returns newest-first; poller reverses to chronological for delivery. + mock_adapter.get_messages.side_effect = [ + [baseline], + [real_user, bot_other, bot_self], + ] + + poller = self._make_poller(mock_adapter, mock_broker) + # Set bot id manually so we don't depend on start() + poller._bot_user_id = "bot-001" + await poller._refresh_channels(verbose=True) + await poller._poll_once() + + await asyncio.sleep(0) + + # Only the real-user message should be delivered + assert mock_broker.handle_inbound.await_count == 1 + delivered = mock_broker.handle_inbound.await_args.args[0] + assert delivered.message_id == "103" + # last_id advances past all of them so we don't refetch + assert poller._last_id["chan-A"] == "103" + + @pytest.mark.asyncio + async def test_explicit_watched_channels_override_discovery(self, mock_adapter, mock_broker): + """If settings.watched_channels is set, discover_text_channels must NOT be called.""" + # Empty channel = "0" sentinel + mock_adapter.get_messages.return_value = [] + + poller = self._make_poller( + mock_adapter, mock_broker, + watched_channels=["explicit-1", "explicit-2"], + ) + await poller._refresh_channels(verbose=True) + + assert sorted(poller.watched_channels) == ["explicit-1", "explicit-2"] + mock_adapter.discover_text_channels.assert_not_called() + + @pytest.mark.asyncio + async def test_empty_channel_primes_with_zero_sentinel(self, mock_adapter, mock_broker): + """A channel with no messages at priming time should record last_id='0'.""" + mock_adapter.get_messages.return_value = [] # priming finds nothing + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + + assert poller._last_id["chan-A"] == "0" + + @pytest.mark.asyncio + async def test_priming_delivers_fresh_first_test_message( + self, mock_adapter, mock_broker, + ): + """Fresh (<30s) non-bot message at prime time → backed-off floor so it gets delivered. + + Scenario: bot joins server, user sends test message, then discovery + sweep runs ~seconds later. Without this fix, the test message becomes + the floor and is silently dropped. + """ + from datetime import datetime, timezone + fresh_msg = Message( + platform=Platform.discord, + chat_id="chan-A", + sender="brad", + content="hi bot", + # Brand new — well within the 30s freshness window + timestamp=datetime.now(timezone.utc), + message_id="200", + metadata={"author_id": "user-1", "is_bot": False}, + ) + mock_adapter.get_messages.side_effect = [ + [fresh_msg], # priming + [fresh_msg], # poll picks it up because floor was backed off to "199" + ] + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + + # Floor must be id-1 so the next poll fetches this message + assert poller._last_id["chan-A"] == "199" + + await poller._poll_once() + await asyncio.sleep(0) + + # The fresh test message should have been delivered through the broker + assert mock_broker.handle_inbound.await_count == 1 + delivered = mock_broker.handle_inbound.await_args.args[0] + assert delivered.message_id == "200" + assert delivered.content == "hi bot" + + @pytest.mark.asyncio + async def test_priming_old_message_uses_message_as_floor( + self, mock_adapter, mock_broker, + ): + """Stale (>30s) most-recent message → use it as floor, NOT delivered.""" + from datetime import datetime, timezone + old_msg = Message( + platform=Platform.discord, + chat_id="chan-A", + sender="brad", + content="ancient", + timestamp=datetime(2020, 1, 1, tzinfo=timezone.utc), + message_id="100", + metadata={"author_id": "user-1", "is_bot": False}, + ) + mock_adapter.get_messages.side_effect = [ + [old_msg], # priming + [], # poll finds nothing new + ] + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + await poller._poll_once() + await asyncio.sleep(0) + + # Floor is the message id itself — replay-prevention behavior preserved + assert poller._last_id["chan-A"] == "100" + mock_broker.handle_inbound.assert_not_called() + + @pytest.mark.asyncio + async def test_priming_fresh_bot_message_does_not_back_off( + self, mock_adapter, mock_broker, + ): + """Fresh-but-bot message → use as floor (don't loop on bot self-reply).""" + from datetime import datetime, timezone + fresh_bot = Message( + platform=Platform.discord, + chat_id="chan-A", + sender="OtherBot", + content="bot chatter", + timestamp=datetime.now(timezone.utc), + message_id="300", + metadata={"author_id": "bot-other", "is_bot": True}, + ) + mock_adapter.get_messages.side_effect = [ + [fresh_bot], # priming + [], # poll + ] + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + await poller._poll_once() + await asyncio.sleep(0) + + # Bot message is the floor as-is — not backed off, not delivered + assert poller._last_id["chan-A"] == "300" + mock_broker.handle_inbound.assert_not_called() + + @pytest.mark.asyncio + async def test_poll_drops_after_filter_for_zero_sentinel(self, mock_adapter, mock_broker): + """When last_id='0' (empty channel), the poll must NOT pass after=0 to the adapter.""" + # Priming: empty + # Poll: a real first message arrives + first = _msg(msg_id="500", content="first ever", author_id="user-9") + mock_adapter.get_messages.side_effect = [ + [], # priming + [first], # poll + ] + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + await poller._poll_once() + + await asyncio.sleep(0) + + # Inspect the second call (the actual poll) + poll_call = mock_adapter.get_messages.call_args_list[1] + kwargs = poll_call.kwargs + # `after` should be empty (sentinel dropped), not "0" + assert kwargs.get("after", "") == "" + # And the first message did get delivered + assert mock_broker.handle_inbound.await_count == 1 + + @pytest.mark.asyncio + async def test_messages_routed_in_chronological_order(self, mock_adapter, mock_broker): + """Discord returns newest-first; poller must reverse so broker sees chronological order.""" + baseline = _msg(msg_id="100", content="baseline") + # Discord returns newest-first + newer = _msg(msg_id="103", content="third") + middle = _msg(msg_id="102", content="second") + older = _msg(msg_id="101", content="first") + mock_adapter.get_messages.side_effect = [ + [baseline], + [newer, middle, older], + ] + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + await poller._poll_once() + + await asyncio.sleep(0) + + delivered_ids = [ + call.args[0].message_id + for call in mock_broker.handle_inbound.await_args_list + ] + assert delivered_ids == ["101", "102", "103"] + + @pytest.mark.asyncio + async def test_discovery_failure_keeps_existing_set(self, mock_adapter, mock_broker): + """A transient discovery failure must not blank the watch list.""" + mock_adapter.get_messages.return_value = [] + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + # Now simulate failure on next discovery + mock_adapter.discover_text_channels.side_effect = DiscordError( + "transient", status_code=502, + ) + + await poller._refresh_channels(verbose=False) + assert poller.watched_channels == ["chan-A"] # unchanged + + @pytest.mark.asyncio + async def test_rate_limit_propagates_for_outer_sleep(self, mock_adapter, mock_broker): + """A 429 mid-poll must propagate so the outer loop can sleep retry_after.""" + baseline = _msg(msg_id="100", content="baseline") + mock_adapter.get_messages.side_effect = [ + [baseline], + DiscordRateLimited(retry_after=0.5), + ] + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + with pytest.raises(DiscordRateLimited): + await poller._poll_once() + + def test_poll_interval_floor(self, mock_adapter, mock_broker): + """Sub-quarter-second intervals must be clamped to keep us under budget.""" + poller = self._make_poller( + mock_adapter, mock_broker, poll_interval=0.05, + ) + assert poller._poll_interval == 0.25 + + def test_stop_clears_running_flag(self, mock_adapter, mock_broker): + poller = self._make_poller(mock_adapter, mock_broker) + poller._running = True + poller.stop() + assert poller.is_running is False + + @pytest.mark.asyncio + async def test_get_channel_cached_across_burst(self, mock_adapter, mock_broker): + """A burst of N messages on one channel must trigger only ONE get_channel call. + + Regression for the burst pathology Pushok flagged in PR #383: prior code + looked up channel metadata inside the per-message loop, fanning out to N + round-trips for a chatty channel. Now hoisted to once per channel per poll + sweep with a discovery-cycle cache. + """ + baseline = _msg(msg_id="100", content="baseline") + burst = [ + _msg(msg_id=f"{200 + i}", content=f"msg-{i}", author_id="user-9") + for i in range(5) + ] + # Discord returns newest-first + mock_adapter.get_messages.side_effect = [ + [baseline], # priming + list(reversed(burst)), # poll: newest-first + ] + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + await poller._poll_once() + + await asyncio.sleep(0) + + assert mock_broker.handle_inbound.await_count == 5 + # Hoisted lookup: one call per channel per poll, not per message + assert mock_adapter.get_channel.call_count == 1 + + @pytest.mark.asyncio + async def test_get_channel_cache_reused_across_polls(self, mock_adapter, mock_broker): + """Cache survives across polls within a discovery cycle.""" + baseline = _msg(msg_id="100", content="baseline") + first = _msg(msg_id="101", content="first", author_id="user-9") + second = _msg(msg_id="102", content="second", author_id="user-9") + mock_adapter.get_messages.side_effect = [ + [baseline], # priming + [first], # poll 1 + [second], # poll 2 + ] + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + await poller._poll_once() + await poller._poll_once() + + await asyncio.sleep(0) + + # Still one get_channel call total — cached after first miss + assert mock_adapter.get_channel.call_count == 1 + + @pytest.mark.asyncio + async def test_get_channel_cache_invalidated_on_rediscovery( + self, mock_adapter, mock_broker, + ): + """A discovery refresh drops the cache so renames/type-changes are picked up.""" + baseline = _msg(msg_id="100", content="baseline") + first = _msg(msg_id="101", content="first", author_id="user-9") + second = _msg(msg_id="102", content="second", author_id="user-9") + mock_adapter.get_messages.side_effect = [ + [baseline], # priming (during first refresh) + [first], # poll 1 + [second], # poll 2 (after rediscovery) + ] + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + await poller._poll_once() + # Force a rediscovery — should invalidate the cache + await poller._refresh_channels(verbose=False) + await poller._poll_once() + + await asyncio.sleep(0) + + # First poll → 1 lookup, rediscovery clears cache, second poll → 1 more lookup + assert mock_adapter.get_channel.call_count == 2 + + @pytest.mark.asyncio + async def test_broker_delivery_failure_logged(self, mock_adapter, mock_broker, capsys): + """If broker.handle_inbound raises, the failure must be visible in poller logs.""" + baseline = _msg(msg_id="100", content="baseline") + new_msg = _msg(msg_id="101", content="boom", author_id="user-9") + mock_adapter.get_messages.side_effect = [[baseline], [new_msg]] + + async def _raise(*_args, **_kwargs): + raise RuntimeError("broker exploded") + + mock_broker.handle_inbound = AsyncMock(side_effect=_raise) + + poller = self._make_poller(mock_adapter, mock_broker) + await poller._refresh_channels(verbose=True) + await poller._poll_once() + + # Let the fire-and-forget task settle and the done_callback fire. + await asyncio.sleep(0) + await asyncio.sleep(0) + + captured = capsys.readouterr() + assert "broker delivery failed" in captured.err + assert "broker exploded" in captured.err diff --git a/tests/test_pinky_self_tools.py b/tests/test_pinky_self_tools.py index 1d6cf86a..86d41c1d 100644 --- a/tests/test_pinky_self_tools.py +++ b/tests/test_pinky_self_tools.py @@ -1473,6 +1473,102 @@ def test_error(self, srv): result = _tools(srv)["update_and_restart"]() assert "failed" in result.lower() + def test_force_reset_in_output(self, srv): + """force=True → response shows discarded files in human output.""" + resp = { + "updated": True, + "before_hash": "abc123", + "after_hash": "def456", + "commits": [], + "deps_rebuilt": False, + "frontend_rebuilt": False, + "forced_reset": True, + "forced_files": ["frontend-dist/index.html", "frontend-dist/assets/app.js"], + "restarting": True, + } + with _ok(resp): + result = _tools(srv)["update_and_restart"](force=True) + assert "Force reset" in result + assert "2 tracked file" in result + assert "frontend-dist/index.html" in result + + def test_force_url_includes_param(self, srv): + """force=True → request URL includes force=true query param.""" + captured: dict = {} + + def _urlopen(req, timeout=30): + captured["url"] = req.full_url if hasattr(req, "full_url") else str(req) + body = json.dumps({"updated": True, "before_hash": "a", "after_hash": "b", + "commits": [], "deps_rebuilt": False, "frontend_rebuilt": False, + "restarting": False}).encode() + resp = MagicMock() + resp.read.return_value = body + resp.__enter__ = MagicMock(return_value=resp) + resp.__exit__ = MagicMock(return_value=False) + return resp + + with patch("urllib.request.urlopen", side_effect=_urlopen): + _tools(srv)["update_and_restart"](branch="beta", force=True) + + assert "force=true" in captured["url"] + assert "branch=beta" in captured["url"] + + def test_force_deps_in_url(self, srv): + """force_deps=True must reach /admin/update as a query param.""" + captured = {} + + def _urlopen(req, timeout=30): + captured["url"] = req.full_url if hasattr(req, "full_url") else str(req) + body = json.dumps({ + "updated": True, + "before_hash": "a", "after_hash": "a", + "deps_rebuilt": True, "restarting": True, + }).encode() + resp = MagicMock() + resp.read.return_value = body + resp.__enter__ = lambda s: s + resp.__exit__ = MagicMock(return_value=False) + return resp + + with patch("urllib.request.urlopen", side_effect=_urlopen): + _tools(srv)["update_and_restart"](force_deps=True) + assert "force_deps=true" in captured["url"] + + def test_force_deps_with_branch(self, srv): + """force_deps and branch must combine cleanly in the URL.""" + captured = {} + + def _urlopen(req, timeout=30): + captured["url"] = req.full_url if hasattr(req, "full_url") else str(req) + body = json.dumps({ + "updated": True, + "before_hash": "a", "after_hash": "a", + "restarting": False, + }).encode() + resp = MagicMock() + resp.read.return_value = body + resp.__enter__ = lambda s: s + resp.__exit__ = MagicMock(return_value=False) + return resp + + with patch("urllib.request.urlopen", side_effect=_urlopen): + _tools(srv)["update_and_restart"](branch="beta", force_deps=True) + assert "branch=beta" in captured["url"] + assert "force_deps=true" in captured["url"] + + def test_deps_error_surfaced(self, srv): + """Backend deps_error string is surfaced to the caller.""" + with _ok({ + "updated": True, + "before_hash": "a", "after_hash": "b", + "deps_rebuilt": False, + "deps_error": "pip install failed: boom", + "restarting": True, + }): + result = _tools(srv)["update_and_restart"]() + assert "deps rebuild error" in result.lower() + assert "boom" in result + # ── restart_daemon ──────────────────────────────────────────────────────────── diff --git a/tests/test_streaming_session.py b/tests/test_streaming_session.py index a1f187a6..def9bed7 100644 --- a/tests/test_streaming_session.py +++ b/tests/test_streaming_session.py @@ -202,3 +202,177 @@ async def test_idle_sleep_returns_false_when_already_disconnected() -> None: assert result is False assert ss.is_idle_sleeping is False + + +# -- Auth-failure dedupe across AssistantMessage + ResultMessage paths -------- +# +# A single failed turn can surface auth errors on BOTH paths the reader_loop +# watches: an AssistantMessage with error="authentication_failed", followed +# by the terminal ResultMessage with api_error_status=401. Without dedupe, +# AuthFailureTracker.record_failure() would increment twice for one real +# failure — tripping the operator-alert threshold early and skewing the +# host-wide multi-agent baseline. Reader_loop must collapse the two-path +# emission into a single auth-alert-callback invocation per turn. +# +# Per Murzik's PR #404 review: "I verified locally with a synthetic reader +# stream: callback fired as [('agent', 'authentication_failed'), ('agent', +# 'api_error_status=401')]." This test pins down the fix. + + +def _make_assistant_message(*, error: str | None = None): + """Build an AssistantMessage with the minimum fields the reader_loop reads.""" + from claude_agent_sdk.types import AssistantMessage + + return AssistantMessage( + content=[], + model="claude-opus-4-7", + error=error, + usage={"input_tokens": 0, "output_tokens": 0}, + stop_reason="error" if error else "end_turn", + ) + + +def _make_result_message( + *, + is_error: bool = False, + api_error_status: int | None = None, + errors: list[str] | None = None, +): + """Build a ResultMessage with the minimum fields the reader_loop reads.""" + from claude_agent_sdk.types import ResultMessage + + return ResultMessage( + subtype="success" if not is_error else "error", + duration_ms=10, + duration_api_ms=5, + is_error=is_error, + num_turns=1, + session_id="test-session", + stop_reason="end_turn" if not is_error else "error", + api_error_status=api_error_status, + errors=errors, + ) + + +async def _run_reader_against_stream(ss: StreamingSession, messages: list) -> None: + """Wire up a fake client whose receive_messages() yields the given list, + then run the reader_loop until the iterator drains.""" + + async def _receive_messages(): + for msg in messages: + yield msg + + ss._client.receive_messages = _receive_messages + await ss._reader_loop() + + +@pytest.mark.asyncio +async def test_auth_callback_fires_once_when_both_paths_signal_for_same_turn() -> None: + """Regression for PR #404 / Murzik's review. + + SDK emits AssistantMessage(error="authentication_failed") followed by + ResultMessage(is_error=True, api_error_status=401) for a single failed + turn. The auth-alert callback must fire exactly once — not once per path.""" + ss = _make_session() + callback = AsyncMock() + ss._auth_alert_callback = callback + + await _run_reader_against_stream( + ss, + [ + _make_assistant_message(error="authentication_failed"), + _make_result_message( + is_error=True, + api_error_status=401, + errors=["Invalid API key"], + ), + ], + ) + + assert callback.await_count == 1, ( + f"Expected one callback per failed turn (dedupe across " + f"AssistantMessage + ResultMessage paths); got {callback.await_count}: " + f"{callback.await_args_list}" + ) + # The AssistantMessage path should win (it fires first in the stream). + args, _ = callback.await_args + assert args == (ss.agent_name, "authentication_failed") + + +@pytest.mark.asyncio +async def test_auth_callback_fires_on_result_path_when_assistant_path_silent() -> None: + """If the failed turn surfaces only at the ResultMessage (no + AssistantMessage error mid-turn), the result-path detection must still + fire the alert. Dedupe must not break this case.""" + ss = _make_session() + callback = AsyncMock() + ss._auth_alert_callback = callback + + await _run_reader_against_stream( + ss, + [ + _make_result_message( + is_error=True, + api_error_status=403, + errors=["Forbidden"], + ), + ], + ) + + assert callback.await_count == 1 + args, _ = callback.await_args + assert args[0] == ss.agent_name + assert "api_error_status=403" in args[1] + # Enriched detail string should include msg.errors for triage context. + assert "Forbidden" in args[1] + + +@pytest.mark.asyncio +async def test_auth_dedupe_resets_between_turns() -> None: + """Two separate failed turns in one session must produce two callback + invocations — dedupe is per-turn, not per-session.""" + ss = _make_session() + callback = AsyncMock() + ss._auth_alert_callback = callback + + await _run_reader_against_stream( + ss, + [ + # Turn 1: AssistantMessage auth + ResultMessage 401 — one alert + _make_assistant_message(error="authentication_failed"), + _make_result_message(is_error=True, api_error_status=401), + # Turn 2: ResultMessage 401 only — one more alert + _make_result_message(is_error=True, api_error_status=401), + ], + ) + + assert callback.await_count == 2, ( + f"Expected two callbacks (one per failed turn); got {callback.await_count}" + ) + + +@pytest.mark.asyncio +async def test_billing_error_assistant_does_not_block_subsequent_auth_alert() -> None: + """Edge case: AssistantMessage with error="billing_error" (NOT auth) on + one turn must not set the dedupe flag, so a real auth failure on the + NEXT turn still alerts.""" + ss = _make_session() + callback = AsyncMock() + ss._auth_alert_callback = callback + + await _run_reader_against_stream( + ss, + [ + # Turn 1: billing error — must not fire auth alert, must not + # set the dedupe flag (it's not an auth error). + _make_assistant_message(error="billing_error"), + _make_result_message(is_error=True, api_error_status=402), + # Turn 2: real auth failure — must alert. + _make_assistant_message(error="authentication_failed"), + _make_result_message(is_error=True, api_error_status=401), + ], + ) + + assert callback.await_count == 1 + args, _ = callback.await_args + assert args == (ss.agent_name, "authentication_failed") diff --git a/uv.lock b/uv.lock index f20c6cda..990ce522 100644 --- a/uv.lock +++ b/uv.lock @@ -163,7 +163,7 @@ wheels = [ [[package]] name = "anthropic" -version = "0.96.0" +version = "0.98.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "anyio" }, @@ -175,9 +175,9 @@ dependencies = [ { name = "sniffio" }, { name = "typing-extensions" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/b9/7e/672f533dee813028d2c699bfd2a7f52c9118d7353680d9aa44b9e23f717f/anthropic-0.96.0.tar.gz", hash = "sha256:9de947b737f39452f68aa520f1c2239d44119c9b73b0fb6d4e6ca80f00279ee6", size = 658210, upload-time = "2026-04-16T14:28:02.846Z" } +sdist = { url = "https://files.pythonhosted.org/packages/24/60/a9e4426dfe594e5eec8a9757d48e3d8dcf529a0a35a4fc8aefa352bd95fe/anthropic-0.98.1.tar.gz", hash = "sha256:62205edec42f5877df63d58be8e9443843d3e032215836e228fba1f59514a433", size = 725085, upload-time = "2026-05-04T21:40:39.496Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/48/5a/72f33204064b6e87601a71a6baf8d855769f8a0c1eaae8d06a1094872371/anthropic-0.96.0-py3-none-any.whl", hash = "sha256:9a6e335a354602a521cd9e777e92bfd46ba6e115bf9bbfe6135311e8fb2015b2", size = 635930, upload-time = "2026-04-16T14:28:01.436Z" }, + { url = "https://files.pythonhosted.org/packages/9b/6f/7f7f80f714e6de0784518f1999f71fd632076aefd3e22fe0ccd27ca9571f/anthropic-0.98.1-py3-none-any.whl", hash = "sha256:107ebf954415382fdcea6a94f9cf334a53199ad64794403590dc55366cefcc28", size = 699604, upload-time = "2026-05-04T21:40:41.311Z" }, ] [[package]] @@ -508,20 +508,20 @@ wheels = [ [[package]] name = "claude-agent-sdk" -version = "0.1.72" +version = "0.1.77" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "anyio" }, { name = "mcp" }, { name = "sniffio" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/5d/b7/ce35a79f4891797ef60b5cf437453bf22e3e420f5946007312c9e144f5e0/claude_agent_sdk-0.1.72.tar.gz", hash = "sha256:e737f919301d5fc65a3ebc6f438911b73e237b16fa9fa3061420e79727cfa307", size = 241286, upload-time = "2026-05-01T02:17:34.37Z" } +sdist = { url = "https://files.pythonhosted.org/packages/60/d4/7e3ca29d1b6f76639b4bd4becb796ef68a492be3a561c6cd19abe5fe903a/claude_agent_sdk-0.1.77.tar.gz", hash = "sha256:cb292268ecab294047f02365298ecbf8cc17146c4c86fda54b068a1d38e1ebbb", size = 250297, upload-time = "2026-05-08T00:03:03.199Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/e8/de/098dd15dec8ce3c5ed9c9c2415a290fb5c24ad9cd1214cfc3e20c3be0342/claude_agent_sdk-0.1.72-py3-none-macosx_11_0_arm64.whl", hash = "sha256:3a16ee041db0734e8f82d1fca7bd7c122227c0d8c150a301b365ab3f0a83a27b", size = 63695454, upload-time = "2026-05-01T02:17:38.468Z" }, - { url = "https://files.pythonhosted.org/packages/55/de/bea82fdd65244bab9cf6aa3492c2b6cc5d97213836730febd89c0a342975/claude_agent_sdk-0.1.72-py3-none-macosx_11_0_x86_64.whl", hash = "sha256:4b3c630b5b07cd4f3d57631d833539b77045937093ec4975f47ee29de6b9a9df", size = 65539313, upload-time = "2026-05-01T02:17:43.129Z" }, - { url = "https://files.pythonhosted.org/packages/1c/26/78e45a08c4acb262b8f99f6885fb5b96ccf32f27be008610403b86cc3da8/claude_agent_sdk-0.1.72-py3-none-manylinux_2_17_aarch64.whl", hash = "sha256:6fd7c6cbab95928ceb39fe82f413bd124e83dc5ed9bf63b6dffb9dc2b83f67bc", size = 76872182, upload-time = "2026-05-01T02:17:48.616Z" }, - { url = "https://files.pythonhosted.org/packages/58/6d/9641f9a3119adec03d33cb40be8a194801cde0c15b3c497f07e36b38dab1/claude_agent_sdk-0.1.72-py3-none-manylinux_2_17_x86_64.whl", hash = "sha256:e2cf7beb573b608832cf2c644b330e51b79cfdab85286f064cf92f8f63c66382", size = 77032401, upload-time = "2026-05-01T02:17:54.322Z" }, - { url = "https://files.pythonhosted.org/packages/88/a1/35eeda6bac78c5c236c0f3c36fb8634503514ff78ada4e46b77397922ed7/claude_agent_sdk-0.1.72-py3-none-win_amd64.whl", hash = "sha256:9159ac974b496bb50d05027f49b44b76a595f1b4df3bfdef1136b56c95fc956d", size = 78421027, upload-time = "2026-05-01T02:18:00.614Z" }, + { url = "https://files.pythonhosted.org/packages/80/fc/dbb90aff01fe7bdd7708077aaff4b36d5be48a38318fd4ca2216aa64ed87/claude_agent_sdk-0.1.77-py3-none-macosx_11_0_arm64.whl", hash = "sha256:a5697c838cb3a0c78f73698933cd2eea98e8baea99be0740e314fc9e1f69fd4c", size = 60659903, upload-time = "2026-05-08T00:03:06.461Z" }, + { url = "https://files.pythonhosted.org/packages/37/6e/c984d60f4b3cbbf7d84157283338493d11a2b557a2f6389118b5a7f8adc2/claude_agent_sdk-0.1.77-py3-none-macosx_11_0_x86_64.whl", hash = "sha256:a85e3a8cf5d521116d964d28939f25e367d18e5bdb2232cae260b8e8bd9d946f", size = 62721493, upload-time = "2026-05-08T00:03:09.808Z" }, + { url = "https://files.pythonhosted.org/packages/00/83/575e798ce53b58e14d775db104cdb2558706e7f8d22dc647752208fa853f/claude_agent_sdk-0.1.77-py3-none-manylinux_2_17_aarch64.whl", hash = "sha256:16864b91bf369e99590e3ffba82e2d711807caaa4329b11462212779224cd355", size = 70393564, upload-time = "2026-05-08T00:03:13.383Z" }, + { url = "https://files.pythonhosted.org/packages/00/67/50aed27534a9183e204cca33fbef36d0fe5f5145ebf4061c01f2edeaf6db/claude_agent_sdk-0.1.77-py3-none-manylinux_2_17_x86_64.whl", hash = "sha256:7db011c9c8ca4168249f1ce90d4372e13ebab2b122498167e6fb2a5c9c1bd427", size = 70582992, upload-time = "2026-05-08T00:03:16.997Z" }, + { url = "https://files.pythonhosted.org/packages/83/3f/a9d396c46e40d025d4209cf0f6f01a62c70c33b70efceb2d2e991e8ed08c/claude_agent_sdk-0.1.77-py3-none-win_amd64.whl", hash = "sha256:fe7dd006a15a85c1d9a2698d6fffd58e0ce689db1f0040f5b5e4c06d51ce2505", size = 71224295, upload-time = "2026-05-08T00:03:20.695Z" }, ] [[package]] @@ -2254,11 +2254,11 @@ dev = [ [package.metadata] requires-dist = [ - { name = "anthropic", marker = "extra == 'voice'", specifier = ">=0.96" }, + { name = "anthropic", marker = "extra == 'voice'", specifier = ">=0.98" }, { name = "beautifulsoup4", marker = "extra == 'web'", specifier = ">=4.12" }, { name = "caldav", marker = "extra == 'calendar'", specifier = ">=1.3" }, { name = "camoufox", marker = "extra == 'web'", specifier = ">=0.4" }, - { name = "claude-agent-sdk", specifier = ">=0.1.72" }, + { name = "claude-agent-sdk", specifier = ">=0.1.77" }, { name = "cryptography", specifier = ">=41.0" }, { name = "cryptography", marker = "extra == 'calendar'", specifier = ">=41.0" }, { name = "discord-py", marker = "extra == 'discord'", specifier = ">=2.3" },