mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-27 14:24:47 -06:00
perf/webui-transcript-windowing
10 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Patrick Buckley
|
73f4fb5933 |
test: address Copilot review on the leaked-thread guard
- The guard snapshotted live threads by `Thread.ident`, but idents are recycled after a thread exits — a new leaked thread reusing an exited thread's ident would be mistaken for pre-existing and missed (false negative). Snapshot the Thread OBJECTS and compare by identity instead. - Fix the `serve` fixture docstring: the factory returns the ephemeral port, not the server. |
||
|
Patrick Buckley
|
a7d8895287 |
test: eliminate leaked-thread test pollution + guard against it
Background daemons, event loops, and test servers that outlived their test bled into later tests' captured output — an intermittent "I/O operation on closed file" heisenbug, and the same class behind a past multi-day CI-hang investigation. - conftest: a fail-on-leak autouse guard (`_no_leaked_threads`) snapshots threads at setup and fails any test that leaves one running past teardown, with an `allow_thread_leak` opt-out — so the next leak is caught in minutes, not days. Plus `logging.raiseExceptions = False` to mute the benign logging-vs-capture-teardown race, and shared loop/server teardown helpers (`stop_loop_thread`, `serve_until_exit`). - collector (PRODUCT FIX): the node-discovery loop slept uninterruptibly, so `ClusterCollector.stop()` couldn't join the `console-discovery` thread until the full interval elapsed — a real shutdown hang in production (up to `discovery_interval`). It now sleeps on an interruptible Event that `stop()` sets and `start()` clears. - test fixtures: docker_healthcheck's HTTP servers, the MCP background event loops (shutdown_default_executor + close), and the FastMCP uvicorn upstreams (timeout_graceful_shutdown=0 + force_exit) now tear down cleanly instead of leaking. Full non-live suite: 7456 passed, 0 closed-file errors, 0 leaked threads, and ~1.5 min faster (the leaks were dragging it). |
||
|
Patrick Buckley
|
29c42c1427 |
feat(mcp): per-(user, server) OAuth 2.1 + PKCE flow
Lands the OAuth flow that uses the token-at-rest store from the prior
commit: discovery (RFC 9728 PRM + RFC 8414 AS metadata with operator-
override precedence), PKCE S256 (mandatory — refuse AS without it),
RFC 8707 resource indicator on every authorize and token request,
RFC 7591 minimal one-shot dynamic client registration, authorization-
code exchange, refresh-token grant with re-read-after-acquire single-
flight lock, and the /v1/api/mcp/oauth/{start,callback} endpoints
mounted on both server and console.
Refactored:
- validate_url_no_ssrf, validate_discovered_endpoint, is_localhost,
effective_port, sanitize_log_text moved out of oidc.py into a shared
oauth_ssrf module; oidc.py re-exports for compatibility. The shared
helpers also expose async wrappers (validate_url_no_ssrf_async,
validate_discovered_endpoint_async) so OAuth-MCP discovery — invoked
from async handlers — does not block the event loop on the
synchronous socket.getaddrinfo call.
- MCPTokenStore.get_oauth_client_secret reader path added (the prior
commit was write-only)
- Storage protocol gains create/pop/cleanup_*_mcp_oauth_pending_state
and get_mcp_oauth_client_secret_ct (mirror OIDC pending-state
pattern: SQLite BEGIN IMMEDIATE select-then-delete, Postgres atomic
DELETE...RETURNING)
Refresh-grant correctness:
- When the AS omits refresh_token (RFC 6749 §6 — MAY rotate), the
existing refresh value is preserved at the OAuth-flow layer rather
than cleared, so production ASes (Google, Auth0 default, Okta) don't
force re-consent every hour
- expires_in accepts int, float, str-with-decimal — earlier int-coerce
through str() failed on float and silently dropped expiry tracking
- The refresh-grant `resource=` parameter (RFC 8707) is the canonical
MCP server URL, not the audience. Audience and resource are distinct
concepts; using audience as resource would mismatch the AS RS
allowlist.
Audience handling:
- _validate_token_audience accepts str or tuple; the callback resolves
accepted_audiences = {server_url, oauth_audience} and validates
against the set, so Auth0-style ASes that honor `audience=` (not
RFC 8707 `resource=`) issue tokens that pass audience-bound
validation
- build_authorize_url emits both `resource=` (RFC 8707) and
`audience=` (Auth0-style) per server config; comment documents which
AS implementations need which form
Security hardening:
- redirect_uri pinned to oidc_config.redirect_base instead of the
request Host header — closes the same Host-header injection PR #476
fixed for OIDC. Both /start and /callback return 503 with operator-
actionable hint when redirect_base is unset
- DCR registration runs under per-server asyncio.Lock with re-fetch
inside the lock, so concurrent /start callers don't both register
and overwrite each other's client_id (the second user's code is no
longer rejected on callback)
- /callback error branch pops the pending state row before redirecting
so a leaked state can't be replayed against a separately-obtained
code in the 60s cleanup window
- WWW-Authenticate Bearer parser handles RFC 7235 quoted-string
escapes (\" and \\) instead of the naive [^"]+ regex
- AS-controlled response bodies and error_description query params go
through sanitize_log_text before reaching exception messages or
audit details. AS error responses are parsed for the standard
RFC 6749 fields (error, error_description, error_uri), each
capped at 80 chars and run through redact_credentials to defend
against ASes that echo the request body back into their error
payload.
- oauth_as_issuer_cached is re-validated against the SSRF guard on
read; on rejection the column is cleared and PRM rediscovery runs
- DCR / token-endpoint / refresh-endpoint response bodies cap at 64
KiB (PRM/AS metadata cap stays at 256 KiB) so a hostile or
malfunctioning AS can't exhaust client memory.
- oauth_client_secret operator input capped at 1024 chars at the
admin-form boundary; longer plaintext rejected with 400.
- /start and /callback responses stamp `X-Frame-Options: DENY` so the
redirected pages can't be framed by attacker sites.
- delete_user cascades to mcp_user_tokens and mcp_oauth_pending so
user deletion no longer leaves dangling per-user OAuth state.
- Renaming or deleting an oauth_user MCP server purges per-user
tokens and pending OAuth state for the previous server name
(delete_mcp_oauth_rows_by_server_name). The OAuth tables key on the
mutable server_name; without this purge, a future server with the
same name (and an attacker-controlled URL) would silently rebind
prior user tokens. A future schema migration will replace the
server_name key with a server_id FK + ON DELETE CASCADE.
- get_user_access_token catches MCPTokenDecryptError (raised when no
installed key can decrypt the row, e.g. after key rotation) and
falls through to None so dispatch surfaces a re-consent rather than
crashing.
- oauth_user MCP server rows are skipped in the static auto-connect
path. Auto-connecting them at startup with empty headers fails the
AS check and trips the circuit breaker; per-user tokens come online
lazily once the user has consented.
Audit (mcp_server.oauth.* prefix):
- consent_started, consent_completed, consent_failed, token_refreshed,
token_revoked, dcr_registered. _audit_event is async and wraps
record_audit in asyncio.to_thread so the audit write doesn't block
the event loop. resource_id on the audit row is the immutable
server_id (PK UUID) so admin-driven server renames don't break
event correlation; server_name is exposed in detail for cross-
reference. dcr_registered detail.has_secret reflects whether the
DCR-issued secret was actually persisted (the prior code reported
has_secret=true even on persistence failure).
- _admin_mcp_action audits the immutable server_id, not the mutable
server_name (which is what the column is — the table's PK was
always server_id).
- All OAuth-flow log keys use the mcp_server.oauth.* prefix to match
the audit-action taxonomy.
Lifespan close-order in turnstone.server and turnstone.console.server
is reversed (LIFO) — mcp_oauth → mcp_crypto → oidc — to match init
order.
Deferred until the upcoming per-user pool integration:
- Multi-node refresh-lock contention via pg_advisory_lock
- DCR re-register on token-endpoint 401 (the dispatch path surfaces
those 401s)
- TTL-LRU caching of decrypted plaintext access tokens
- DNS-rebinding hardening (httpx Transport pin) — documented as
limitation in oauth_ssrf module docstring
Tests: 7 new test files / ~85 new tests covering discovery precedence
+ PRM quoted-string parsing, PKCE round-trip, SSRF helper extraction,
authorize/callback handlers including 503-on-no-redirect-base + DCR
concurrency + JWT audience polymorphism + callback-error-pops-pending,
refresh single-flight lock, refresh resource-vs-audience regression,
decrypt-error fallthrough, _db_servers_to_config skipping oauth_user,
pending-state CRUD round-trip.
|
||
|
Patrick Buckley
|
be0950bb98 |
refactor(mcp): consolidate per-server state into StaticServerState dataclass
Phase 0 of the OAuth-MCP RFC: prepare MCPClientManager for the per-(user, server) session pool that lands in Phase 5, without changing static-path behavior. Two changes: 1. Hardening helpers _pre_close_streams and _tcp_probe rename their first parameter from `name` to `key`. Type stays `str` for now; widening to `str | tuple[str, str]` happens in Phase 5 when callers actually pass tuples. _safe_close_stack takes the stack directly and is unchanged. 2. The eleven parallel name-keyed dicts (_sessions, _per_server_stacks, _per_server_tools, _per_server_resources, _per_server_prompts, _supports_list_changed, _supports_resources, _supports_resource_list_changed, _supports_prompts, _supports_prompt_list_changed, _server_streams) are consolidated into _static_servers: dict[str, StaticServerState]. Server- level state (circuit breaker, notification debounce, last-error, db-managed, merged catalog maps, listener lists) stays on the manager, unchanged. PoolEntryState is defined for Phase 5 use but no code instantiates it. The typed map declarations (dict[str, StaticServerState] vs dict[tuple[str, str], PoolEntryState]) make accidental cross-keying lookups easier to catch. PR #296 hardening preserved exactly: - pre-close-streams atomic take-and-clear before stack teardown - stale-session-and-stack guard at _connect_one top: both state.session and state.stack checked, cleared independently, entry preserved (not popped) - transport-error session-eviction in dispatch sets state.session=None only, leaving stack/streams for the next connect-time guard sweep - _safe_close_stack CancelledError suppression unchanged - TCP probe before streamablehttp_client unchanged - future.cancel() after TimeoutError in all sync bridges unchanged - notification debounce stays manager-level (not migrated into the dataclass) Refresh helpers (_refresh_server_tools/_resources/_prompts) snapshot state.session into a local immediately after the None guard so concurrent transport-error eviction during await cannot null the session reference mid-call. Tests: shared _seed_static_state helper in tests/conftest.py replaces eleven direct dict mutations; new test_reconnect_preserves_static_state_identity guards the entry-preservation invariant. Pass count rises 5266 → 5267. |
||
|
Patrick Buckley
|
5d4a50d2cd |
chore(oidc): consolidate test OIDCConfig helper + fix exceptions banner (cumulative q-4, q-5)
q-4: tests/test_oidc.py's _make_config and tests/test_oidc_handlers.py's _make_oidc_config built the same OIDCConfig with sensible defaults but had drifted — only the handlers helper set redirect_base. After b3 made redirect_base operationally required, every test_oidc.py test that exercised redirect_base had to override it explicitly. A future test could omit redirect_base and silently exercise the wrong production path. Moves make_oidc_test_config to tests/conftest.py with the more complete handler-version defaults (including redirect_base). Both test files import it under their existing local alias (_make_config / _make_oidc_config) so the 60+ call sites in test_oidc.py and the handler tests don't have to change. q-5: section banner '# Exception' (singular) at oidc.py:79 became inconsistent after b5 (callback robustness) added OIDCKeyNotFoundError. Renamed to '# Exceptions'. |
||
|
Patrick Buckley
|
fb44652850 |
refactor(core): unify approve_tools across both kinds (#436)
* refactor(core): unify approve_tools across kinds + judge visibility + perf Lift WebUI.approve_tools to SessionUIBase so both interactive and coordinator workstreams run the same body. The shared body now owns tool-policy gating, per-tool auto-approve, blanket carve-out for __budget_override__, activity tagging, heuristic-verdict persistence, and the approve_request/approval_event blocking pattern. Subclass hooks layer kind-specific surfaces on top. This closes the drift the LLM-judge audit flagged on coord — the judge (heuristic + LLM tier) now sees actual tool args for every coord tool call instead of empty func_args. spawn_batch projects the full children list so a malicious mid-batch entry is no longer hidden. = Unification core = - SessionUIBase.approve_tools: lifted body covering policy / per-tool auto-approve / blanket / activity tagging / heuristic-verdict persistence / approval gate - _APPROVAL_WAIT_TIMEOUT class constant + _record_judge_metric hook - WebUI.approve_tools deleted; _record_judge_metric override fires per-node MetricsCollector.record_judge_verdict - ConsoleCoordinatorUI.approve_tools deleted; _record_judge_metric + on_intent_verdict overrides fire ConsoleMetrics.record_judge_verdict - ConsoleMetrics.record_judge_verdict + turnstone_judge_verdicts_total in /metrics text output (cluster PromQL rolls coord+interactive up uniformly) - _console_metrics class attribute wired in console lifespan - Frontend: coord SSE event tools_auto_approved -> tool_info for parity = Judge args visibility = - _evaluate_intent populates func_args for all coord tools that hit approval (spawn_workstream / spawn_batch / send_to_workstream / close_workstream / close_all_children / cancel_workstream / delete_workstream / task_list) - spawn_batch projects every child's skill / initial_message[:200] / target_node so the judge sees the full fan-out (was first child only) - fire_judge_verdict_metric helper collapses 4 sites of identical record_judge_verdict shape across WebUI + ConsoleCoordinatorUI = Hardening = - __budget_override__ carve-out reads from pre-filter items list, not post-filter pending; policy block skips matching the synthetic name entirely so a wildcard `*: allow` cannot strip the override before the gate sees it - _persist_intent_verdict default_tier parameter so heuristic + llm paths share the storage write helper = Performance = - TTL cache on list_tool_policies in turnstone/core/policy.py (60s, keyed by org_id, lock-free hits) - Storage-layer invalidation: create/update/delete_tool_policy on both SQLite and PostgreSQL backends call invalidate_policy_cache (covers admin-API path + direct test fixtures + any future caller) - Admin-API handlers also call invalidate_policy_cache as defense-in-depth - storage.create_intent_verdicts_bulk on both backends: one multi-row INSERT + one commit instead of N round-trips. approve_tools switches to the bulk path so a fan-out turn no longer pays N x commit before the approval prompt enqueues - _persist_intent_verdicts_bulk helper on SessionUIBase = Test coverage = - tests/test_coord_ui_approve_tools.py (NEW, 17 cases): inheritance regression, tool-policy deny/allow/mixed on coord, heuristic verdict persistence (bulk path), activity tagging on auto-approve and pending, judge_pending dynamic flag (true + false), event-name parity, per-tool auto-approve, __budget_override__ carve-out under blanket + wildcard policy, _record_judge_metric wired/unwired, on_intent_verdict llm-tier metric - tests/test_console_metrics.py: 3 cases for the new record_judge_verdict counter - tests/test_judge_storage.py: 3 cases for create_intent_verdicts_bulk - tests/test_coordinator_tools.py: 3 cases pinning the spawn_batch full-children projection (truncation, mid-batch visibility, empty defensive) - tests/conftest.py: autouse _clear_policy_cache fixture so the process-level cache doesn't leak between tests with distinct storage instances = Drift fixes (review feedback) = - Refresh stale "no-op on coord" comments now that coord overrides the hook - WebUI.on_plan_review timeout uses self._APPROVAL_WAIT_TIMEOUT instead of literal 3600 - Drop redundant bool() wrapper around any() in judge_pending - Rephrase broken docstring grammar in _coord_spawn_metrics - Hoist redundant get_storage import out of approve_tools per-item loop (folded into _persist_intent_verdicts_bulk helper) = Validation = - pytest -m "not live": 4679 passed, 3 deselected - ruff check + ruff format: clean - mypy: no issues in 175 source files * fix(approval): apply Copilot feedback on PR #436 - Policy-cache invalidation now drops both the org-scoped slot AND the default ``""`` slot on ``create_tool_policy`` for both SQLite and PostgreSQL backends. ``list_tool_policies("")`` returns rows from every org_id, and the production evaluators (SessionUIBase.approve_tools / cli.py) read with the default ``org_id=""``, so an org-scoped insert that only invalidated its own slot would leave the default cache slot stale until the TTL window expired. - Cap ``reason`` to 200 chars in ``_evaluate_intent`` for ``close_workstream`` and ``close_all_children`` — both fields are LLM/user-provided and the preparer doesn't size-limit them, so an unbounded reason could bloat the persisted verdict row's func_args. Matches the cap applied to other free-form coord tool fields (initial_message, message, title). - Refresh ``_PolicyCache`` docstring: it claimed lock-free reads on cache hit but ``get()`` always acquires ``self._lock``. Updated to reflect that the lock is held briefly to copy the policies reference. Validation: targeted suite 201/201, ruff + mypy clean. |
||
|
Patrick Buckley
|
ab1a71c86c |
feat: add PostgreSQL CI integration tests (#156)
* feat: add PostgreSQL CI integration tests Add --storage-backend pytest option and shared storage_backend fixture in conftest.py that creates SQLiteBackend or PostgreSQLBackend based on the flag. Migrate 13 storage test files to use shared fixture instead of local SQLiteBackend fixtures. Add test-postgres CI job with PostgreSQL 17 service container that runs the full test suite against real PostgreSQL. * fix: use TRUNCATE CASCADE for PG cleanup, wrap in try/finally TRUNCATE is faster than per-table DELETE and resets autoincrement sequences. try/except ensures reset_storage() always runs even if cleanup fails due to a corrupted connection from a failing test. * fix: document _engine coupling in PG cleanup comment |
||
|
Patrick Buckley
|
2b58c127b1 |
Add pluggable storage backend (SQLite + PostgreSQL) and deployment packaging (#20)
* Add pluggable storage backend (SQLite + PostgreSQL) and deployment packaging Database abstraction: StorageBackend protocol with 21 methods, SQLAlchemy Core schema, SQLite backend (FTS5), PostgreSQL backend (tsvector/ILIKE), Alembic migrations, singleton registry. memory.py reduced to thin facade. Session.py open_db() calls replaced with generic KV methods. [database] config section with env var support. Deployment: Docker Compose production profile with PostgreSQL, Dockerfile with postgres extras and migration entrypoint, Helm chart with bitnami subcharts, Terraform AWS ECS/Fargate module with RDS + ElastiCache + ALB. 39 new storage tests (934 total). mypy strict clean. Docs and diagrams updated. * Address PR #20 review feedback (16 items) - Backends only call create_all() when Alembic migrations are disabled - Helm configmap uses correct TURNSTONE_DB_BACKEND env var; DB URL constructed via env expansion with secret reference instead of ConfigMap - Migration errors fail fast for PostgreSQL (only non-fatal for SQLite) - save_memory/delete_memory wrapped in exception handling like other facade fns - pool_size passed through from config/env to init_storage() in cli + server - Terraform: DB URL moved to Secrets Manager, auth enabled flag set, optional TLS listeners with certificate_arn, Redis transit encryption on - Docker entrypoint no longer suppresses migration output - Diagram fixes: removed StaticPool claim, removed non-existent migration ref - compose.yaml/README: clarified production profile requires DB env vars |
||
|
Patrick Buckley
|
9be155b97a |
Quality overhaul: code tooling, CI/CD, architecture diagrams, UI rede… (#1)
* Quality overhaul: code tooling, CI/CD, architecture diagrams, UI redesign, and legacy cleanup - Add ruff (lint+format) and mypy (strict) with zero errors across 37 source files - Add GitHub Actions CI (lint, typecheck, test matrix 3.11/3.12/3.13) and PyPI publish workflow - Create 12 PlantUML architecture diagrams with PNG renders covering all subsystems - Refresh README and docs with badges, diagram links, and current descriptions - Refactor test_server_live.py with mock streaming helpers for deterministic CI testing - Update dependencies to current versions (openai>=2.24, httpx>=0.28, redis>=7.2) Console dashboard: - Move state indicators from top cards to fixed bottom status bar with cluster metrics - Replace flat 50-node list with hostname-prefix grouped nodes (expand/collapse, up to 1000) - Apply "Instrument Panel" visual redesign: IBM Plex Mono + Outfit fonts, warm amber accent, LED glow state indicators, deep charcoal surfaces, WCAG AA contrast compliance - Add render cache, stale indicator, active filter highlight, loading states Server web UI: - Apply matching Instrument Panel aesthetic for visual consistency with console - Fix branding (pcode → turnstone), extract inline styles to CSS classes - Rename pcode localStorage keys and history state to turnstone Legacy cleanup: - Remove persona-model-specific --persona flag and /persona slash command - Remove model_identity from chat_template_kwargs (vLLM-specific mechanism) - Refactor plan agent to use standard developer message instead of model_identity - Remove dead code (unused date/has_tools variables, noqa suppressions) * Fix CI typecheck: add mypy overrides for optional sympy/numpy imports The math sandbox optionally imports sympy and numpy at runtime (try/except ImportError). In CI these packages are not installed, so mypy raises import-not-found rather than import-untyped. Add mypy overrides to ignore missing imports for these optional dependencies. * Fix Copilot review findings: ARIA role, status bar cache, and pulse opacity - Change #node-table from role="tree" to role="list" and group elements from role="treeitem" to role="listitem" (proper ARIA semantics) - Include currentView and currentFilter.state in renderStatusBar cache key so active pill highlight updates when switching views - Align pulse animation to 0.35 opacity (already applied in CSS) |
||
|
Patrick Buckley
|
0d6252dd7d | Initial commit — turnstone multi-node AI orchestration platform. |