mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 15:40:18 +00:00
* feat: complete multi-tenant isolation — per-user budgets, model selection, heartbeat cycling Finishes the remaining isolation work from phases 2–4 of #59: Phase 2 (DB scoping): Fix /status and /list commands to use _for_user DB variants instead of global queries that leaked cross-user job data. Phase 3 (Runtime isolation): Per-user workspace in routine engine's spawn_fire so lightweight routines run in the correct user context. Per-user daily cost tracking in CostGuard with configurable budget via MAX_COST_PER_USER_PER_DAY_CENTS. Multi-user heartbeat that cycles through all users with routines, auto-detected from GATEWAY_USER_TOKENS. Phase 4 (Provider/tools): Per-user model selection via preferred_model setting — looked up from SettingsStore on first iteration, threaded through ReasoningContext.model_override to CompletionRequest. Works with providers that support per-request model overrides (NearAI). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: use selected_model setting key to match /model command persistence The dispatcher was reading "preferred_model" but the /model command (merged from staging) persists to "selected_model". Since set_setting is already per-user scoped, using the same key makes /model work as the per-user model override in multi-tenant mode. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: heartbeat hygiene, /model multi-tenant guard, RigAdapter model override Three follow-up fixes for multi-tenant isolation: 1. Multi-user heartbeat now runs memory hygiene per user before each heartbeat check, matching single-user heartbeat behavior. 2. /model command in multi-tenant mode only persists to per-user settings (selected_model) without calling set_model() on the shared LlmProvider. The per-request model_override in the dispatcher reads from the same setting. Added multi_tenant flag to AgentConfig (auto-detected from GATEWAY_USER_TOKENS). 3. RigAdapter now supports per-request model overrides by injecting the model name into rig-core's additional_params. OpenAI/Anthropic/Ollama API servers use last-key-wins for duplicate JSON keys, so the override takes effect via serde's flatten serialization order. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address PR review — cost model attribution, heartbeat concurrency, pruning Fixes from review comments on #1614: - Cost tracking now uses the override model name (not active_model_name) when a per-user model override is active, for accurate attribution. - Multi-user heartbeat runs per-user checks concurrently via JoinSet instead of sequentially, preventing one slow user from blocking others. - Per-user failure counts tracked independently; users exceeding max_failures are skipped (matching single-user semantics). - per_user_daily_cost HashMap pruned on day rollover to prevent unbounded growth in long-lived deployments. - Doc comment fixed: says "routines" not "active routines". Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: /status ownership, model persistence scoping, heartbeat robustness Addresses second round of PR review on #1614: - /status <job_id> DB path now validates job.user_id == requesting user before returning data (was missing ownership check, security fix). - persist_selected_model takes user_id param instead of owner_id, and skips .env/TOML writes in multi-tenant mode (these are shared global files). handle_system_command now receives user_id from caller. - JoinSet collection handles Err(JoinError) explicitly instead of silently dropping panicked tasks. - Notification forwarder extracts owner_id from response metadata in multi-tenant mode for per-user routing instead of broadcasting to the agent owner. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: cost pricing, fire_manual workspace, heartbeat concurrency cap Round 3 review fixes: - Cost tracking passes None for cost_per_token when model override is active, letting CostGuard look up pricing by model name instead of using the default provider's rates (serrrfirat). - fire_manual() now uses per-user workspace, matching spawn_fire() pattern (serrrfirat). - Removed MULTI_TENANT env var — multi-tenant mode is auto-detected solely from GATEWAY_USER_TOKENS presence (serrrfirat + Copilot). - Multi-user heartbeat capped at 8 concurrent tasks to avoid flooding the LLM provider (serrrfirat + Copilot). - Fixed inject_model_override doc comment accuracy (Copilot). - Added comment explaining multi-tenant notification routing priority (Copilot). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat: user-scoped webhook endpoint for multi-tenant isolation Adds POST /api/webhooks/u/{user_id}/{path} — a user-scoped webhook endpoint that filters the routine lookup by user_id, preventing cross-user webhook triggering when paths collide. The existing /api/webhooks/{path} endpoint remains unchanged for backward compatibility in single-user deployments. Changes: - get_webhook_routine_by_path gains user_id: Option<&str> param - Both postgres and libsql implementations add AND user_id = ? filter when user_id is provided - New webhook_trigger_user_scoped_handler extracts (user_id, path) from URL and passes to shared fire_webhook_inner logic - Route registered on public router (webhooks are called by external services that can't send bearer tokens) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat(db): add UserStore trait with users, api_tokens, invitations tables Foundation for DB-backed user management (#1605): - UserRecord, ApiTokenRecord, InvitationRecord types in db/mod.rs - UserStore sub-trait (17 methods) added to Database supertrait - PostgreSQL migration V14__users.sql (users, api_tokens, invitations) - libSQL schema + incremental migration V14 - Full implementations for both PgBackend (via Store delegation) and LibSqlBackend (direct SQL in libsql/users.rs) - authenticate_token JOINs api_tokens+users with active/non-revoked checks; has_any_users for bootstrap detection Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat(web): DB-backed auth, user/token/invitation API handlers Adds the web gateway layer for DB-backed user management (#1605): Auth refactor: - CombinedAuthState wraps env-var tokens (MultiAuthState) + optional DbAuthenticator for DB-backed token lookup with LRU cache (60s TTL, 1024 max entries) - auth_middleware tries env-var tokens first, then DB fallback - From<MultiAuthState> impl for backward compatibility - main.rs wires with_db_auth when database is available API handlers (12 new endpoints): - /api/admin/users — CRUD: create, list, detail, update, suspend, activate - /api/tokens — create (returns plaintext once), list, revoke - /api/invitations — create, list, accept (creates user + first token) Token creation: 32 random bytes → hex plaintext, SHA-256 hash stored. Invitation accept: validates hash + pending + not expired, creates user record and first API token atomically. All test files updated for CombinedAuthState type change. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat: startup env-var user migration + UserStore integration tests Completes the DB-backed user management feature (#1605): - Startup migration: when GATEWAY_USER_TOKENS is set and the users table is empty, inserts env-var users + hashed tokens into DB. Logs deprecation notice when DB already has users. - hash_token made pub for reuse in migration code. - 10 integration tests for UserStore (libsql file-backed): - has_any_users bootstrap detection - create/get/get_by_email/list/update user lifecycle - token create → authenticate → revoke → reject cycle - suspended user tokens rejected - wrong-user token revoke returns false - invitation create → accept → user created - record_login and record_token_usage timestamps - libSQL migration: removed FK constraints from V14 (incompatible with execute_batch inside transactions). Tables in both base SCHEMA and incremental migration for fresh and existing databases. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: remove GATEWAY_USER_TOKENS, fix review feedback GATEWAY_USER_TOKENS never went to production — replaced entirely by DB-backed user management via /api/admin/users and /api/tokens. Removed: - UserTokenConfig struct and GATEWAY_USER_TOKENS env var parsing - user_tokens field from GatewayConfig - GatewayChannel::new_multi_auth() constructor - Env-var user migration block in main.rs (~90 lines) - multi_tenant auto-detection from GATEWAY_USER_TOKENS (now runtime via db.has_any_users() in app.rs) Review fixes (zmanian): - User ID generation: UUID instead of display-name derivation (#1) - Invitation accept moved to public router (no auth needed) (#3) - libSQL get_invitation_by_hash aligned with postgres: filters status='pending' AND expires_at > now (#4) - UUID parse: returns DatabaseError::Serialization instead of unwrap_or_default (#7) - PostgreSQL SELECT * replaced with explicit column lists (#8) - Sort order aligned (both backends use DESC) (#6) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat: add role-based access control (admin/member) Adds a `role` field (admin|member) to user management: Schema: - `role TEXT NOT NULL DEFAULT 'member'` added to users table in both PostgreSQL V14 migration and libSQL schema/incremental migration - UserRecord gains `role: String` field - UserIdentity gains `role: String` field, populated from DB in DbAuthenticator and defaulting to "admin" for single-user mode Access control: - AdminUser extractor: returns 403 Forbidden if role != "admin" - /api/admin/users/* handlers: require AdminUser (create, list, detail, update, suspend, activate) - POST /api/invitations: requires AdminUser (only admins can invite) - User creation accepts optional "role" param (defaults to "member") - Invitation acceptance creates users with "member" role Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat(web): add Users admin tab to web UI Adds a Users tab to the web gateway UI for managing users, tokens, and roles without needing direct API calls. Features: - User list table with ID, name, email, role, status, created date - Create user form with display name, email, role selector - Suspend/activate actions per user - Create API token for any user (shows plaintext once with copy button) - Role badges (admin highlighted, member muted) - Non-admin users see "Admin access required" message - Keyboard shortcut: Cmd/Ctrl+5 switches to Users tab CSS: - Reuses routines-table styles for the user list - Badge, token-display, btn-small, btn-danger, btn-primary components Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: move Users to Settings subtab, bootstrap admin user on first run - Moved Users from top-level tab to Settings sidebar subtab (under Skills, before Theme toggle) - On first startup with empty users table, automatically creates an admin user from GATEWAY_USER_ID config with a corresponding API token from GATEWAY_AUTH_TOKEN. This ensures the owner appears in the Users panel immediately. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: user creation shows token, + Token works, no password save popup Three UI/UX fixes: 1. Create user now generates an initial API token and shows it in a copy-able banner instead of triggering the browser's password save dialog. Uses autocomplete="off" and type="text" for email field. 2. "+ Token" button works: exposed createTokenForUser/suspendUser/ activateUser on window for inline onclick handlers in dynamically generated table rows. Token creation uses showTokenBanner helper. 3. Admin token creation: POST /api/tokens now accepts optional "user_id" field when the requesting user is admin, allowing token creation for other users from the Users panel. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: use event delegation for user action buttons (CSP compliance) Inline onclick handlers are blocked by the Content-Security-Policy (script-src 'self' without 'unsafe-inline'). Switched to data-action attributes with a delegated click listener on the users table. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: add i18n for Users subtab, show login link on user creation - Added 'settings.users' i18n key for English and Chinese - Token banner now shows a full login link (domain/?token=xxx) with a Copy Link button, plus the raw token below - Login link works automatically via existing ?token= auto-auth Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: token hash mismatch — hash hex string, not raw bytes Critical auth bug: token creation hashed the raw 32 bytes (hasher.update(token_bytes)) but authentication hashed the hex-encoded string (hash_token(candidate) where candidate is the hex string the user sends). This meant newly created tokens could never authenticate. Fixed all 4 token creation sites (users, tokens, invitations create, invitations accept) to use hash_token(&plaintext_token) which hashes the hex string consistently with the auth lookup path. Removed now-unused sha2::Digest imports from handlers. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: remove invitation system The invitation flow is redundant — admin create user already generates a token and shows a login link. Invitations add complexity without value until email integration exists. Removed: - InvitationRecord struct and 4 UserStore trait methods - invitations table from V14 migration (postgres + both libsql schemas) - PostgreSQL Store methods (create/get/accept/list invitations) - libSQL UserStore invitation methods + row_to_invitation helper - invitations.rs handler file (212 lines) - /api/invitations routes (create, list, accept) - test_invitation_lifecycle test Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat: user deletion, self-service profile, per-user job limits, usage API Four multi-tenancy improvements: 1. User deletion cascade (DELETE /api/admin/users/{id}): Deletes user and all data across 11 user-scoped tables (settings, secrets, routines, memory, jobs, conversations, etc.). Admin only. 2. Self-service profile (GET/PATCH /api/profile): Users can read and update their own display_name and metadata without admin privileges. 3. Per-user job concurrency (MAX_JOBS_PER_USER env var): Scheduler checks active_jobs_for(user_id) before dispatch. Prevents one user from exhausting all job slots. 4. Usage reporting (GET /api/admin/usage?user_id=X&period=day|week|month): Aggregates LLM costs from llm_calls via agent_jobs.user_id. Returns per-user, per-model breakdown of calls, tokens, and cost. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat: add TenantCtx for compile-time tenant isolation Implements zmanian's architectural proposal from #1614 review: two-tier scoped database access (TenantScope/AdminScope) so handler code cannot accidentally bypass tenant scoping. TenantScope (default): wraps user_id + Arc<dyn Database>, auto-binds user_id on every operation. ID-based lookups return None for cross- tenant resources. No escape hatch — forgetting to scope is a compile error. AdminScope (explicit opt-in): cross-tenant access for system-level components (heartbeat, routine engine, self-repair, scheduler, worker). TenantCtx bundles TenantScope + workspace + cost guard + per-user rate limiting. Constructed once per request in handle_message, threaded through all command handlers and ChatDelegate. Key changes: - New src/tenant.rs (~920 lines): TenantScope, AdminScope, TenantCtx, TenantRateState, TenantRateRegistry - All command handlers: user_id: &str → ctx: &TenantCtx - ChatDelegate: cost check/record/settings via self.tenant - System components: store field changed to AdminScope - Config: TENANT_MAX_LLM_CONCURRENT, TENANT_MAX_JOBS_CONCURRENT env vars - Fixes bug: /status <job_id> cross-tenant leak (now auto-filtered) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address PR #1626 review feedback — bounded LRU cache, admin auth, FK cleanup - Replace HashMap with lru::LruCache in DbAuthenticator so the token cache is hard-bounded at 1024 entries (evicts LRU, not just expired) - Gate admin user endpoints (list/detail/update/suspend/activate) with AdminUser extractor so members get 403 instead of full access - Add api_tokens to libSQL delete_user cleanup list to prevent orphaned tokens (libSQL has no FK cascade) - Add regression tests for all three fixes Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: update CA certificates in runtime Docker image Ensures the root certificate bundle is current so TLS handshakes to services like Supabase succeed on Railway. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: resolve CI failures — formatting, no-panics check - Run cargo fmt on test code - Replace .expect() with const NonZeroUsize in DbAuthenticator - Add // safety: comments for test-only code in multi_tenant.rs Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: switch PostgreSQL TLS from rustls to native-tls rustls with rustls-native-certs fails TLS handshake on Railway's slim container (empty or stale root cert store). native-tls delegates to OpenSSL on Linux which handles system certs more reliably. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * Adding user management api * feat: admin secrets provisioning API + API documentation - Add PUT/GET/DELETE /api/admin/users/{id}/secrets/{name} endpoints for application backends to provision per-user secrets (AES-256-GCM encrypted) - Add secrets_store field to GatewayState with builder wiring - Create docs/USER_MANAGEMENT_API.md with full API spec covering users, secrets, tokens, profile, and usage endpoints - Update web gateway CLAUDE.md route table Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: add CatchPanicLayer to capture handler panics Without this, panics in async handlers silently drop the connection and the edge proxy returns a generic 503. Now panics are caught, logged, and returned as 500 with the panic message. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address second-round review — transactional delete, overflow, error logging - C1: Wrap PostgreSQL delete_user() in a transaction so partial cleanup can't leave users in a half-deleted state - M2: Add job_events to delete cleanup (both backends) — FK to agent_jobs without CASCADE would cause FK violation - H1/M4: Cap expires_in_days to 36500 before i64 cast (tokens + secrets) - H2: Validate target user exists before creating admin token to prevent orphan tokens on libSQL - H3: Log DB errors in DbAuthenticator::authenticate() instead of silently swallowing them as 401 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: revert to rustls with webpki-roots fallback for PostgreSQL TLS native-tls/OpenSSL caused silent crashes (segfaults in C code) during DB writes on Railway containers. Switch back to rustls but add webpki-roots as a fallback when system certs are missing, which was the original TLS handshake failure on slim container images. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * chore: update Cargo.lock for rustls + webpki-roots Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * debug: add /api/debug/db-write endpoint to diagnose user insert failure Temporary diagnostic endpoint that tests DB INSERT to users table with full error logging. No auth required. Will be removed after debugging. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * perf: use cargo-chef in Dockerfile for dependency caching Splits the build into planner/deps/builder stages. Dependencies are only recompiled when Cargo.toml or Cargo.lock change. Source-only changes skip straight to the final build stage. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * debug: add tracing to users_create_handler Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: guard created_by FK in user creation handler The auth identity user_id (from owner_id scope) may not match any user row in the DB, causing a FK violation on the created_by column. Check that the referenced user exists before setting created_by. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: collapse GATEWAY_USER_ID into IRONCLAW_OWNER_ID Remove the separate GATEWAY_USER_ID config. The gateway now uses IRONCLAW_OWNER_ID (config.owner_id) directly for auth identity, bootstrap user creation, and workspace scoping. Previously, with_owner_scope() rebinds the auth identity to owner_id while keeping default_sender_id as the gateway user_id. This caused a FK constraint violation when creating users because the auth identity ("default") didn't match any user in the DB ("nearai"). Changes: - Remove GATEWAY_USER_ID env var and gateway_user_id from settings - Remove user_id field from GatewayConfig - Add owner_id parameter to GatewayChannel::new() - Remove with_owner_scope() method - Remove default_sender_id from GatewayState - Remove sender override logic in chat/approval handlers - Remove debug endpoint and tracing from prior debugging - Update all tests and E2E fixtures Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: hide Users tab for non-admins, remove auth hint text - Fetch /api/profile after login and hide the Users settings tab when the user's role is not admin - Remove the "Enter the GATEWAY_AUTH_TOKEN" hint from the login page since tokens are now managed via the admin panel, not .env files Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address review feedback (auth 503, token expiry, CORS PATCH) - DB auth errors now return 503 instead of 401 so outages are distinguishable from invalid tokens (serrrfirat H3) - Cap expires_in_days to 36500 before i64 cast to prevent negative duration from u64 overflow (serrrfirat H1) - Add PATCH to CORS allowed methods for profile/user update endpoints (Copilot) - Stop leaking panic details in CatchPanicLayer response body Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: harden multi-tenant isolation — review fixes from #1614 - Add conversation ownership checks in TenantScope: add_conversation_message, touch_conversation, list_conversation_messages (+ paginated), update_conversation_metadata_field, get_conversation_metadata now return NotFound for conversations not owned by the tenant (cross-tenant data leak) - Fix multi-user heartbeat: clear notify_user_id per runner so notifications persist to the correct user, not the shared config target - Move hygiene tasks into bounded JoinSet instead of unbounded tokio::spawn - Revert send_notification to private visibility (only used within module) - Use effective_model_name() for cost attribution in dispatcher so providers that ignore per-request model overrides report the actual model used - Fix inject_model_override doc comment; add 3 unit tests - Fix heartbeat doc comment ("routines" not "active routines") Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat: add Jobs, Cost, Last Active columns to admin Users table Add UserSummaryStats struct and user_summary_stats() batch query to the UserStore trait (both PostgreSQL and libSQL backends). The admin users list endpoint now fetches per-user aggregates (job count, total LLM spend, most recent activity) in a single query and includes them inline in the response. The frontend Users table displays three new columns. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address review comments and CI formatting failures CI fixes: - cargo fmt fixes in cli/mod.rs and db/tls.rs Security/correctness (from Copilot + serrrfirat + pranavraja99 reviews): - Token create: reject expires_in_days > 36500 with 400 instead of silent clamp - Token create: return 404 when admin targets non-existent user - User create: map duplicate email constraint violations to 409 Conflict - User create: remove unnecessary DB roundtrip for created_by (use AdminUser directly) - DB auth: log warn on DB lookup failures instead of silently swallowing errors - libSQL: add FK constraints on users.created_by and api_tokens.user_id Config fixes: - agent.multi_tenant: resolve from AGENT_MULTI_TENANT env var instead of hardcoding false - heartbeat.multi_tenant: fix doc comment to match actual env-var-based behavior UI fix: - showTokenBanner: pass correct title ("Token created!" vs "User created!") Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address remaining review comments (round 2) - Secrets handlers: normalize name to lowercase before store operations, validate target user_id exists (returns 404 if not found) - libSQL: propagate cost parsing errors instead of unwrap_or_default() in both user_usage_stats and user_summary_stats - users_list_handler: propagate user_summary_stats DB errors (was silently swallowed with unwrap_or_default) - loadUsers: distinguish 401/403 (admin required) from other errors - Docs: fix users.id type (TEXT not UUID), remove "invitation flow" from V14 migration comment Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat: i18n for Users tab, atomic user+token creation, transactional delete_user i18n: - Add 31 translation keys for all Users tab strings (en + zh-CN) - Wire data-i18n attributes on HTML elements (headings, buttons, inputs, table headers, empty state) - Replace all hard-coded strings in app.js with I18n.t() calls Atomic user+token creation: - Add create_user_with_token() to UserStore trait - PostgreSQL: wraps both INSERTs in conn.transaction() with auto-rollback - libSQL: wraps in explicit BEGIN/COMMIT with ROLLBACK on error - Handler uses single atomic call instead of two separate operations Transactional delete_user for libSQL: - Wrap multi-table DELETE cascade in BEGIN/COMMIT transaction - ROLLBACK on any error to prevent partial cleanup / inconsistent state - Matches the PostgreSQL implementation which already used transactions Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: revert V14 migration to match deployed checksum [skip-regression-check] Refinery checksums applied migrations — editing V14__users.sql after it was already applied causes deployment failures. Revert the cosmetic comment changes (added in df40b22f) to restore the original checksum. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: bootstrap onboarding flow for multi-tenant users The bootstrap greeting and workspace seeding only ran for the owner workspace at startup, so new users created via the admin API never received the welcome message or identity files (BOOTSTRAP.md, SOUL.md, AGENTS.md, USER.md, etc.). Three fixes: - tenant_ctx(): seed per-user workspace on first creation via seed_if_empty(), which writes identity files and sets bootstrap_pending when the workspace is truly fresh - handle_message(): check take_bootstrap_pending() on the tenant workspace (not the owner workspace) and persist the greeting to the user's own assistant conversation + broadcast via SSE - WorkspacePool: seed new per-user workspaces in the web gateway so memory tools also see identity files immediately The existing single-user bootstrap in Agent::run() is preserved for non-multi-tenant deployments. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address remaining PR review comments (round 3) - Docs: fix metadata description from "merge patch" to "full replacement" - Secrets: reject expires_in_days > 36500 with 400 (was silently clamped) - libSQL: CAST(SUM(cost) AS TEXT) in user_usage_stats and user_summary_stats to prevent SQLite numeric coercion from crashing get_text() — this was the root cause of the Copilot "SUM returns numeric type" comments - Add 3 regression tests: user_summary_stats (empty + with data) and user_usage_stats (multi-model aggregation) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat: add role change support for users (admin/member toggle) - Add update_user_role() to UserStore trait + both backends (PostgreSQL and libSQL) - Extend PATCH /api/admin/users/{id} to accept optional "role" field with validation (must be "admin" or "member") - Add "Make Admin" / "Make Member" toggle button in Users table actions - Add i18n keys for role change (en + zh-CN) - Update API docs to document the role field on PATCH - Fix test helpers to use fmt_ts() for timestamps (was using SQLite datetime('now') which produces incompatible format for string comparison) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: show live LLM spend in Users table instead of only DB-recorded costs [skip-regression-check] Chat turns record LLM cost in CostGuard (in-memory) but don't create agent_jobs/llm_calls DB rows — those are only written for background jobs. The Users table was querying only from DB, so it showed $0.00 for users who only chatted. Now supplements DB stats with CostGuard.daily_spend_for_user() — the same source displayed in the status bar token counter. Shows whichever is larger (DB historical total vs live daily spend). Also falls back to last_login_at for "Last Active" when no DB job activity exists. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: persist chat LLM calls to DB and fix usage stats query Two root causes for zero usage stats: 1. ChatDelegate only recorded LLM costs to CostGuard (in-memory) — never to the llm_calls DB table. Added DB persistence via TenantScope.record_llm_call() after each chat LLM call, with job_id=NULL and conversation_id=thread_id. 2. user_summary_stats query only joined agent_jobs→llm_calls, missing chat calls (which have job_id=NULL). Redesigned query to start from llm_calls and resolve user_id via COALESCE(agent_jobs.user_id, conversations.user_id) — covers both job and chat LLM calls. Both PostgreSQL and libSQL queries updated. TenantScope gets record_llm_call() method. Tests updated for new query semantics. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address review comments — input validation, cost semantics, panic safety [skip-regression-check] - Validate display_name: trim whitespace, reject empty strings (create + update) - Validate metadata: must be a JSON object, return 400 if not (admin + profile) - secrets_list_handler: verify target user_id exists before listing - Cost display: use DB total directly (chat calls now persist to DB), remove confusing max(db,live) CostGuard fallback - CatchPanicLayer: truncate panic payload to 200 chars in log to limit potential sensitive data exposure Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address Copilot round 5 — docs, secrets consistency, token name, provider field [skip-regression-check] - Docs: users.id note updated to "typically UUID v4 strings (bootstrap admin may use a custom ID)" - secrets_list_handler: return 503 when DB store is None (was falling through to list secrets without user validation) - tokens_create: trim + reject empty token name (matching display_name pattern) - LlmCallRecord.provider: use llm_backend ("nearai","openai") instead of model_name() which returns the model identifier - user_summary_stats zero-LLM users: acceptable — handler already falls back to 0 cost and last_login_at for missing entries Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: DB auth returns 503 on outage, scheduler counts only blocking jobs From serrrfirat review: - DB auth: return Err(()) on database errors so middleware returns 503 instead of silently returning Ok(None) → 401 (auth miss) - Scheduler: add parallel_blocking_count_for() that uses is_parallel_blocking() (Pending/InProgress/Stuck) instead of is_active() for per-user concurrency — Completed/Submitted jobs no longer count against MAX_JOBS_PER_USER From Copilot: - CLAUDE.md: fix secrets route paths from {id} to {user_id} - token_hash: use .as_slice() instead of .to_vec() to avoid heap allocation on every token auth/creation call Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: immediate auth cache invalidation on security-critical actions (zmanian review #6) Add DbAuthenticator::invalidate_user() that evicts all cached entries for a user. Called after: - Suspend user (immediate lockout, was 60s delay) - Activate user (immediate access restoration) - Role change (admin↔member takes effect immediately) - Token revocation (revoked token can't be reused from cache) The DbAuthenticator is shared (via Clone, which Arc-clones the cache) between the auth middleware and GatewayState, so handlers can evict entries from the same cache the middleware reads. Also from zmanian's review: - Items 1-5, 7-11 were already resolved in prior commits - Item 12 (String→enum for status/role) is deferred as a broader refactor Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: last-admin protection, usage stats for chat calls, UTF-8 safe panic truncation Last-admin protection: - Suspend, delete, and role-demotion of the last active admin now return 409 Conflict instead of succeeding and locking out the admin API - Helper is_last_admin() checks active admin count before destructive ops Usage stats: - user_usage_stats() now includes chat LLM calls (job_id=NULL) by joining via conversations.user_id, matching user_summary_stats() - Both PostgreSQL and libSQL queries updated Panic handler: - Use floor_char_boundary(200) instead of byte-index [..200] to prevent panic on multi-byte UTF-8 characters in panic messages Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: workspace seed race, bootstrap atomicity, email trim, secrets upsert response [skip-regression-check] - WorkspacePool: await seed_if_empty() synchronously after inserting into cache (drop lock first to avoid blocking), so callers see identity files immediately instead of racing a background task - Bootstrap admin: use create_user_with_token() for atomic user+token creation, matching the admin create endpoint - Email: trim whitespace, treat empty as None to prevent " " being stored and breaking uniqueness - Secrets PUT: report "updated" vs "created" based on prior existence - Last token_hash.to_vec() → .as_slice() in authenticate_token Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: disable unscoped webhook endpoint in multi-tenant mode [skip-regression-check] The original /api/webhooks/{path} endpoint looks up routines across all users. In multi-tenant mode, anyone who knows the webhook path + secret could trigger another user's routine. Now returns 410 Gone with a message pointing to the scoped endpoint /api/webhooks/u/{user_id}/{path}. Detection uses state.db_auth.is_some() — present only when DB-backed auth is enabled (multi-tenant). Single-user deployments are unaffected. From: standardtoaster review comment Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: webhook multi-tenant check, secrets error propagation, stale doc comment [skip-regression-check] - Webhook: use workspace_pool.is_some() instead of db_auth.is_some() for multi-tenant detection — db_auth is set for any DB deployment, workspace_pool is only set when has_any_users() was true at startup - Secrets: propagate exists() errors instead of unwrap_or(false) so backend outages surface as 500 rather than incorrect "created" status - Config: fix stale workspace_read_scopes comment referencing user_id Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
1393 lines
48 KiB
Rust
1393 lines
48 KiB
Rust
//! Context manager for handling multiple job contexts.
|
|
|
|
use std::collections::HashMap;
|
|
use std::time::Duration;
|
|
|
|
use tokio::sync::RwLock;
|
|
use uuid::Uuid;
|
|
|
|
use crate::context::{JobContext, JobState, Memory};
|
|
use crate::error::JobError;
|
|
|
|
/// Manages contexts for multiple concurrent jobs.
|
|
pub struct ContextManager {
|
|
/// Active job contexts.
|
|
contexts: RwLock<HashMap<Uuid, JobContext>>,
|
|
/// Memory for each job.
|
|
memories: RwLock<HashMap<Uuid, Memory>>,
|
|
/// Maximum concurrent jobs.
|
|
max_jobs: usize,
|
|
}
|
|
|
|
impl ContextManager {
|
|
/// Create a new context manager.
|
|
pub fn new(max_jobs: usize) -> Self {
|
|
Self {
|
|
contexts: RwLock::new(HashMap::new()),
|
|
memories: RwLock::new(HashMap::new()),
|
|
max_jobs,
|
|
}
|
|
}
|
|
|
|
/// Create a new job context.
|
|
pub async fn create_job(
|
|
&self,
|
|
title: impl Into<String>,
|
|
description: impl Into<String>,
|
|
) -> Result<Uuid, JobError> {
|
|
self.create_job_for_user("default", title, description)
|
|
.await
|
|
}
|
|
|
|
/// Create a new job context for a specific user.
|
|
pub async fn create_job_for_user(
|
|
&self,
|
|
user_id: impl Into<String>,
|
|
title: impl Into<String>,
|
|
description: impl Into<String>,
|
|
) -> Result<Uuid, JobError> {
|
|
let context = JobContext::with_user(user_id, title, description);
|
|
let job_id = context.job_id;
|
|
self.insert_context(context).await?;
|
|
Ok(job_id)
|
|
}
|
|
|
|
/// Register a sandbox job with a pre-determined ID.
|
|
///
|
|
/// Unlike `create_job_for_user` (which generates its own UUID), this method
|
|
/// accepts an existing `job_id` — used by `execute_sandbox()` which creates
|
|
/// the UUID before the container so it can be shared with Docker labels and
|
|
/// DB persistence.
|
|
///
|
|
/// The job starts in `InProgress` state since the container is about to be
|
|
/// created. Counts against `max_jobs` like any other job.
|
|
pub async fn register_sandbox_job(
|
|
&self,
|
|
job_id: Uuid,
|
|
user_id: impl Into<String>,
|
|
title: impl Into<String>,
|
|
description: impl Into<String>,
|
|
) -> Result<(), JobError> {
|
|
let mut context = JobContext::with_user(user_id, title, description);
|
|
context.job_id = job_id;
|
|
context.state = JobState::InProgress;
|
|
context.started_at = Some(chrono::Utc::now());
|
|
self.insert_context(context).await
|
|
}
|
|
|
|
/// Check max_jobs limit, insert context, and allocate memory.
|
|
///
|
|
/// Holds the write lock for the entire check-insert to prevent TOCTOU
|
|
/// races where two concurrent calls both pass the parallel_count check.
|
|
async fn insert_context(&self, context: JobContext) -> Result<(), JobError> {
|
|
let mut contexts = self.contexts.write().await;
|
|
let parallel_count = contexts
|
|
.values()
|
|
.filter(|c| c.state.is_parallel_blocking())
|
|
.count();
|
|
|
|
if parallel_count >= self.max_jobs {
|
|
return Err(JobError::MaxJobsExceeded { max: self.max_jobs });
|
|
}
|
|
|
|
let job_id = context.job_id;
|
|
contexts.insert(job_id, context);
|
|
drop(contexts);
|
|
|
|
self.memories
|
|
.write()
|
|
.await
|
|
.insert(job_id, Memory::new(job_id));
|
|
|
|
Ok(())
|
|
}
|
|
|
|
/// Get a job context by ID.
|
|
pub async fn get_context(&self, job_id: Uuid) -> Result<JobContext, JobError> {
|
|
self.contexts
|
|
.read()
|
|
.await
|
|
.get(&job_id)
|
|
.cloned()
|
|
.ok_or(JobError::NotFound { id: job_id })
|
|
}
|
|
|
|
/// Get a mutable reference to update a job context.
|
|
pub async fn update_context<F, R>(&self, job_id: Uuid, f: F) -> Result<R, JobError>
|
|
where
|
|
F: FnOnce(&mut JobContext) -> R,
|
|
{
|
|
let mut contexts = self.contexts.write().await;
|
|
let context = contexts
|
|
.get_mut(&job_id)
|
|
.ok_or(JobError::NotFound { id: job_id })?;
|
|
Ok(f(context))
|
|
}
|
|
|
|
/// Atomically update a job context and return the updated context.
|
|
///
|
|
/// This method holds the write lock for the entire update-and-read sequence,
|
|
/// preventing concurrent workers from interleaving modifications between the
|
|
/// update and the subsequent read (Issue #807: non-transactional context updates).
|
|
/// Use this when you need to update context and immediately persist it to DB.
|
|
pub async fn update_context_and_get<F>(
|
|
&self,
|
|
job_id: Uuid,
|
|
f: F,
|
|
) -> Result<JobContext, JobError>
|
|
where
|
|
F: FnOnce(&mut JobContext),
|
|
{
|
|
let mut contexts = self.contexts.write().await;
|
|
let context = contexts
|
|
.get_mut(&job_id)
|
|
.ok_or(JobError::NotFound { id: job_id })?;
|
|
f(context);
|
|
Ok(context.clone())
|
|
}
|
|
|
|
/// Get job memory.
|
|
pub async fn get_memory(&self, job_id: Uuid) -> Result<Memory, JobError> {
|
|
self.memories
|
|
.read()
|
|
.await
|
|
.get(&job_id)
|
|
.cloned()
|
|
.ok_or(JobError::NotFound { id: job_id })
|
|
}
|
|
|
|
/// Update job memory.
|
|
pub async fn update_memory<F, R>(&self, job_id: Uuid, f: F) -> Result<R, JobError>
|
|
where
|
|
F: FnOnce(&mut Memory) -> R,
|
|
{
|
|
let mut memories = self.memories.write().await;
|
|
let memory = memories
|
|
.get_mut(&job_id)
|
|
.ok_or(JobError::NotFound { id: job_id })?;
|
|
Ok(f(memory))
|
|
}
|
|
|
|
/// List all active job IDs.
|
|
pub async fn active_jobs(&self) -> Vec<Uuid> {
|
|
self.contexts
|
|
.read()
|
|
.await
|
|
.iter()
|
|
.filter(|(_, c)| c.state.is_active())
|
|
.map(|(id, _)| *id)
|
|
.collect()
|
|
}
|
|
|
|
/// List all job IDs.
|
|
pub async fn all_jobs(&self) -> Vec<Uuid> {
|
|
self.contexts.read().await.keys().cloned().collect()
|
|
}
|
|
|
|
/// List all active job IDs for a specific user.
|
|
pub async fn active_jobs_for(&self, user_id: &str) -> Vec<Uuid> {
|
|
self.contexts
|
|
.read()
|
|
.await
|
|
.iter()
|
|
.filter(|(_, c)| c.user_id == user_id && c.state.is_active())
|
|
.map(|(id, _)| *id)
|
|
.collect()
|
|
}
|
|
|
|
/// Count jobs consuming a parallel execution slot for a specific user.
|
|
///
|
|
/// Uses `is_parallel_blocking()` (Pending/InProgress/Stuck) rather than
|
|
/// `is_active()`, so Completed/Submitted jobs don't count against the
|
|
/// per-user concurrency limit.
|
|
pub async fn parallel_blocking_count_for(&self, user_id: &str) -> usize {
|
|
self.contexts
|
|
.read()
|
|
.await
|
|
.iter()
|
|
.filter(|(_, c)| c.user_id == user_id && c.state.is_parallel_blocking())
|
|
.count()
|
|
}
|
|
|
|
/// List all job IDs for a specific user.
|
|
pub async fn all_jobs_for(&self, user_id: &str) -> Vec<Uuid> {
|
|
self.contexts
|
|
.read()
|
|
.await
|
|
.iter()
|
|
.filter(|(_, c)| c.user_id == user_id)
|
|
.map(|(id, _)| *id)
|
|
.collect()
|
|
}
|
|
|
|
/// Get count of active jobs.
|
|
pub async fn active_count(&self) -> usize {
|
|
self.contexts
|
|
.read()
|
|
.await
|
|
.values()
|
|
.filter(|c| c.state.is_active())
|
|
.count()
|
|
}
|
|
|
|
/// Remove a completed job (cleanup).
|
|
pub async fn remove_job(&self, job_id: Uuid) -> Result<(JobContext, Memory), JobError> {
|
|
let context = self
|
|
.contexts
|
|
.write()
|
|
.await
|
|
.remove(&job_id)
|
|
.ok_or(JobError::NotFound { id: job_id })?;
|
|
|
|
let memory = self
|
|
.memories
|
|
.write()
|
|
.await
|
|
.remove(&job_id)
|
|
.ok_or(JobError::NotFound { id: job_id })?;
|
|
|
|
Ok((context, memory))
|
|
}
|
|
|
|
/// Find stuck jobs.
|
|
///
|
|
/// Returns jobs that are explicitly in `Stuck` state, plus `InProgress`
|
|
/// jobs that have been running longer than `elapsed_threshold` (if provided).
|
|
/// The threshold-based detection catches jobs that never transitioned to
|
|
/// `Stuck` (e.g., due to a deadlock or unhandled timeout).
|
|
pub async fn find_stuck_jobs(&self) -> Vec<Uuid> {
|
|
self.find_stuck_jobs_with_threshold(None).await
|
|
}
|
|
|
|
/// Find stuck jobs with an optional elapsed threshold for `InProgress` detection.
|
|
pub async fn find_stuck_jobs_with_threshold(
|
|
&self,
|
|
elapsed_threshold: Option<Duration>,
|
|
) -> Vec<Uuid> {
|
|
let now = chrono::Utc::now();
|
|
self.contexts
|
|
.read()
|
|
.await
|
|
.iter()
|
|
.filter(|(_, c)| {
|
|
// Always include explicitly Stuck jobs.
|
|
if c.state == crate::context::JobState::Stuck {
|
|
return true;
|
|
}
|
|
// Detect InProgress jobs that have been running beyond the elapsed threshold.
|
|
// NOTE: `started_at` is set on the first transition to InProgress and is
|
|
// NOT reset when a job recovers from Stuck back to InProgress. This means
|
|
// a recovered job may be re-detected on the next scan. A future improvement
|
|
// could track `in_progress_since` or use the most recent StateTransition
|
|
// with `to == InProgress` to avoid false positives on recovered jobs.
|
|
if c.state == crate::context::JobState::InProgress
|
|
&& let Some(threshold) = elapsed_threshold
|
|
&& let Some(started) = c.started_at
|
|
{
|
|
let elapsed = now.signed_duration_since(started);
|
|
let elapsed_secs = elapsed.num_seconds().max(0) as u64;
|
|
return elapsed_secs > threshold.as_secs();
|
|
}
|
|
false
|
|
})
|
|
.map(|(id, _)| *id)
|
|
.collect()
|
|
}
|
|
|
|
/// Get summary of all jobs.
|
|
pub async fn summary(&self) -> ContextSummary {
|
|
let contexts = self.contexts.read().await;
|
|
|
|
let mut summary = ContextSummary::default();
|
|
for ctx in contexts.values() {
|
|
match ctx.state {
|
|
crate::context::JobState::Pending => summary.pending += 1,
|
|
crate::context::JobState::InProgress => summary.in_progress += 1,
|
|
crate::context::JobState::Completed => summary.completed += 1,
|
|
crate::context::JobState::Submitted => summary.submitted += 1,
|
|
crate::context::JobState::Accepted => summary.accepted += 1,
|
|
crate::context::JobState::Failed => summary.failed += 1,
|
|
crate::context::JobState::Stuck => summary.stuck += 1,
|
|
crate::context::JobState::Cancelled => summary.cancelled += 1,
|
|
}
|
|
}
|
|
|
|
summary.total = contexts.len();
|
|
summary
|
|
}
|
|
|
|
/// Get summary of all jobs for a specific user.
|
|
pub async fn summary_for(&self, user_id: &str) -> ContextSummary {
|
|
let contexts = self.contexts.read().await;
|
|
|
|
let mut summary = ContextSummary::default();
|
|
for ctx in contexts.values().filter(|c| c.user_id == user_id) {
|
|
match ctx.state {
|
|
crate::context::JobState::Pending => summary.pending += 1,
|
|
crate::context::JobState::InProgress => summary.in_progress += 1,
|
|
crate::context::JobState::Completed => summary.completed += 1,
|
|
crate::context::JobState::Submitted => summary.submitted += 1,
|
|
crate::context::JobState::Accepted => summary.accepted += 1,
|
|
crate::context::JobState::Failed => summary.failed += 1,
|
|
crate::context::JobState::Stuck => summary.stuck += 1,
|
|
crate::context::JobState::Cancelled => summary.cancelled += 1,
|
|
}
|
|
}
|
|
|
|
summary.total = summary.pending
|
|
+ summary.in_progress
|
|
+ summary.completed
|
|
+ summary.submitted
|
|
+ summary.accepted
|
|
+ summary.failed
|
|
+ summary.stuck
|
|
+ summary.cancelled;
|
|
summary
|
|
}
|
|
}
|
|
|
|
impl Default for ContextManager {
|
|
fn default() -> Self {
|
|
Self::new(10)
|
|
}
|
|
}
|
|
|
|
/// Summary of all job contexts.
|
|
#[derive(Debug, Default)]
|
|
pub struct ContextSummary {
|
|
pub total: usize,
|
|
pub pending: usize,
|
|
pub in_progress: usize,
|
|
pub completed: usize,
|
|
pub submitted: usize,
|
|
pub accepted: usize,
|
|
pub failed: usize,
|
|
pub stuck: usize,
|
|
pub cancelled: usize,
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
#[tokio::test]
|
|
async fn test_create_job() {
|
|
let manager = ContextManager::new(5);
|
|
let job_id = manager.create_job("Test", "Description").await.unwrap();
|
|
|
|
let context = manager.get_context(job_id).await.unwrap();
|
|
assert_eq!(context.title, "Test");
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn test_create_job_for_user_sets_user_id() {
|
|
let manager = ContextManager::new(5);
|
|
let job_id = manager
|
|
.create_job_for_user("user-123", "Test", "Description")
|
|
.await
|
|
.unwrap();
|
|
|
|
let context = manager.get_context(job_id).await.unwrap();
|
|
assert_eq!(context.user_id, "user-123");
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn test_max_jobs_limit() {
|
|
let manager = ContextManager::new(2);
|
|
|
|
manager.create_job("Job 1", "Desc").await.unwrap();
|
|
manager.create_job("Job 2", "Desc").await.unwrap();
|
|
|
|
// Start the jobs to make them active
|
|
for job_id in manager.all_jobs().await {
|
|
manager
|
|
.update_context(job_id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
}
|
|
|
|
// Third job should fail
|
|
let result = manager.create_job("Job 3", "Desc").await;
|
|
assert!(matches!(result, Err(JobError::MaxJobsExceeded { max: 2 })));
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn test_update_context() {
|
|
let manager = ContextManager::new(5);
|
|
let job_id = manager.create_job("Test", "Desc").await.unwrap();
|
|
|
|
manager
|
|
.update_context(job_id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
let context = manager.get_context(job_id).await.unwrap();
|
|
assert_eq!(context.state, crate::context::JobState::InProgress);
|
|
}
|
|
|
|
// === QA Plan P3 - 4.2: Concurrent job stress tests ===
|
|
|
|
#[tokio::test]
|
|
async fn concurrent_creates_produce_unique_ids() {
|
|
let manager = std::sync::Arc::new(ContextManager::new(100));
|
|
|
|
let handles: Vec<_> = (0..50)
|
|
.map(|i| {
|
|
let mgr = std::sync::Arc::clone(&manager);
|
|
tokio::spawn(async move {
|
|
mgr.create_job(format!("Job {i}"), format!("Desc {i}"))
|
|
.await
|
|
})
|
|
})
|
|
.collect();
|
|
|
|
let mut ids = std::collections::HashSet::new();
|
|
for handle in handles {
|
|
let result = handle.await.expect("task should not panic");
|
|
let job_id = result.expect("create_job should succeed");
|
|
assert!(ids.insert(job_id), "Duplicate job ID: {job_id}");
|
|
}
|
|
|
|
assert_eq!(ids.len(), 50);
|
|
assert_eq!(manager.all_jobs().await.len(), 50);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn concurrent_creates_respect_max_jobs_limit() {
|
|
// max_jobs = 5, but create_job only counts *active* jobs (InProgress).
|
|
// Pending jobs don't count against the limit, so we need to transition them.
|
|
let manager = std::sync::Arc::new(ContextManager::new(5));
|
|
|
|
// First, create 5 jobs and make them active.
|
|
for i in 0..5 {
|
|
let id = manager
|
|
.create_job(format!("Job {i}"), "desc")
|
|
.await
|
|
.unwrap();
|
|
manager
|
|
.update_context(id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
}
|
|
|
|
// Now try to create 10 more concurrently -- all should fail.
|
|
let handles: Vec<_> = (0..10)
|
|
.map(|i| {
|
|
let mgr = std::sync::Arc::clone(&manager);
|
|
tokio::spawn(async move { mgr.create_job(format!("Overflow {i}"), "desc").await })
|
|
})
|
|
.collect();
|
|
|
|
for handle in handles {
|
|
let result = handle.await.expect("task should not panic");
|
|
assert!(
|
|
matches!(result, Err(JobError::MaxJobsExceeded { .. })),
|
|
"Expected MaxJobsExceeded, got: {:?}",
|
|
result
|
|
);
|
|
}
|
|
|
|
// Still exactly 5 jobs.
|
|
assert_eq!(manager.all_jobs().await.len(), 5);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn concurrent_creates_and_reads_no_corruption() {
|
|
let manager = std::sync::Arc::new(ContextManager::new(100));
|
|
|
|
// Spawn writers that create jobs.
|
|
let writer_handles: Vec<_> = (0..20)
|
|
.map(|i| {
|
|
let mgr = std::sync::Arc::clone(&manager);
|
|
tokio::spawn(async move {
|
|
mgr.create_job_for_user(
|
|
format!("user-{}", i % 5),
|
|
format!("Job {i}"),
|
|
format!("Description for job {i}"),
|
|
)
|
|
.await
|
|
})
|
|
})
|
|
.collect();
|
|
|
|
// Concurrently, spawn readers that list jobs.
|
|
let reader_handles: Vec<_> = (0..20)
|
|
.map(|_| {
|
|
let mgr = std::sync::Arc::clone(&manager);
|
|
tokio::spawn(async move {
|
|
let _all = mgr.all_jobs().await;
|
|
let _active = mgr.active_jobs().await;
|
|
let _summary = mgr.summary().await;
|
|
})
|
|
})
|
|
.collect();
|
|
|
|
// Wait for all writers.
|
|
let mut ids = Vec::new();
|
|
for handle in writer_handles {
|
|
let result = handle.await.expect("writer should not panic");
|
|
ids.push(result.expect("create should succeed"));
|
|
}
|
|
|
|
// Wait for all readers.
|
|
for handle in reader_handles {
|
|
handle.await.expect("reader should not panic");
|
|
}
|
|
|
|
// All 20 jobs created with unique IDs.
|
|
let unique: std::collections::HashSet<_> = ids.iter().collect();
|
|
assert_eq!(unique.len(), 20);
|
|
|
|
// Each user has 4 jobs (20 jobs / 5 users).
|
|
for u in 0..5 {
|
|
let user_jobs = manager.all_jobs_for(&format!("user-{u}")).await;
|
|
assert_eq!(user_jobs.len(), 4, "user-{u} should have 4 jobs");
|
|
}
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn concurrent_updates_do_not_lose_state() {
|
|
let manager = std::sync::Arc::new(ContextManager::new(100));
|
|
|
|
// Create 10 jobs.
|
|
let mut job_ids = Vec::new();
|
|
for i in 0..10 {
|
|
let id = manager
|
|
.create_job(format!("Job {i}"), "desc")
|
|
.await
|
|
.unwrap();
|
|
job_ids.push(id);
|
|
}
|
|
|
|
// Concurrently transition all to InProgress.
|
|
let handles: Vec<_> = job_ids
|
|
.iter()
|
|
.map(|&id| {
|
|
let mgr = std::sync::Arc::clone(&manager);
|
|
tokio::spawn(async move {
|
|
mgr.update_context(id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
})
|
|
})
|
|
.collect();
|
|
|
|
for handle in handles {
|
|
let result = handle.await.expect("task should not panic");
|
|
result
|
|
.expect("update should succeed")
|
|
.expect("transition should succeed");
|
|
}
|
|
|
|
// All 10 should now be InProgress.
|
|
let active = manager.active_jobs().await;
|
|
assert_eq!(active.len(), 10);
|
|
for id in &job_ids {
|
|
let ctx = manager.get_context(*id).await.unwrap();
|
|
assert_eq!(ctx.state, crate::context::JobState::InProgress);
|
|
}
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn get_context_not_found() {
|
|
let manager = ContextManager::new(5);
|
|
let bogus_id = Uuid::new_v4();
|
|
let result = manager.get_context(bogus_id).await;
|
|
assert!(matches!(result, Err(JobError::NotFound { id }) if id == bogus_id));
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn update_context_not_found() {
|
|
let manager = ContextManager::new(5);
|
|
let bogus_id = Uuid::new_v4();
|
|
let result = manager.update_context(bogus_id, |_ctx| {}).await;
|
|
assert!(matches!(result, Err(JobError::NotFound { id }) if id == bogus_id));
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn remove_job_returns_context_and_memory() {
|
|
let manager = ContextManager::new(5);
|
|
let job_id = manager.create_job("Removable", "bye bye").await.unwrap();
|
|
|
|
let (ctx, mem) = manager.remove_job(job_id).await.unwrap();
|
|
assert_eq!(ctx.title, "Removable");
|
|
assert_eq!(mem.job_id, job_id);
|
|
|
|
// After removal, get should fail
|
|
assert!(matches!(
|
|
manager.get_context(job_id).await,
|
|
Err(JobError::NotFound { .. })
|
|
));
|
|
assert!(matches!(
|
|
manager.get_memory(job_id).await,
|
|
Err(JobError::NotFound { .. })
|
|
));
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn remove_job_not_found() {
|
|
let manager = ContextManager::new(5);
|
|
let result = manager.remove_job(Uuid::new_v4()).await;
|
|
assert!(matches!(result, Err(JobError::NotFound { .. })));
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn get_memory_and_update_memory() {
|
|
let manager = ContextManager::new(5);
|
|
let job_id = manager.create_job("Mem test", "desc").await.unwrap();
|
|
|
|
// Fresh memory should be empty
|
|
let mem = manager.get_memory(job_id).await.unwrap();
|
|
assert_eq!(mem.job_id, job_id);
|
|
assert!(mem.actions.is_empty());
|
|
assert!(mem.conversation.is_empty());
|
|
|
|
// Update memory by adding a message
|
|
manager
|
|
.update_memory(job_id, |m| {
|
|
m.add_message(crate::llm::ChatMessage::user("hello from test"));
|
|
})
|
|
.await
|
|
.unwrap();
|
|
|
|
let mem = manager.get_memory(job_id).await.unwrap();
|
|
assert_eq!(mem.conversation.len(), 1);
|
|
assert_eq!(mem.conversation.messages()[0].content, "hello from test");
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn update_memory_not_found() {
|
|
let manager = ContextManager::new(5);
|
|
let result = manager.update_memory(Uuid::new_v4(), |_| {}).await;
|
|
assert!(matches!(result, Err(JobError::NotFound { .. })));
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn get_memory_not_found() {
|
|
let manager = ContextManager::new(5);
|
|
let result = manager.get_memory(Uuid::new_v4()).await;
|
|
assert!(matches!(result, Err(JobError::NotFound { .. })));
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn find_stuck_jobs_returns_only_stuck() {
|
|
let manager = ContextManager::new(10);
|
|
|
|
let id1 = manager.create_job("Job 1", "desc").await.unwrap();
|
|
let id2 = manager.create_job("Job 2", "desc").await.unwrap();
|
|
let id3 = manager.create_job("Job 3", "desc").await.unwrap();
|
|
|
|
// Transition id1 and id2 to InProgress, then mark id2 as stuck
|
|
for id in [id1, id2, id3] {
|
|
manager
|
|
.update_context(id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
}
|
|
manager
|
|
.update_context(id2, |ctx| ctx.mark_stuck("timed out"))
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
let stuck = manager.find_stuck_jobs().await;
|
|
assert_eq!(stuck.len(), 1);
|
|
assert_eq!(stuck[0], id2);
|
|
}
|
|
|
|
/// Regression test for #1223: InProgress jobs exceeding the threshold
|
|
/// should be detected as stuck even if they never transitioned to Stuck.
|
|
#[tokio::test]
|
|
async fn find_stuck_jobs_with_threshold_detects_idle_in_progress() {
|
|
let manager = ContextManager::new(10);
|
|
|
|
let id1 = manager.create_job("Active job", "desc").await.unwrap();
|
|
let id2 = manager.create_job("Idle job", "desc").await.unwrap();
|
|
|
|
// Both transition to InProgress
|
|
for id in [id1, id2] {
|
|
manager
|
|
.update_context(id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
}
|
|
|
|
// Backdate id2's started_at to simulate a long-running job
|
|
manager
|
|
.update_context(id2, |ctx| -> Result<(), crate::error::JobError> {
|
|
ctx.started_at = Some(chrono::Utc::now() - chrono::Duration::seconds(600));
|
|
Ok(())
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
// With a 5-minute threshold, only id2 (10 min) should be detected
|
|
let stuck = manager
|
|
.find_stuck_jobs_with_threshold(Some(Duration::from_secs(300)))
|
|
.await;
|
|
assert_eq!(stuck.len(), 1);
|
|
assert_eq!(stuck[0], id2);
|
|
|
|
// Without threshold, neither InProgress job is detected (no explicit Stuck state)
|
|
let stuck_no_threshold = manager.find_stuck_jobs().await;
|
|
assert!(stuck_no_threshold.is_empty());
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn active_count_tracks_non_terminal_jobs() {
|
|
let manager = ContextManager::new(10);
|
|
|
|
let id1 = manager.create_job("J1", "d").await.unwrap();
|
|
let id2 = manager.create_job("J2", "d").await.unwrap();
|
|
|
|
// Both pending (active)
|
|
assert_eq!(manager.active_count().await, 2);
|
|
|
|
// Transition id1 through to Failed (terminal)
|
|
manager
|
|
.update_context(id1, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
manager
|
|
.update_context(id1, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::Failed, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
// id1 is terminal, id2 still pending
|
|
assert_eq!(manager.active_count().await, 1);
|
|
|
|
// Transition id2 to cancelled
|
|
manager
|
|
.update_context(id2, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::Cancelled, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
assert_eq!(manager.active_count().await, 0);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn active_jobs_for_filters_by_user() {
|
|
let manager = ContextManager::new(10);
|
|
|
|
manager
|
|
.create_job_for_user("alice", "A1", "d")
|
|
.await
|
|
.unwrap();
|
|
manager
|
|
.create_job_for_user("alice", "A2", "d")
|
|
.await
|
|
.unwrap();
|
|
let bob_id = manager.create_job_for_user("bob", "B1", "d").await.unwrap();
|
|
|
|
assert_eq!(manager.active_jobs_for("alice").await.len(), 2);
|
|
assert_eq!(manager.active_jobs_for("bob").await.len(), 1);
|
|
assert_eq!(manager.active_jobs_for("nobody").await.len(), 0);
|
|
|
|
// Make bob's job terminal
|
|
manager
|
|
.update_context(bob_id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
manager
|
|
.update_context(bob_id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::Failed, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
assert_eq!(manager.active_jobs_for("bob").await.len(), 0);
|
|
// But all_jobs_for still shows it
|
|
assert_eq!(manager.all_jobs_for("bob").await.len(), 1);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn summary_counts_states_correctly() {
|
|
let manager = ContextManager::new(10);
|
|
|
|
let id1 = manager.create_job("J1", "d").await.unwrap();
|
|
let id2 = manager.create_job("J2", "d").await.unwrap();
|
|
let id3 = manager.create_job("J3", "d").await.unwrap();
|
|
|
|
// id1: Pending -> InProgress -> Completed
|
|
manager
|
|
.update_context(id1, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
manager
|
|
.update_context(id1, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::Completed, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
// id2: Pending -> InProgress -> Failed
|
|
manager
|
|
.update_context(id2, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
manager
|
|
.update_context(id2, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::Failed, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
// id3: stays Pending
|
|
|
|
let s = manager.summary().await;
|
|
assert_eq!(s.total, 3);
|
|
assert_eq!(s.pending, 1);
|
|
assert_eq!(s.completed, 1);
|
|
assert_eq!(s.failed, 1);
|
|
assert_eq!(s.in_progress, 0);
|
|
assert_eq!(s.stuck, 0);
|
|
assert_eq!(s.cancelled, 0);
|
|
assert_eq!(s.submitted, 0);
|
|
assert_eq!(s.accepted, 0);
|
|
|
|
// Suppress unused field warning
|
|
let _ = id3;
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn summary_for_scopes_to_user() {
|
|
let manager = ContextManager::new(10);
|
|
|
|
manager
|
|
.create_job_for_user("alice", "A1", "d")
|
|
.await
|
|
.unwrap();
|
|
let bob_id = manager.create_job_for_user("bob", "B1", "d").await.unwrap();
|
|
|
|
// Transition bob's job to InProgress
|
|
manager
|
|
.update_context(bob_id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
let alice_summary = manager.summary_for("alice").await;
|
|
assert_eq!(alice_summary.total, 1);
|
|
assert_eq!(alice_summary.pending, 1);
|
|
assert_eq!(alice_summary.in_progress, 0);
|
|
|
|
let bob_summary = manager.summary_for("bob").await;
|
|
assert_eq!(bob_summary.total, 1);
|
|
assert_eq!(bob_summary.pending, 0);
|
|
assert_eq!(bob_summary.in_progress, 1);
|
|
|
|
let nobody_summary = manager.summary_for("nobody").await;
|
|
assert_eq!(nobody_summary.total, 0);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn default_context_manager_has_max_10() {
|
|
let manager = ContextManager::default();
|
|
// Create 10 jobs and make them active
|
|
for i in 0..10 {
|
|
let id = manager
|
|
.create_job(format!("Job {i}"), "desc")
|
|
.await
|
|
.unwrap();
|
|
manager
|
|
.update_context(id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
}
|
|
// 11th should fail
|
|
let result = manager.create_job("overflow", "d").await;
|
|
assert!(matches!(result, Err(JobError::MaxJobsExceeded { max: 10 })));
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn all_jobs_returns_all_regardless_of_state() {
|
|
let manager = ContextManager::new(10);
|
|
|
|
let id1 = manager.create_job("J1", "d").await.unwrap();
|
|
manager.create_job("J2", "d").await.unwrap();
|
|
|
|
// Make id1 terminal
|
|
manager
|
|
.update_context(id1, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
manager
|
|
.update_context(id1, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::Failed, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
// all_jobs includes terminal, active_jobs does not
|
|
assert_eq!(manager.all_jobs().await.len(), 2);
|
|
assert_eq!(manager.active_jobs().await.len(), 1);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn create_job_uses_default_user() {
|
|
let manager = ContextManager::new(5);
|
|
let job_id = manager.create_job("Test", "desc").await.unwrap();
|
|
let ctx = manager.get_context(job_id).await.unwrap();
|
|
assert_eq!(ctx.user_id, "default");
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn concurrent_remove_and_read() {
|
|
let manager = std::sync::Arc::new(ContextManager::new(100));
|
|
|
|
// Create 20 jobs
|
|
let mut job_ids = Vec::new();
|
|
for i in 0..20 {
|
|
let id = manager
|
|
.create_job(format!("Job {i}"), "desc")
|
|
.await
|
|
.unwrap();
|
|
job_ids.push(id);
|
|
}
|
|
|
|
// Concurrently remove the first 10 while reading the last 10
|
|
let remove_handles: Vec<_> = job_ids[..10]
|
|
.iter()
|
|
.map(|&id| {
|
|
let mgr = std::sync::Arc::clone(&manager);
|
|
tokio::spawn(async move { mgr.remove_job(id).await })
|
|
})
|
|
.collect();
|
|
|
|
let read_handles: Vec<_> = job_ids[10..]
|
|
.iter()
|
|
.map(|&id| {
|
|
let mgr = std::sync::Arc::clone(&manager);
|
|
tokio::spawn(async move { mgr.get_context(id).await })
|
|
})
|
|
.collect();
|
|
|
|
for handle in remove_handles {
|
|
handle
|
|
.await
|
|
.expect("remove task should not panic")
|
|
.expect("remove should succeed");
|
|
}
|
|
|
|
for handle in read_handles {
|
|
let ctx = handle
|
|
.await
|
|
.expect("read task should not panic")
|
|
.expect("read should succeed");
|
|
assert!(job_ids[10..].contains(&ctx.job_id));
|
|
}
|
|
|
|
assert_eq!(manager.all_jobs().await.len(), 10);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn update_context_and_get_atomicity_regression_issue_807() {
|
|
// Regression test for Issue #807: non-transactional context updates.
|
|
// Verify that update_context_and_get returns the exact state that was set,
|
|
// without allowing concurrent workers to interleave modifications.
|
|
let manager = std::sync::Arc::new(ContextManager::new(100));
|
|
let job_id = manager
|
|
.create_job("Atomicity Test", "verify no race condition")
|
|
.await
|
|
.unwrap(); // safety: test code
|
|
|
|
// Update and get atomically, setting metadata
|
|
let metadata = serde_json::json!({ "priority": "high", "user_id": 42 });
|
|
let returned_ctx = manager
|
|
.update_context_and_get(job_id, |ctx| {
|
|
ctx.metadata = metadata.clone();
|
|
ctx.max_tokens = 5000;
|
|
})
|
|
.await
|
|
.unwrap(); // safety: test code
|
|
|
|
// Verify the returned context has the exact updates we set
|
|
assert_eq!(returned_ctx.metadata, metadata); // safety: test code
|
|
assert_eq!(returned_ctx.max_tokens, 5000); // safety: test code
|
|
|
|
// Verify a fresh get returns the same state
|
|
let fresh_ctx = manager.get_context(job_id).await.unwrap(); // safety: test code
|
|
assert_eq!(fresh_ctx.metadata, metadata); // safety: test code
|
|
assert_eq!(fresh_ctx.max_tokens, 5000); // safety: test code
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn update_context_and_get_no_concurrent_interleave() {
|
|
// Verify that concurrent updates cannot interleave during update_context_and_get.
|
|
// If the lock were released too early, a concurrent state transition could
|
|
// get mixed into the returned context.
|
|
let manager = std::sync::Arc::new(ContextManager::new(100));
|
|
let job_id = manager
|
|
.create_job("Concurrent Race Test", "ensure atomicity")
|
|
.await
|
|
.unwrap(); // safety: test code
|
|
|
|
let metadata = serde_json::json!({ "test": "race_condition" });
|
|
let metadata_clone = metadata.clone();
|
|
|
|
// Spawn a task that will update_context_and_get
|
|
let mgr1 = std::sync::Arc::clone(&manager);
|
|
let returned_ctx_handle = tokio::spawn(async move {
|
|
mgr1.update_context_and_get(job_id, |ctx| {
|
|
ctx.metadata = metadata_clone;
|
|
ctx.max_tokens = 3000;
|
|
})
|
|
.await
|
|
});
|
|
|
|
// The returned context should have *only* the metadata update, not any
|
|
// concurrent state transitions that might happen during the operation.
|
|
let returned_ctx = returned_ctx_handle.await.unwrap().unwrap(); // safety: test code
|
|
|
|
// Verify atomicity: returned context has the metadata we set
|
|
assert_eq!(returned_ctx.metadata, metadata); // safety: test code
|
|
assert_eq!(returned_ctx.max_tokens, 3000); // safety: test code
|
|
// And it's in the initial state (Pending), not modified by concurrent workers
|
|
assert_eq!(returned_ctx.state, crate::context::JobState::Pending); // safety: test code
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn sequential_routines_unlimited_completed_not_counted() {
|
|
// TEST: Sequential (non-parallel) routines should NOT be limited by max_jobs.
|
|
//
|
|
// Completed/Submitted jobs should NOT count toward the parallel job limit,
|
|
// since they're no longer actively consuming execution resources.
|
|
//
|
|
// Scenario: Create 10 sequential routines, each completing before the next starts.
|
|
// Currently FAILS because Completed jobs still count as "active".
|
|
// After fix, should PASS because only Pending/InProgress/Stuck count.
|
|
|
|
let manager = ContextManager::new(5); // max 5 truly parallel jobs
|
|
|
|
// Try to create and complete 10 sequential routines
|
|
for i in 0..10 {
|
|
let result = manager
|
|
.create_job(format!("Sequential Routine {}", i), "one at a time")
|
|
.await;
|
|
|
|
match result {
|
|
Ok(job_id) => {
|
|
// Simulate execution: Pending -> InProgress -> Completed
|
|
manager
|
|
.update_context(job_id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
manager
|
|
.update_context(job_id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::Completed, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
println!("✓ Routine {} created and completed", i);
|
|
}
|
|
Err(JobError::MaxJobsExceeded { max }) => {
|
|
panic!(
|
|
"✗ Routine {} FAILED to create: MaxJobsExceeded (max={}).\n\
|
|
This shows the bug: Completed jobs from routines 0-4 are still counting \
|
|
toward the limit even though they're not running.\n\
|
|
After the fix, this test should pass because Completed jobs won't count.",
|
|
i, max
|
|
);
|
|
}
|
|
Err(e) => {
|
|
panic!("Unexpected error for routine {}: {:?}", i, e);
|
|
}
|
|
}
|
|
}
|
|
|
|
// If we reach here, all 10 routines succeeded (bug is fixed)
|
|
assert_eq!(manager.all_jobs().await.len(), 10);
|
|
println!("✓ SUCCESS: All 10 sequential routines created despite max_jobs=5 limit");
|
|
println!(" This is correct: Completed jobs don't count toward parallel limit");
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn parallel_jobs_limit_enforced_for_active_jobs() {
|
|
// TEST: Parallel (simultaneous) jobs ARE limited by max_jobs.
|
|
//
|
|
// Jobs in Pending/InProgress/Stuck states consume execution slots.
|
|
// The 6th truly-active job should fail because the limit is 5.
|
|
//
|
|
// This test verifies the limit DOES work correctly for parallel execution.
|
|
|
|
let manager = ContextManager::new(5); // max 5 parallel jobs
|
|
|
|
// Create 5 jobs and make them InProgress (simulating parallel execution)
|
|
let mut job_ids = Vec::new();
|
|
for i in 0..5 {
|
|
let job_id = manager
|
|
.create_job(format!("Parallel Job {}", i), "running in parallel")
|
|
.await
|
|
.expect("First 5 jobs should create successfully");
|
|
job_ids.push(job_id);
|
|
|
|
// Transition to InProgress (simulating active execution)
|
|
manager
|
|
.update_context(job_id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
}
|
|
|
|
// Verify all 5 jobs are InProgress
|
|
for job_id in &job_ids {
|
|
let ctx = manager.get_context(*job_id).await.unwrap();
|
|
assert_eq!(
|
|
ctx.state,
|
|
crate::context::JobState::InProgress,
|
|
"All jobs should be InProgress"
|
|
);
|
|
}
|
|
|
|
// Check active count - should be 5 (all InProgress)
|
|
let active_count = manager.active_count().await;
|
|
assert_eq!(
|
|
active_count, 5,
|
|
"Active count should be 5 (all InProgress jobs count)"
|
|
);
|
|
|
|
// Try to create a 6th job - should FAIL because limit is reached
|
|
let result = manager.create_job("Parallel Job 6", "sixth job").await;
|
|
|
|
match result {
|
|
Err(JobError::MaxJobsExceeded { max: 5 }) => {
|
|
println!("✓ SUCCESS: Parallel job limit correctly enforced at 5 active jobs");
|
|
println!("✓ 6th InProgress job correctly blocked when 5 are already running");
|
|
}
|
|
Ok(_) => {
|
|
panic!(
|
|
"FAILED: 6th parallel job should have been blocked \
|
|
but was created. Limit enforcement is broken."
|
|
);
|
|
}
|
|
Err(e) => {
|
|
panic!(
|
|
"UNEXPECTED ERROR: Expected MaxJobsExceeded but got: {:?}",
|
|
e
|
|
);
|
|
}
|
|
}
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn completed_jobs_should_free_slots_after_fix() {
|
|
// TEST: After the fix, Completed jobs should NOT count toward the limit.
|
|
//
|
|
// This test demonstrates that when a job transitions from InProgress -> Completed,
|
|
// it should free up a slot in the parallel execution limit.
|
|
//
|
|
// Currently FAILS (bug not fixed), proving Completed jobs incorrectly stay in the limit.
|
|
// After fix, this will PASS (Completed jobs freed their slot).
|
|
|
|
let manager = ContextManager::new(5); // max 5 parallel jobs
|
|
|
|
// Create 5 InProgress jobs (fill the limit)
|
|
let mut job_ids = Vec::new();
|
|
for i in 0..5 {
|
|
let job_id = manager
|
|
.create_job(format!("Job {}", i), "parallel")
|
|
.await
|
|
.unwrap();
|
|
job_ids.push(job_id);
|
|
|
|
manager
|
|
.update_context(job_id, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
}
|
|
|
|
// Verify limit is hit
|
|
let result = manager.create_job("Job 5", "should fail").await;
|
|
assert!(
|
|
matches!(result, Err(JobError::MaxJobsExceeded { max: 5 })),
|
|
"Limit should be hit with 5 InProgress jobs"
|
|
);
|
|
println!("✓ Limit enforced: 5 InProgress jobs block 6th creation");
|
|
|
|
// Now transition job 0 from InProgress -> Completed
|
|
manager
|
|
.update_context(job_ids[0], |ctx| {
|
|
ctx.transition_to(crate::context::JobState::Completed, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
println!("✓ Job 0 transitioned: InProgress -> Completed");
|
|
|
|
// Try to create a 6th job - this will FAIL until the bug is fixed
|
|
let result = manager
|
|
.create_job("Job 5 (retry)", "after 1 Completed")
|
|
.await;
|
|
|
|
match result {
|
|
Ok(job_6) => {
|
|
println!("✓ SUCCESS: 6th job created after job 0 completed");
|
|
println!("✓ This proves Completed jobs don't count toward the limit (BUG FIXED)");
|
|
|
|
// Verify we can transition it to InProgress
|
|
manager
|
|
.update_context(job_6, |ctx| {
|
|
ctx.transition_to(crate::context::JobState::InProgress, None)
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
println!("✓ 6th job now InProgress: 4 remaining + 1 new = 5 limit reached");
|
|
}
|
|
Err(JobError::MaxJobsExceeded { max: 5 }) => {
|
|
panic!(
|
|
"✗ BUG NOT FIXED: 6th job creation still blocked after freeing slot.\n\
|
|
State: 1 Completed (job 0) + 4 InProgress (jobs 1-4) = 5 active\n\
|
|
BUG: Completed job 0 still counts toward limit\n\
|
|
EXPECTED: Only 4 InProgress count, 1 slot free"
|
|
);
|
|
}
|
|
Err(e) => {
|
|
panic!("Unexpected error: {:?}", e);
|
|
}
|
|
}
|
|
}
|
|
|
|
// === Regression: sandbox jobs must be visible to query tools ===
|
|
// Before the fix, execute_sandbox() only persisted to DB but never
|
|
// registered in ContextManager, making sandbox jobs invisible to
|
|
// list_jobs, job_status, job_events, and resolve_job_id.
|
|
|
|
#[tokio::test]
|
|
async fn register_sandbox_job_visible_to_queries() {
|
|
let manager = ContextManager::new(5);
|
|
let job_id = Uuid::new_v4();
|
|
|
|
manager
|
|
.register_sandbox_job(
|
|
job_id,
|
|
"user-42",
|
|
"Run tests",
|
|
"Execute test suite in sandbox",
|
|
)
|
|
.await
|
|
.unwrap();
|
|
|
|
// Job should be retrievable by ID (used by job_status, job_events)
|
|
let ctx = manager.get_context(job_id).await.unwrap();
|
|
assert_eq!(ctx.job_id, job_id);
|
|
assert_eq!(ctx.user_id, "user-42");
|
|
assert_eq!(ctx.title, "Run tests");
|
|
assert_eq!(ctx.state, JobState::InProgress);
|
|
assert!(ctx.started_at.is_some());
|
|
|
|
// Job should appear in all_jobs (used by resolve_job_id prefix matching)
|
|
let all = manager.all_jobs().await;
|
|
assert!(all.contains(&job_id));
|
|
|
|
// Job should appear in user-scoped listing (used by list_jobs)
|
|
let user_jobs = manager.all_jobs_for("user-42").await;
|
|
assert!(user_jobs.contains(&job_id));
|
|
|
|
// Job should appear in active jobs listing
|
|
let active = manager.active_jobs_for("user-42").await;
|
|
assert!(active.contains(&job_id));
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn register_sandbox_job_respects_max_jobs() {
|
|
let manager = ContextManager::new(2);
|
|
|
|
// Fill up the slots with sandbox jobs
|
|
manager
|
|
.register_sandbox_job(Uuid::new_v4(), "user-1", "Job 1", "desc")
|
|
.await
|
|
.unwrap();
|
|
manager
|
|
.register_sandbox_job(Uuid::new_v4(), "user-1", "Job 2", "desc")
|
|
.await
|
|
.unwrap();
|
|
|
|
// Third should fail
|
|
let result = manager
|
|
.register_sandbox_job(Uuid::new_v4(), "user-1", "Job 3", "desc")
|
|
.await;
|
|
assert!(matches!(result, Err(JobError::MaxJobsExceeded { max: 2 })));
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn register_sandbox_job_transitions_correctly() {
|
|
let manager = ContextManager::new(5);
|
|
let job_id = Uuid::new_v4();
|
|
|
|
manager
|
|
.register_sandbox_job(job_id, "user-1", "Task", "desc")
|
|
.await
|
|
.unwrap();
|
|
|
|
// Should be able to transition InProgress -> Completed
|
|
manager
|
|
.update_context(job_id, |ctx| ctx.transition_to(JobState::Completed, None))
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
let ctx = manager.get_context(job_id).await.unwrap();
|
|
assert_eq!(ctx.state, JobState::Completed);
|
|
}
|
|
}
|