mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
main
18 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Patrick Buckley
|
2b51b2f8fa | Resolve FastMCP settings before lifecycle tests | ||
|
Patrick Buckley
|
c019ab41d7 |
feat(server): opt-in server.require_project gate
Add an opt-in, default-off `server.require_project` setting. When an admin enables it, creating an interactive chat is refused unless it is filed under a project. The feature is inert and byte-identical when off, and can only ever fail toward "off" (a missing config store or unset key reads as disabled). - settings_registry: server.require_project (bool, default False, live read). - auth: require_project_enabled + require_project_denies_create predicates (service scope / coordinator token_source exempt; NOT admin.coordinator), plus REQUIRE_PROJECT_ERROR / REQUIRE_PROJECT_CODE. - node create gate via a declarative cfg.create_gate_require_project (wired True on the interactive mount only; coordinator spawns stay ungated). - fork/resume: a fork's project is structurally its source's. Any explicit project_id is discarded, so a fork can never be re-filed under an unrelated project (which would move its copied history across a tenancy boundary). Inaccessible / projectless / nonexistent sources are uniform on body and status, so there is no cross-tenant oracle. - console cluster-create proxy surfaces only the coded require_project 400 and masks every other node outcome (401/429/3xx/5xx, un-coded 400) to a sanitized 502, guarding both body reads. - list_projects advisory field + projects.js requireProject() (fail-open). - fresh-create project picker requires an explicit project choice under the flag (no silent auto-select); forks hide the picker (inheritance is server-enforced) and get an accurate refusal message. - tests: predicate matrix, resume-inheritance oracle discriminators, console masking, and end-to-end node-gate mount wiring. |
||
|
Patrick Buckley
|
1a80466369 |
fix(mcp): review round 4 — removal/reconcile lifecycle, honest skip reporting, same-kind debounce recovery
reconcile_sync no longer abandons a DB-driven removal that timed out: both the removal loop and the config-update loop keep the name in _db_managed (and skip the follow-on add) when remove_server_sync returns its mutated-nothing False, so the next pass retries instead of the deleted/reconfigured server serving stale tools until restart. remove_server_sync is now cancel-safe end to end: it FORCE-drops the session before queueing (parked push runners bail at their session gate instead of serializing ≤30s list calls ahead of the removal — the noisy #839 server was exactly the one whose runners could starve its own removal), and wraps the post-lock cleanup in try/finally so a caller-timeout cancel landing mid-teardown still completes the state pop, catalog rebuild, and lock retirement rather than stranding a config-gone ghost catalog. Config survives a park-cancel, so the health loop recovers it. _refresh_all reports None (not a fake ([], [])) for a busy-skip or supersede, stamps a 'skipped' status row, and /mcp refresh renders it distinctly — the operator is no longer told a never-refreshed server is current. A same-kind push lost to the debounce window (the prior runner already finished; the server won't re-announce) arms the health-tick retry, closing the one staleness hole the per-kind debounce still had; a push covered by a queued runner does not arm (no lost change). Static resource/prompt catalogs are capped at connect discovery and every refresh. _list_resource_pair's reap is bounded so a future SDK cancel-regression can't wedge the lock. Cleanups: _arm_refresh_retry (retry-arm gate, ×3), _spawn_full_refresh (discard+spawn, ×3), _popen_mcp_server (live-server spawn, ×2), the tautological stamp-arithmetic TestNotificationDebounce deleted. Suite 9395 green. Refs #839 |
||
|
Patrick Buckley
|
53f11454ad |
fix(mcp): review round 3 — cap static catalogs, atomic removal, unify the list_changed protocol twins
- Static resource/prompt catalogs are now size-capped at connect
discovery AND on every refresh (mirrors the pool twins and the static
tools path): a misbehaving server's push ran uncapped through the new
spawned refresh path and could balloon the shared node's merged
catalogs on every notification.
- remove_server_sync mutates NOTHING outside the per-name lock: the
up-front config pop meant a removal cancelled while parked (behind
the push-refresh runners that now share this lock) left a
half-removed server — config gone, session and published catalogs
alive, no driver able to reconnect or cleanly re-remove. A timed-out
removal is now honestly retryable.
- _refresh_all's DISCONNECTED branch busy-skips too (parking inside
_ensure_static_connected burned the pass's 30s budget on one
mid-reconnect server), and a busy-skip on either branch ARMS the
health-tick retry — an operator-requested refresh can no longer be
silently dropped with output indistinguishable from 'no changes'.
- reconnect_sync drops the session before queueing on the lock (FORCE
semantics already rebuilt live sessions): parked push runners bail
at their session gate instead of serializing up to one 30s list call
per kind ahead of the operator's recovery action. Residual: one
mid-list holder can still precede the 45s attempt; a timed-out
reconnect is honest and retryable.
- _refresh_server's supersede check gains the session arm: a spawned
retry/post-reconnect pass racing an eviction skipped instead of
manufacturing a false 'not connected' error pill (and a re-arm loop)
for a self-healing condition.
- The list_changed protocol twins are UNIFIED (Closes #842): the
admission half (_admit_list_changed) and the runner half
(_run_list_changed_refresh) each exist once as plain parametrized
methods — values and small closures, no factory layer (mcp v2 drops
the factory pattern; the two thin message_handler closures remain
only as SDK-v1 bindings). The one true asymmetry — coalesce-marker
ownership on the superseded path — is a documented boolean: pool
markers are only ever cleared by their runner; static markers are
cleared by remove_server_sync, so a present marker belongs to the
re-added generation. Both runners keep their names and signatures;
the notification suites pass unchanged.
- Cleanups: per-kind staleness rechecks stripped from the static
refreshers (unreachable under the lock discipline — the MUST-hold-
lock contract is documented instead); _run_hl (5th run-on-loop copy)
replaced at 44 call sites; _poll_until centralizes the live-test
wait loops; docs no longer describe the periodic refresh tier
removed in
|
||
|
Patrick Buckley
|
f8f191686f |
fix(mcp): review round 2 — busy-skip the refresh pass, fail-fast list pairs, health-tick refresh retry
- _refresh_server never parks on a held connect lock: the holder is itself a catalog publisher whose publish supersedes the pass, and parking burned refresh_sync's whole 30s budget on ONE busy server (a reconnect attempt holds the lock up to 45s), failing the operator pass for every healthy server queued behind it. Busy → skip (None), no publish, no status writes; the identity/state recheck stays as belt-and-braces for the one-tick check→acquire race. - _list_resource_pair: the ONE copy of the paired resources/templates list protocol (both twins). Fail-fast — a fast real error (auth / method rejection) surfaces as ITSELF instead of being masked behind a hung sibling's eventual 30s TimeoutError — with the survivor CANCELLED and REAPED inside the timeout scope, never left detached on the shared session. - Health-tick refresh retry: there is NO periodic refresh pass (removed in eb2a119d; the docs still claimed the 4h tier — fixed), so a push refresh that failed while the transport stayed up had no automatic recovery and the shared catalog stayed stale for every user until an operator intervened. Failures and busy-skips arm _static_refresh_retry via the shared recorder; the health tick drains it with one bounded, lock-serialized full pass per tick; success, session drops, removal, and the post-reconnect spawns clear it. This also un-latches the error pill: the retry's completion clears it within a tick. - _record_refresh_failure: the bearer-redaction policy (type + message, never exc_info) lives exactly once; all three refresh-failure sites route through it. - Static runner discards its coalesce marker only AFTER the lock-identity check: on the superseded path a marker present in the set belongs to the re-added generation's parked runner, and discarding it would mint duplicates past the one-parked-runner bound (the pool runner deliberately differs — nothing else clears pool markers, so its marker is its own to release). - _clear_static_push_state: the ONE (server, kind) keyspace walk for stamps + retry flag (+ markers on removal). - Tests: busy-skip, superseded-no-status, fail-fast + reap (<5s bound), retry arm/drain/re-arm/clear quartet, logged-wrapper contract updated to the shared recorder's arg shape; vacuous stamp-math test deleted (behavioral per-kind coverage retained); _free_port/_wait_tcp_ready/_wait_session_live hoisted to conftest for both live tests. Refs #839 |
||
|
Patrick Buckley
|
aefcf53405 |
fix(mcp): review round 1 — supersede retired-lock refreshes, per-kind debounce, complete gather pairs
- _refresh_server: post-acquire lock-identity + state-existence recheck; a pass superseded by remove (or remove + re-add) returns None and writes NO status — it must not run its list calls as a second, unserialized publisher against the re-add's discovery wiring, resurrect status rows for a removed server, or stamp a false "ok" over a generation it never refreshed. _refresh_all treats None as a deliberate skip (no breaker success record). - Debounce stamps are per (server, kind) on BOTH paths: refreshes are kind-scoped, so a server-scoped stamp dropped a different-kind notification inside the window outright — a tools push swallowed the prompts push 100ms behind it, and nothing observed the prompt change until the server pushed that kind again. Teardown pops loop the kinds; remove_server_sync also discards the server's coalesce markers so a parked old-generation runner's marker cannot coalesce away a re-added server's first push. - Resource refreshers (static + pool) gather with return_exceptions=True: fail-fast gather left the surviving list call running detached — outside the timeout scope and the lock serialization — as an unbounded in-flight request on the shared session. - Spawned post-reconnect refreshes route through _refresh_server_logged: the re-raise escaped into _spawn_background's done-callback, whose exc_info log serializes the chained httpx.Request carrying the configured bearer for auth_type=static servers; _refresh_all's except drops exc_info for the same reason. Failure diagnostics widen to "Type: message" in logs and the error pill — the message text is header-free; only the serialized chain leaks. - Accepted + documented: connect-lock contention on dispatch reconnects is bounded to one in-flight list call (parked runners bail instantly post-eviction); the error pill persists until the next COMPLETED refresh (a notification's arrival proves nothing about whether the failure resolved). - Tests: per-kind debounce independence, superseded-pass writes nothing, gather-sibling completion, logged-wrapper swallow with the exc_info channel asserted SILENT, remove clears markers; _run_on_loop/_drain_background hoisted to conftest (4 drifted copies); proc.kill() portability in the live push test. Runner-twin dedup (static/pool protocol duplication) deferred to #842. Refs #839 |
||
|
Patrick Buckley
|
9bf8d5699b |
fix(test): harden resolve_when_pending — cancellable worker (review)
Address Copilot review: cancel() now signals the worker to stop (a cancellation Event) and joins only when started, so a test that errors before the approval registers can't leak the worker or resolve late into a finished test. The worker reads _pending_approval via getattr so a UI without it can't crash the thread into a silent death (leaving approve_tools blocked the full timeout), and only resolves when it actually observed the registration. |
||
|
Patrick Buckley
|
c0ff00a1ff |
fix(test): eliminate lost-wakeup race in approval-prompt tests
The UI-approval tests drive a blocking approve_tools() by firing resolve_approval() from a fixed 0.05s threading.Timer. approve_tools does _approval_event.clear() -> register _pending_approval -> wait(3600s); on a slow/loaded runner the timer can fire the event's .set() BEFORE that .clear(), so the wakeup is wiped and approve_tools blocks the full _APPROVAL_WAIT_TIMEOUT (one hour) -- surfacing as an intermittent CI hang (observed on the 3.12 runner ~15% into the suite; fast runners win the race, so 3.11/3.13 pass the same commit). Replace the fixed-delay timer with resolve_when_pending() (tests/conftest.py): it waits until the approval is actually registered -- which happens AFTER the clear -- before resolving, so the set can never be lost. The helper mirrors threading.Timer's start()/cancel() so the surrounding scaffolding is unchanged. 10 sites across 3 files; the verdict-delivery timer (bounded to its own 5s budget, not a hang) is left as-is. Validated: the 3 files pass 20/20 under single-CPU stress (taskset -c 0) with no hang or thread leak. |
||
|
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. |