Fix: Add missing Docker labels to system agent container - #1
Merged
Conversation
The system agent was missing required Docker labels (trinity.ssh-port, trinity.cpu, trinity.memory, trinity.created), causing port allocation conflicts when creating new agents. Without trinity.ssh-port label: - get_agent_status_from_container() returns port=0 - get_next_available_port() filters out port 0 - System agent's port (2290) appears available - New agents fail with 'port already allocated' error This fix adds all required labels to match the standard agent creation pattern in routers/agents.py, ensuring proper port tracking and conflict prevention.
vybe
added a commit
that referenced
this pull request
Dec 27, 2025
Bug #1: Terminal session lost when switching tabs - Changed v-if to v-show for terminal tab content in AgentDetail.vue - Keeps terminal component mounted, preserving WebSocket connection Bug #2: MCP deploy_local_agent only copied CLAUDE.md - Updated startup.sh to copy ALL template files instead of hardcoded list - Now includes template.yaml and custom directories (src/, lib/, etc.) - Added .trinity-initialized marker to prevent re-copying on restart 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
6 tasks
This was referenced Apr 20, 2026
Closed
4 tasks
dolho
added a commit
that referenced
this pull request
Apr 23, 2026
Move the pre-check exec logic out of routers/internal.py into a dedicated services/pre_check_service.py. Aligns with Invariant #1 (Router → Service → DB): the router becomes a 5-line passthrough, all business logic (path constant, two exec calls, stdout capping, contract dict) lives in the service. Same shape as services/slot_service.py, services/task_execution_service.py, services/monitoring_service.py — testable in isolation, discoverable via grep, won't trip /validate-architecture later. Behavior unchanged. 13/13 scheduler unit tests still pass; live endpoint hit returns same contract dict. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dolho
added a commit
that referenced
this pull request
Apr 24, 2026
Move the pre-check exec logic out of routers/internal.py into a dedicated services/pre_check_service.py. Aligns with Invariant #1 (Router → Service → DB): the router becomes a 5-line passthrough, all business logic (path constant, two exec calls, stdout capping, contract dict) lives in the service. Same shape as services/slot_service.py, services/task_execution_service.py, services/monitoring_service.py — testable in isolation, discoverable via grep, won't trip /validate-architecture later. Behavior unchanged. 13/13 scheduler unit tests still pass; live endpoint hit returns same contract dict. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7 tasks
4 tasks
vybe
pushed a commit
that referenced
this pull request
Apr 27, 2026
… (#501) Replaces raw `DELETE FROM agent_git_config` SQL in routers/git.py with the existing `db.delete_git_config()` method (already used at line 435 of the same file for the init-failure rollback path). Restores Architectural Invariant #1 (Three-Layer Backend) for this router. No behavior change — identical SQL, identical parameter binding. Closes #451 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dolho
added a commit
that referenced
this pull request
Apr 27, 2026
Move the pre-check exec logic out of routers/internal.py into a dedicated services/pre_check_service.py. Aligns with Invariant #1 (Router → Service → DB): the router becomes a 5-line passthrough, all business logic (path constant, two exec calls, stdout capping, contract dict) lives in the service. Same shape as services/slot_service.py, services/task_execution_service.py, services/monitoring_service.py — testable in isolation, discoverable via grep, won't trip /validate-architecture later. Behavior unchanged. 13/13 scheduler unit tests still pass; live endpoint hit returns same contract dict. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Apr 28, 2026
* feat(scheduler): agent-owned pre-check hook (#454) New optional contract: agents implement POST /api/pre-check in their container; scheduler calls it before firing a cron-triggered chat. Endpoint absent or any error → fire as usual (fail-open). fire=false records a skipped execution. fire=true with a message overrides the schedule.message for that invocation. - docker/base-image/agent_server/routers/pre_check.py: new router that dynamically loads /home/developer/.trinity/pre-check.py (template- supplied) and calls its check() function - agent-server main.py: mount pre_check_router - scheduler/agent_client.py: pre_check() method with fail-open semantics on 404/5xx/timeout/malformed-response - scheduler/service.py: _run_pre_check + pre-check branch in _execute_schedule_with_lock (cron only; manual triggers bypass) - tests/scheduler_tests/test_pre_check.py: 12 tests covering client- and service-level behavior; 161/161 scheduler suite passes Zero schema change — reuses existing ExecutionStatus.SKIPPED and create_skipped_execution. Closes the "wake agent on every cron tick" cost gap noted in docs/planning/PR_REVIEWER_AGENT.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(#454): scheduler pre-check feature flow + arch + requirements - feature-flows/scheduler-pre-check.md: new flow doc with contract, fail-open semantics, error table, testing summary - architecture.md: add /api/pre-check to agent-server endpoint list and pre-check note to Scheduler Service row - requirements.md: SCHED-COND-001 entry under §10 (Scheduling & Execution) - feature-flows.md: index row - docs/planning/PR_REVIEWER_AGENT.md: design doc from which this feature was extracted — committed for traceability Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: address PR #455 review feedback - pre_check.py: asyncio.get_event_loop() → get_running_loop() (deprecated in 3.10+) - pre_check.py: oversized message override no longer dropped silently — response now carries message_truncated="override dropped: N bytes exceeds 32000 cap" so scheduler/operator can see what happened; log escalated to ERROR with size+limit details - pre_check.py: module-level docstring expanded to note the security scope of check() (full Python interpreter access, same sandbox as chat tools — operators should review .trinity/pre-check.py like any executable template file) and the intentional no-cache behavior - tests/unit/test_pre_check_router.py: 15 new router/unit tests covering oversized-message drop path and non-dict return → 500 (both previously only exercised by inspection). Uses importlib to load pre_check.py directly, avoiding python-multipart requirement from sibling routers - feature-flows/scheduler-pre-check.md: document truncation behavior, security scope expectation, and updated test summary (12 scheduler + 15 router = 176 total passing) Lock-scope concern noted in review is not an issue: the skip path returns from _execute_schedule_with_lock, and the outer _execute_schedule holds the lock in a try/finally that covers the return. No leak. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(#454): docker exec instead of agent-server HTTP endpoint Review feedback on #455 flagged that the HTTP-endpoint design introduced a new system edge (scheduler → agent-server direct) and a novel code- loading pattern (importlib in a router). Both broke with Trinity's established convention that all "run something in an agent container" flows go through `services/docker_service.execute_command_in_container` — the same primitive used by: - services/git_service.py (persistent-state allowlist, #384 S3) - services/ssh_service.py (key provisioning) - services/agent_service/terminal.py (web SSH) - routers/system_agent.py (admin exec) - adapters/message_router.py (Slack file ingest) - routers/voice.py, monitoring_service.py This commit swaps the design accordingly. Changes: - Delete docker/base-image/agent_server/routers/pre_check.py and its router registration. No new HTTP surface on agent-server. - Delete tests/unit/test_pre_check_router.py (router is gone). - Add src/backend/routers/internal.py → POST /api/internal/agents/{name}/pre-check. Runs the template-shipped `.trinity/pre-check.py` via execute_command_in_container. Two-step: `test -f` for existence, then `python3 .../pre-check.py`. Returns {hook_present, exit_code, stdout, stderr}. Gated by existing X-Internal-Secret header (C-003). - Rewrite src/scheduler/service.py::_run_pre_check to call the backend endpoint (scheduler no longer opens a direct edge to agent-server). Translates backend response to the same {fire, message, reason} shape so _execute_schedule_with_lock is unchanged. - Delete AgentClient.pre_check from src/scheduler/agent_client.py. - Rewrite tests/scheduler_tests/test_pre_check.py to mock the backend HTTP (httpx.AsyncClient) instead of the agent HTTP client. 13 tests covering translation: hook absent → None, non-zero exit → None, empty stdout → skip, non-empty stdout → fire with override, 404, 5xx, connection error, malformed JSON. - Update docs/memory/architecture.md §Agent Containers and §Background Services, docs/memory/requirements.md SCHED-COND-001, and docs/memory/feature-flows/scheduler-pre-check.md to reflect the new topology. feature-flows.md index updated. Template side (dolho/pr-reviewer-agent commit 4330d2a): .trinity/ pre-check.py rewritten as a standalone shebanged script that prints the chat prompt to stdout or exits 0 empty. Benefits: - Preserves "scheduler → backend → agent" topology (Invariant #11). - Uses Trinity's dominant pattern for agent-container command execution. - No importlib, no module caching, no pydantic contract — exit code + stdout is unambiguous. - Smaller code footprint: internal endpoint is ~50 lines, scheduler translation is ~50 lines, template script is ~50 lines. Test coverage: 162/162 scheduler suite passing (up from 161 — new case for malformed backend JSON). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(#454): make pre-check hook language-agnostic Drop the `python3` invocation from the backend's pre-check exec and rename the convention path from `~/.trinity/pre-check.py` to `~/.trinity/pre-check`. Trinity now execs the file directly — the interpreter is selected by the file's shebang line. Templates can ship Python, bash, node, or a compiled binary; Trinity stops caring about language. `test -f` (not `-x`) is kept for the existence check so a present-but-non-executable file surfaces as an exec failure (exit 126) in the operator log instead of silently falling through to the backward-compat "no hook" path. Docs (architecture / feature-flow / requirements / index) updated to reflect the new contract. Scheduler-side translation tests are unaffected — they assert the JSON contract, not the exec command string. 13/13 tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(#454): extract pre_check_service from internal router Move the pre-check exec logic out of routers/internal.py into a dedicated services/pre_check_service.py. Aligns with Invariant #1 (Router → Service → DB): the router becomes a 5-line passthrough, all business logic (path constant, two exec calls, stdout capping, contract dict) lives in the service. Same shape as services/slot_service.py, services/task_execution_service.py, services/monitoring_service.py — testable in isolation, discoverable via grep, won't trip /validate-architecture later. Behavior unchanged. 13/13 scheduler unit tests still pass; live endpoint hit returns same contract dict. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 29, 2026
vybe
added a commit
that referenced
this pull request
Apr 30, 2026
* feat(webhooks): agent schedule webhook triggers (WEBHOOK-001, #291)
Add public webhook URLs so external systems (CI/CD, CRMs, monitoring) can
trigger agent schedule executions via a simple HTTP POST with no Trinity
account required — authenticated by a 256-bit opaque token embedded in the URL.
Changes:
- New public router POST /api/webhooks/{token}: rate-limited (10/60s per
token), audit-logged, 202 Accepted, delegates to existing scheduler trigger
- JWT-auth CRUD: POST/GET/DELETE /api/agents/{name}/schedules/{id}/webhook
- DB migration: webhook_token (TEXT UNIQUE), webhook_enabled (INTEGER DEFAULT 0)
on agent_schedules; partial unique index for O(1) token lookup
- Scheduler updated to accept triggered_by param in JSON body so executions
record triggered_by="webhook" correctly
- Webhook context field framed as data to reduce prompt injection surface
- 12 integration tests in tests/test_webhook_triggers.py
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(security): patch 4 Dependabot alerts — happy-dom + vite (#486)
Bumps two dev-only dependencies to patched versions. Production is
unaffected (happy-dom is test-only; vite only runs in local dev).
- src/frontend: vite ^6.0.6 → ^6.4.2 (closes Dependabot #55, CVE-2026-39363)
- tests/git-sync: happy-dom ^15.11.7 → ^20.9.0 (closes #83/#84/#85:
VM context escape RCE, ESM code exec, fetch cookie leakage)
Verified: frontend `vite build` clean, all 10 git-sync vitest tests pass
under happy-dom 20.9.0. No new critical/high alerts introduced.
Closes #485
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(webhooks): add WEBHOOK-001 to requirements and architecture (fixes #484 review)
Add missing documentation for the webhook trigger feature:
- requirements.md: WEBHOOK-001 entry with description, key features, DB changes,
API endpoints, security model, and feature flow link
- architecture.md: webhooks.py listed in Routers table; Schedules table expanded
from 9 to 12 endpoints with the 3 webhook management endpoints; new Webhook
Triggers section documenting the public POST /api/webhooks/{token} endpoint;
webhook_token and webhook_enabled columns added to agent_schedules schema block
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(#488): add status-in-dev label + PR-merge automation (#489)
Close the gap between "PR merged to dev" and "released to main".
- New GH Action `issue-status-on-merge.yml`: on PR merge to dev,
parse Fixes/Closes/Resolves #N from PR body+title, add
`status-in-dev`, remove `status-in-progress`.
- `/release` skill: read `gh issue list --label status-in-dev` as
the authoritative shipping list for release notes; include
`Closes #N` in the release PR body so issues auto-close on merge
to main.
- `DEVELOPMENT_WORKFLOW.md`: SDLC is now Todo → In Progress →
In Dev → Done, each stage mapped 1:1 to commit-graph location.
Fixes #488
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(channels): file upload Phase 2 — workspace delivery hardening (#487) (#494)
Phase 2 of #354 polishes the shared channel-agnostic file delivery path
in `message_router._handle_file_uploads`. Phase 1 (#355) added Telegram
extraction/download/validation; the actual workspace write path was
introduced for Slack inbound (#222). This change hardens the shared path
for both channels:
- New `_sanitize_filename` helper: NFKC unicode normalize → basename →
safe-chars regex → empty/dotfile fallback to `file_{id}` → 200-char
truncation preserving extension → collision dedup with `-1`, `-2`, …
- Spec injection format: `[File uploaded by {uploader}]: {name} ({size})
saved to {path}`. Uploader is the verified email when present
(Issue #311), else `adapter.get_source_identifier(message)`.
- All-writes-failed handling: when every workspace write attempt fails,
the router replies on the channel with an explicit error and skips
agent execution (#487 AC6). Validation rejections (size/MIME/download
errors) still surface in the description block as before.
- Audit log entries gain an `uploader` field.
Per-session upload directory (`/home/developer/uploads/{session_id}/`)
preserved — keeps user uploads isolated and ephemeral, matches the
existing #222 model.
Tests: +17 unit tests across `TestFilenameSanitization` (12),
`TestFileDeliveryFormat` (2), `TestFileDeliveryFailures` (3). 28/28
passing in `tests/unit/test_file_upload.py`.
Docs: `telegram-integration.md` Phase 2 section + revision row;
`slack-file-sharing.md` flow / router / errors / security sections
updated for the shared change; `feature-flows.md` index row.
Closes #487
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(webhooks): import Request in schedules router (#495)
The WEBHOOK-001 commits (c630931 / 8fdf736) added `request: Request`
parameters to `generate_webhook` and `get_webhook_status` without
importing `Request` from fastapi. Backend module import fails with
NameError on startup, blocking all dev deploys.
Integration tests in tests/test_webhook_triggers.py exercise these
endpoints but never caught the bug because the backend never starts —
test setup fails before any test runs.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(backlog): repair drain spawn — lazy-import target after #95 (#496) (#500)
services/backlog_service.py:240 lazy-imported _execute_task_background
from routers.chat, but #95 deleted that function. Every backlog drain
attempt failed with ImportError; the exception was swallowed at
backlog_service.py:218-228, so BACKLOG-001 (#260) was silently dead.
Live observation: 23 drain failures / 24h on a fan-out workload, only
surface signal was the per-execution `error` column.
Why it shipped silently: the unit happy-path test patched
sys.modules["routers.chat"] with a SimpleNamespace stub of whatever
attribute name it expected, masking the production breakage.
Changes:
- Lazy-import _run_async_task_with_persistence (the post-#95
replacement) and adjust the call shape (drop release_slot, drop
orphaned task_activity_id; the unified executor handles both).
- Capture self-task fields (is_self_task, self_task_activity_id,
inject_result) at enqueue time and rehydrate on drain so
SELF-EXEC-001 (#264) survives backlog overflow.
- Emit a stable log token `backlog_drain_spawn_failed` so log-based
detection (Vector / dashboards) can catch import drift or similar
spawn-time regressions at fleet scale rather than per-row.
- AST-based regression guard in tests/unit/test_backlog.py:
TestLazyImportTarget parses routers/chat.py and asserts the import
target exists; paired test asserts the lazy-import string matches
the validated allow-list. Catches both directions of drift without
booting the backend.
- Update happy-path test to use the new symbol and kwarg surface;
add self-task enqueue+drain round-trip tests.
Closes #496
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(announce): add Twitter/X support via API v2 + OAuth 1.0a
Bumps announce skill to v1.6. Adds a Python helper (scripts/post_twitter.py)
that reads tweet text from stdin and posts via Twitter API v2 using OAuth 1.0a
User Context — same exit-0/1 + structured-JSON contract as the existing
Discord/Slack/Telegram send paths so the sequential-only and no-blind-retry
rules apply uniformly. Credentials live in .env (gitignored) under
ANNOUNCE_TWITTER_* keys.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(chat): sync /task long-polls on backlog at capacity (#498) (#515)
Sync parallel `/task` calls (parallel=true, async=false) at capacity used
to fail terminally with HTTP 429 — they never touched the BACKLOG-001
backlog because the spill block was nested under `if request.async_mode:`.
Observed in production: ~40% terminal-failure rate from one MCP fan-out
caller (214 capacity rejections / 24h, 0 enqueues from 541 dispatches).
Sync calls now spill to the same backlog the async path uses and long-poll
on the open HTTP connection until the queued execution reaches a terminal
status, then return the result inline. True 429 only when the backlog is
also full. Total connection hold capped at 2 × effective_timeout.
Implementation:
- New `services/sync_waiter.py` owns the in-process registry and the
`signal_sync_waiter` / `wait_for_sync_terminal` primitives. Wait combines
an asyncio.Future (set by the drain finally block) with a 5s DB-poll
fallback that covers terminal flips routed outside the drain
(corrupt-metadata, expire_stale, cleanup recovery).
- `routers/chat.py` sync branch now mirrors the async branch:
pre-acquires the slot, on at-capacity calls `backlog.enqueue()` then
`wait_for_sync_terminal()`, returns the inline result on wake.
- `_run_async_task_with_persistence` wraps its body in try/finally and
signals any registered sync waiter with the rich TaskExecutionResult
plus chat_session_id. No-op when no waiter is registered (the common
async fire-and-forget path).
Tests (`tests/unit/test_chat_sync_backlog.py`, 13 new):
- Signal / wait / poll-fallback / timeout / cleanup / concurrent waiters
- Regression test pins TERMINAL_TASK_STATUSES to the enum so a new
TaskExecutionStatus value forces a deliberate update (caught a missing
SKIPPED entry pre-merge)
Trade-off (Policy B): worst-case connection hold doubles to
2 × effective_timeout when the request is queued. Honest envelope —
the caller chose to wait. Documented in the architecture diagram of
`persistent-task-backlog.md`.
Companion issue #505 covers the orchestration-education gap (MCP tool
description + platform prompt) so agents pick the right tool for the
job rather than relying on the platform absorbing every misuse.
Closes #498
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(groom): document SDLC stages and add status-label/board reconciliation
Adds SDLC context (Todo → In Progress → In Dev → Done) so grooming respects
in-flight work, and a Step 1b that reconciles status-* labels with board
columns (labels are authoritative).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(agent): classify signal-killed claude exits as 504, not fake auth failure (#517)
External signal terminations of the claude subprocess (timeout SIGKILL,
OOM-kill, parent SIGTERM, operator cancel) used to fall through to the
auth-fallback heuristics and surface as a misleading "Subscription token
may be expired" 503. Same shape as #361 (max-turns), different exit path.
Adds _classify_signal_exit() consulted before the auth heuristics: matches
Python-native signal exits (return_code < 0) and shell-encoded forms
(130/137/143 for SIGINT/SIGKILL/SIGTERM) and raises HTTP 504 with a clear
"killed by SIGKILL/SIGTERM/SIGINT — likely timeout, OOM, or operator
cancel" message. Tightens the zero-token heuristic with return_code > 0
so signal exits cannot reach it.
The bug became routinely reproducible after #61 (PR #326) added
backend-driven terminate_execution_on_agent() — every timeout now
produces a signal-killed claude subprocess on the agent side, which the
old heuristic block misclassified. Also de-risks PR #508 (auth-class
auto-switch): without this fix, every timeout would trigger an
unnecessary subscription rotation.
Backend's task_execution_service.py only flags AUTH on 503; 504 falls
through to the generic FAILED path. No backend changes required.
Closes #516
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(sprint): align skill with DEVELOPMENT_WORKFLOW.md (#519)
Four divergences between the /sprint playbook and the SDLC documented in
docs/DEVELOPMENT_WORKFLOW.md:
- Step 3 used `gh issue edit --add-label status-in-progress` directly,
bypassing .github/workflows/claim.yml and skipping self-assignment.
Now posts `/claim` as an issue comment, which is the workflow's single
source of truth for the In Progress transition.
- Step 8 invoked pytest directly via `cd tests && source .venv/bin/activate
&& python -m pytest …`. Now defers to `/test-runner [feature]`, with a
documented fallback for brand-new files outside the runner's catalog.
- Step 10 commit + PR body used `closes #N`. Workflow §1 specifies
`Fixes #N`; both auto-close on GitHub but the doc is the contract.
- Step 11 final report didn't mention the post-merge automation. Now
warns that issue-status-on-merge.yml owns the
status-in-progress → status-in-dev transition, so operators don't
manually edit labels post-merge.
Non-breaking: argument signature, automation level (gated), state
dependencies, and pipeline overview unchanged. Net +19/-11.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(agent): classify clean-exit empty-result as 502, not silent success (#520) (#521)
* fix(agent): classify clean-exit empty-result as 502, not silent success (#520)
Sibling of #516/#517 on the return_code == 0 path. When the claude
subprocess exits 0 but the final {"type":"result"} JSON line is dropped
before the reader thread captures it (typical cause: a child subprocess
inherited stdout, kept the pipe open past claude exit, the reader thread
leaked, the pgroup unwind closed the pipe), metadata.cost_usd and
metadata.duration_ms stay None. The success path used to return HTTP 200
anyway — agent-server logged "completed successfully" while backend
silently reaped the execution as an orphan minutes later, masking the
real failure with a misleading "completed on agent but recovered by
watchdog" message.
Adds _classify_empty_result(metadata, raw_message_count) consulted after
the return_code != 0 block (#516 + auth heuristics) and before response
building. When both cost_usd and duration_ms are None, raises HTTP 502
with diagnostic context (tools, turns, raw_messages, cause hint).
Backend's task_execution_service.py:542 only flags AUTH on 503, so 502
falls through to the generic FAILED path with the helpful detail
preserved — no backend changes needed.
The two-field check is conservative: single-field nullability could be a
Claude format quirk; both-None is a strong signal that the terminal
result message never arrived. Test coverage pins the scope so a future
edit can't silently broaden it.
Changes:
- docker/base-image/agent_server/services/claude_code.py — new
_classify_empty_result() helper next to _classify_signal_exit; call
site between the return_code != 0 block and response building.
- tests/unit/test_empty_result_classification.py — 9 new tests, all
pass. Covers both-None → 502, populated metadata → None,
single-field-only → None (Claude format quirk tolerance), zero-cost
and zero-duration → None (is None vs falsy), missing metadata → None.
- docs/memory/feature-flows/parallel-headless-execution.md — changelog
entry under Recent Updates.
- docs/memory/feature-flows/task-execution-service.md — row in
error-translation table + new Empty-Result Pre-Check paragraph.
Requires base-image rebuild after merge:
./scripts/deploy/build-base-image.sh
Closes #520
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(feature-flows): index entry for agent error classification (#516, #520)
Combined Recent Updates entry covering the matching pair of agent-side
error-classification fixes that shipped this week — _classify_signal_exit
(#516, PR #517) and _classify_empty_result (#520, PR #521). Both touch
docker/base-image/agent_server/services/claude_code.py and share the
"agent surfaces the right HTTP status so backend records FAILED with a
useful detail" theme.
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(agent): off-load synchronous terminate cleanup off the event loop (#523)
The async terminate_execution endpoint and the outer asyncio.TimeoutError
handlers in execute_claude_code and execute_headless_task were calling
registry.terminate() / _terminate_process_group() / _safe_close_pipes()
synchronously. Those helpers do up to 7s of process.wait() (SIGINT grace
+ SIGKILL grace), which blocks the asyncio event loop for the entire
window. While blocked, agent-server cannot serve /health, the backend
circuit breaker opens, and UI fan-out hangs for 5+ minutes per page.
This is the actual user-visible mechanism behind the #523 "agent-server
wedge" symptom, not the FD-inheritance / leaked-reader-thread theory in
the original report (see issue comment for the corrected diagnosis —
the FD_CLOEXEC fix as written would not have helped because dup2 strips
CLOEXEC during the child's stdout setup, and the existing post-#407
killpg + safe_close path actually works in the vast majority of cases).
Wrap the three call sites in loop.run_in_executor(None, ...) so the
blocking process.wait() runs on a thread-pool worker. Pipe-inheritance
fragility remains a slow-burn cleanup item to be filed separately.
- routers/chat.py: terminate_execution dispatches registry.terminate to
the default executor
- services/claude_code.py: outer-timeout cleanup in both async paths
off-loads _terminate_process_group + _safe_close_pipes
- tests/unit/test_terminate_async_executor.py: regression test asserts
registry.terminate runs on a non-event-loop thread and the event-loop
yield stays sub-50ms while terminate is in flight
Fixes #523
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(planning): add Tier 2.6 hardening + actor-model destination roadmap
Records the architectural critique from 2026-04-26 review:
- Tier 2.6 (Sprint D′): #524 state machine contract, #525 idempotency
keys, #526 dispatch circuit breaker. Closes the three contract-level
gaps that survive even after Sprint D's plumbing consolidation.
- Future considerations: 7 unranked recommendations (durable
ProcessRegistry, retry-in-funnel, synchronous terminate ack, dual
streams, fairness, EventBus backpressure, lifecycle contract doc).
- Target architecture section: names the actor model as the destination
(mailbox + journal + processor), maps existing components to the
concepts they already implement, defines a 4-phase gated transition
roadmap, and gates Phase 2 (agent-to-agent experiment) on a one-page
message-envelope + journal-format postcard.
Issues: #524, #525, #526 created and added to project board (Epic
#411 Orchestration Invariants, Theme Reliability).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(planning): mark #291 (WEBHOOK-001) shipped, Sprint C now 5/5
#291 closed 2026-04-24, shipped via PR #484 (token-in-URL trigger
through TaskExecutionService) with follow-up fix PR #493. The plan
doc still listed it as the next item to pick up; align it with the
ground truth and re-aim "what to do next" at #428 (after #306 soak)
plus Tier 2.6 hardening (#524/#525/#526) in parallel.
* refactor(capacity): consolidate three queue/slot primitives into CapacityManager (#428) (#527)
* refactor(capacity): consolidate ExecutionQueue + SlotService + BacklogService into CapacityManager (#428)
Single public facade for agent execution capacity. Composes SlotService
(Redis ZSET counter) and BacklogService (SQL persistent overflow) as
private internals; owns the in-memory overflow store (Redis LIST, depth 3,
lifted from the deleted ExecutionQueue).
Why:
- 7 caller sites now go through one API instead of orchestrating three.
- Each new trigger type (retry, webhook, self-exec, fan-out) gets one path
for capacity, not a choice between three primitives.
- Unblocks #429 (CLEANUP-COLLAPSE) and the actor-model destination by
reducing the surface a single capacity store has to expose.
API:
capacity.acquire(agent, exec_id, max_concurrent, *,
overflow_policy='reject'|'queue_in_memory'|'queue_persistent',
overflow_payload=PersistentTaskPayload(...))
capacity.release(agent, exec_id) # idempotent
capacity.release_if_matches(agent, eid) # TOCTOU-safe (watchdog)
capacity.get_status(agent, max_concurrent)
capacity.reclaim_stale(agent_timeouts) # called by cleanup_service
capacity.force_release(agent) # emergency
capacity.cancel_all_overflow(agent, reason) # agent deletion
capacity.run_maintenance(max_age_hours) # 60s tick from main.py
Wire format unchanged: same Redis keys (agent:slots:*, agent:queue:*),
same SQL columns (schedule_executions.queued_at, backlog_metadata).
In-flight executions unaffected; clean revert path.
Deviations from issue spec (user-approved):
- No feature flag — single runtime path. dev-soak + clean revert is the
rollback mechanism, simpler than a per-agent DB column + flag check at
every call site.
- ExecutionQueue deleted in this PR rather than separate cleanup PR.
SlotService and BacklogService kept as private internals (well-factored,
one job each).
Soak deviation: shipped after 5 days of #306 soak rather than the planned
14 days. Mitigated by additive-style refactor (no wire-format change).
Files:
- NEW services/capacity_manager.py (~480 LOC)
- DELETE services/execution_queue.py (~360 LOC)
- 7 caller migrations: routers/chat.py (4 sites), routers/agents.py (2),
routers/agent_config.py (1), services/cleanup_service.py (4),
services/task_execution_service.py (1), services/agent_service/queue.py (3),
main.py (1, callback wiring is now internal).
- NEW tests/unit/test_capacity_manager.py — 21 tests covering acquire/release
for all three overflow policies, drain wiring, status, force_release,
reclaim_stale, cancel_all_overflow.
- UPDATE tests/test_watchdog_unit.py — 11 mock decorator pairs collapsed to
single get_capacity_manager mock.
Tests: 21 new + 35 watchdog + 33 backlog = 89 green for affected surface.
Fixes #428
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(feature-flows): add capacity-management.md, deprecate predecessor flows (#428)
- NEW capacity-management.md — public surface, overflow policies, end-to-end
/chat and /task flows, storage map, maintenance & recovery, what-replaced-what.
- DEPRECATE notes on the three predecessor flows with redirects:
- execution-queue.md (ExecutionQueue deleted)
- parallel-capacity.md (SlotService internalized)
- persistent-task-backlog.md (BacklogService internalized)
- "Now uses CapacityManager" notes on four downstream flows:
- task-execution-service.md, parallel-headless-execution.md,
cleanup-service.md, execution-termination.md
- Index: Recent Updates row + Core Agent Features row for capacity-management.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(requirements): note BACKLOG-001 is now internal to CapacityManager (#428)
Section 10.8 (Persistent Task Backlog) — replace direct SlotService callback
reference with the unified CapacityManager facade. Status bumped with the
2026-04-26 internalization date and #428 cross-ref.
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(agent): drain pipe before close to preserve final result line (#531) (#532)
* fix(agent): drain pipe before close to preserve final result line (#531)
drain_reader_threads previously called safe_close_pipes() immediately
after terminate_process_group(), discarding the kernel pipe buffer before
the reader thread could drain it. On long agentic tasks the final
{"type":"result"} JSON line (cost, duration, answer) was in that buffer
at the moment of close, causing the reader to raise ValueError and
metadata.cost_usd / duration_ms to remain None — triggering the HTTP 502
"Execution completed without a result message" classification from #521.
Fix: reorder so grandchildren are killed first, then the reader is given
post_kill_grace=30s to drain naturally (grandchildren dead → kernel
delivers EOF once the buffer is consumed → reader returns '' and exits
cleanly). safe_close_pipes() is now a true last resort — only called when
the reader is still alive after 30s, which indicates a genuine wedge, not
unfinished backlog drain.
Also extends _classify_empty_result to derive num_turns from raw_messages
when metadata.num_turns is None (result line lost), so the 502 detail
reports an honest turn count instead of always showing 0.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs(tests): update test catalog for #531 drain_reader_threads fix
- Add test_subprocess_pgroup.py and test_empty_result_classification.py
to Test Categories (Operations & Observability, unit section)
- Add 2026-04-27 Recent Test Additions entry with description of the
pipe-drain ordering regression tests and raw_messages fallback tests
- Update unit test count: 165 → 170; total: 2,257 → 2,262
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs(feature-flows): document drain_reader_threads pipe-ordering fix (#531)
Update parallel-headless-execution.md with the root cause fix for
the "Execution completed without a result message" HTTP 502: the old
drain_reader_threads sequence closed the pipe before the reader could
drain the kernel buffer (including the final result JSON line). New
sequence: kill grandchildren → natural drain (post_kill_grace=30s) →
force-close only as last resort.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(tests): update orphaned-recovery mocks to CapacityManager (#533) (#534)
Replace stale services.slot_service sys-mock with services.capacity_manager
so all four recovery scenario tests pass after the #428 consolidation.
Assertions updated from release_slot → release to match the new API.
Fixes #533
Co-authored-by: Claude <noreply@anthropic.com>
* docs(skills): align validate-pr with DEVELOPMENT_WORKFLOW.md
Add quick triage block, base branch check, PR size warning, type-docs
label, Base Branch/PR Size rows in report table, and review pipeline
matrix linking /review and /cso --diff with their complementary roles.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(validate-architecture): stale-citation filter + dedupe guard (#511) (#513)
* fix(validate-architecture): add stale-citation filter + issue dedupe guard (#511)
The /validate-architecture skill produced false-positive issue #479 by:
1. citing file paths the report's snapshot saw, but `main` no longer has
(process engine deleted in #430 the same day);
2. running `gh issue create` with no check for existing open issues with
the same finding fingerprint.
Two targeted edits to .claude/skills/validate-architecture/SKILL.md:
- New Step 2c "Filter Stale Citations" — `git ls-files --error-unmatch`
every cited path before report. Drop ghosts. Downgrade FAIL → PASS
when an invariant has zero remaining real citations.
- Modified Step 4 — fingerprint = sorted invariant numbers; query
open `automated,priority-p1` issues with `--search "in:body
validate-architecture fingerprint=<fp>"`; comment on existing
issue instead of creating duplicate. Issue body now stamps the
current commit SHA for evidence binding.
Closes #511.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(validate-architecture): clarify dedupe branching, distinct fingerprint marker, quote paths (#511)
Follow-up to review feedback on PR #513. Three small skill-prose
hardenings:
- I2 (LLM-driven flow control): the dedupe branch previously relied on
`if [ -n "$EXISTING" ]; then ...; exit 0; fi` followed by a separate
create block. `exit 0` halts a bash subshell, not an LLM walking the
markdown — a future runner could execute both blocks. Replace with
explicit "Path A — COMMENT, then STOP" / "Path B — CREATE" prose
branching and an explicit DO-NOT note.
- I4 (fingerprint collision): replace free-text body search
`validate-architecture fingerprint=$FP` with HTML-comment marker
`<!-- validate-architecture::fingerprint=$FP -->` plus a quoted-phrase
search. Self-evidently programmatic; won't collide with prose.
- I3 (path quoting): the Step 2c example now uses `"$path"` and a note
about shell metachars, so implementers don't strip the quotes.
- Add concurrency caveat documenting that the dedupe is best-effort,
not atomic (no GitHub primitive provides this).
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(config): clean stale Auth0 / AUDIT_URL, document SMTP/SendGrid/FRONTEND_URL (#481) (#509)
- Remove dead Auth0 env vars + build args from docker-compose.prod.yml
and docker/frontend/Dockerfile.prod (Auth0 removed 2026-01-01). The
build-arg fallbacks were also leaking a real Auth0 domain + client ID
into a public repo.
- Drop AUDIT_URL from .env.example (audit-logger service no longer exists;
no Python references it).
- Add FRONTEND_URL to .env.example (required in prod for OAuth post-auth
redirects in slack_service.py / public_links.py and SSH host
auto-detection in ssh_service.py).
- Document SMTP_HOST/PORT/USER/PASSWORD and SENDGRID_API_KEY in
.env.example so the advertised EMAIL_PROVIDER=smtp/sendgrid modes are
actually configurable from the template.
Closes #481
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(auth): require auth on /api/docs endpoints (#452) (#507)
Add Depends(get_current_user) to the three handlers in
src/backend/routers/docs.py so the file no longer violates
Architectural Invariant #8.
Note: this router is not currently registered in main.py, so
the endpoints are not reachable on the running API. The fix is
applied to the file as written so the invariant validator stops
flagging it and so the file is correct if it is ever remounted.
Closes #452
* fix(chat): wrap long unbroken strings in chat bubbles (#457) (#502)
Long URLs, tokens, base64 blobs, and other unbroken strings in agent
chat responses were overflowing their 85% bubble and forcing horizontal
scroll on the entire Chat tab.
Root cause: ChatBubble.vue capped the bubble width but never told
inner content how to handle unbreakable strings. Inline <code> and
the user-text <p> had no overflow-wrap; <pre> defaulted to white-space:
pre with no overflow-x: auto override.
Fix (CSS-only, all 3 render branches — user / self-task / assistant):
- min-w-0 on outer wrapper, overflow-hidden on inner bubble
- break-words on user text and prose container
- prose-pre:overflow-x-auto + prose-pre:max-w-full so code blocks
scroll inside the bubble instead of expanding it
- prose-code:break-words for long inline tokens
- prose-a:break-words for long URLs in markdown links
Verified visually: before/after static test page shows BEFORE leaks
content well past the bubble border; AFTER wraps cleanly with no
regression on normal markdown (headings, lists, links, short code).
* fix(schedules): add missing Request import for webhook endpoints (#493)
Regression from c630931 (WEBHOOK-001 / #291): `routers/schedules.py`
uses `Request` as a type annotation on the `generate_webhook` and
`trigger_webhook` handlers but never imports it, so the module fails
to load and the backend won't start with a NameError.
Minimal fix: add `Request` to the existing `from fastapi import …`
line (line 11). No behavioral change — the annotation was already
intended.
Surfaced while dev-testing FILES-001 (PR #491). uvicorn reload pulled
in the dev branch state and blew up.
Co-authored-by: Pavlo <pash@pashs-MacBook-Pro.local>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(git): route orphan cleanup through db.delete_git_config (#451) (#501)
Replaces raw `DELETE FROM agent_git_config` SQL in routers/git.py with the
existing `db.delete_git_config()` method (already used at line 435 of the same
file for the init-failure rollback path). Restores Architectural Invariant #1
(Three-Layer Backend) for this router. No behavior change — identical SQL,
identical parameter binding.
Closes #451
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(subscription): auto-switch on first failure + auth-class triggers (#441) (#508)
Drop the 2-consecutive-429 gate in `subscription_auto_switch` so a single
subscription failure now triggers a switch — the 2h skip-list on
alternative selection (already pinned by #444 / #476 regression tests) is
sufficient as the lone thrash guard. Broaden the trigger surface to also
fire on auth-class failures (401/403/credit balance/expired OAuth token,
etc.), classified via a centralized `AUTH_INDICATORS` list, so a broken
subscription auto-recovers instead of failing every execution until
manual intervention. Flip the `auto_switch_subscriptions` default to
"true" — operators can still opt out, but the safe behavior is now the
default. Backward-compat shim `handle_rate_limit_error` preserved for
existing 429 callers.
- services/subscription_auto_switch.py: new `handle_subscription_failure`
with `failure_kind` dispatch ("rate_limit" | "auth"); new
`is_auth_failure` classifier; default flipped; notification + log
wording adapts per kind; old shim retained.
- services/task_execution_service.py: 503 / auth-classified errors now
also call the switch path alongside 429.
- routers/chat.py (sync): same broadening on the interactive chat
surface; auth path returns 503+retry hint mirroring the 429 UX.
- routers/subscriptions.py: GET `/auto-switch` default also flipped to
"true" so the UI toggle and runtime gate read the same value.
- scheduler/service.py: dedupe two inline `auth_indicators` copies into
a single module-level constant; cross-reference the canonical list in
backend (cross-container import not viable).
- tests/unit/test_subscription_auto_switch_pingpong.py: new
TestIsAuthFailure + TestSingleEventThreshold classes (8 new tests, all
pingpong + #476 aging tests still green).
- tests/test_subscription_auto_switch.py: flip default-off → default-on.
- docs: SUB-003 feature flow + requirements doc reflect the new
threshold, broadened scope, and on-by-default behavior.
Closes #441
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(feature-flows): clean up #95 drift missed by #500 (#496) (#503)
* fix(backlog): repair drain spawn after #95 rename (#496)
`services/backlog_service.py:_spawn_drain` was lazy-importing
`_execute_task_background` from `routers.chat`, but #95 (PR #316) deleted
that function and replaced it with `_run_async_task_with_persistence`.
Every backlog drain raised `ImportError`, was caught at line 218-228, and
silently marked queued executions FAILED — leaving BACKLOG-001 (#260)
non-functional whenever an agent hit capacity.
Rewire the lazy import to the new helper and adjust the call shape:
- drop `task_activity_id` (not in new signature; chat router already
passes None at enqueue)
- drop `release_slot=True` (the wrapper passes `slot_already_held=True`
to TaskExecutionService, which manages release in its finally block)
- derive `is_self_task` from x_source_agent vs agent_name
- pass `self_task_activity_id=None` (queued items don't carry one;
separate gap, not in scope here)
Add `tests/test_backlog_drain_unit.py` with five regression checks:
two AST-based contract tests that pin the function name and signature
in `routers/chat.py` (would have caught the original break), and three
runtime spy tests covering the kwarg shape `_spawn_drain` forwards. The
existing `tests/unit/test_backlog.py::test_drain_happy_path_spawns_background`
is updated to match the new contract.
Sync the BACKLOG-001 and TaskExecutionService feature-flow docs to
reference the renamed helper.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(feature-flows): sync index + parallel-capacity for #496
- Add #496 entry to feature-flows.md Recent Updates.
- Fix two more stale `release_slot=True` references in
parallel-capacity.md left over from #95 — the param never existed
on `_run_async_task_with_persistence` (slot release happens inside
TaskExecutionService via slot_already_held=True).
Other stale `release_slot=True` references in
authenticated-chat-tab.md and parallel-headless-execution.md are
deeper drift (separate flows, not touched by #496) — leave for a
follow-up doc-cleanup pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(catalog): register test_backlog_drain_unit.py (#496)
Adds the new BACKLOG-001 regression test file to tests/registry.json
so it shows up in the catalog alongside test_event_bus.py and
unit/test_backlog.py.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: drop redundant test_backlog_drain_unit.py
PR #500 (which superseded the original #496 fix scope) shipped
equivalent contract coverage with a more robust setup:
- `TestLazyImportTarget` (AST guard for the lazy-import target)
- `test_drain_threads_self_task_fields` (round-trip via real
BacklogService against sqlite)
The local file used sys.modules stubs which were strictly weaker.
Keeping it would only add maintenance burden for duplicate coverage,
so drop the file and its registry entry. Net effect on PR #503 is
that it becomes a small, focused docs-cleanup PR (parallel-capacity.md
and task-execution-service.md drift from #95, plus the missing
Recent Updates entry for #496).
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(deploy): align scripts, configs, and docs with production operating patterns (#504)
* docs(generate-user-docs): add hub-and-spokes deployment structure and ops-pattern import
Restructure skill to produce guides/deploying/ as a hub plus six spokes
(local-development, single-server, public-access, upgrading,
backup-and-restore, monitoring) instead of one flat deploy guide.
Add an "operational guide" template (When to Run → Pre-flight →
Procedure → Verify → Rollback) for procedural docs that don't fit the
feature-shaped dual-audience template, plus verbatim-reuse snippets for
the load-bearing rules: never down/up, rebuild platform services only,
six-probe verification, resource-thresholds table, alpine cp backup.
Add Step 2h to draw operational patterns from the private ops runbook
under ../trinity-ops/, with explicit safe/forbidden import lists and a
sshpass→localhost rewrite rule.
Strengthen Step 2e to cross-check .env.example keys against each
compose's environment block — docs must not promise behavior the
chosen compose can't deliver.
Add public-safety greps in Step 7 (sshpass, trinity-ops, tailnet, real
IPs, instance-dir refs) so leaked private detail blocks completion.
Tracks issue #504.
* fix(deploy): align scripts, configs, and docs with production operating patterns (#504)
Fixes the first-run blocker (agent creation fails silently without base
image) and removes references to the removed audit-logger service that
caused verify-platform.sh and validate.sh to always fail.
Scripts:
- start.sh: detect missing base image and auto-build on first run; use
`docker compose stop` in help text (not `down`, which destroys agents)
- verify-platform.sh: full rewrite — remove trinity-audit-logger and port
8001 audit checks; fix frontend from port 3000 → 80; check scheduler
health at :8001; add MCP/Vector probes; fix login hint
- validate.sh: remove non-existent `deployment/` dir, `QUICK_START.md`,
and `src/audit-logger/audit_logger.py` from required paths; fix port 3000
Config:
- docker-compose.yml: wire 5 missing env vars into backend (PUBLIC_CHAT_URL,
FRONTEND_URL, EXTRA_CORS_ORIGINS, SLACK_SIGNING_SECRET, SSH_HOST)
- .env.example: remove stale AUDIT_URL; annotate prod-only / overlay-only
vars (SLACK_SIGNING_SECRET, PUBLIC_CHAT_URL, FRONTEND_URL, SSH_HOST,
TRINITY_GIT_BASE_URL) so users know scope before setting them
Docs:
- deploying-trinity.md: add explicit build-base-image.sh step; fix
/trinity:connect to use MCP API key flow (not username/password); add
Upgrading, Health Verification, Resource Thresholds, and Common Recovery
Patterns sections from ops runbook; use `docker compose` (v2 syntax)
- setup.md: remove false claim that start.sh builds the base image; correct
admin account creation (env var driven, not wizard); clarify wizard path
(used only when ADMIN_PASSWORD is unset)
New file:
- quickstart.sh: interactive one-command setup (checks Docker, generates
secrets, sets ADMIN_PASSWORD, builds base image, starts services, verifies)
Skill:
- generate-user-docs: add deployment config reading rules (read scripts
literally; cross-check env vars vs compose; never claim "auto" unless code
proves it); add operational guide template (pre-flight/steps/verify/rollback);
resolve conflict preserving the hub+spokes guide structure from branch
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(generate-user-docs): remove private repo name from SKILL.md
Replace explicit `trinity-ops` repo references with generic path aliases
(`../ops-runbook/`) so the private repo name is not embedded in this
public repository.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(reliability): CAS guards on execution status writes + state machine doc (#524) (#541)
Closes the FAILED→SUCCESS and SUCCESS→FAILED races that were patched by
#378 re-verify logic without eliminating the root cause.
Changes:
- update_execution_status: SUCCESS writes are unconditional (agent wins);
non-success terminal writes blocked when row already terminal
- mark_stale_executions_failed / mark_no_session_executions_failed: inner
UPDATE gains AND status='running' to close the SELECT→UPDATE TOCTOU window
- _recover_execution: routes through mark_execution_failed_by_watchdog
(already CAS-guarded) instead of bare update_execution_status
- TaskExecutionStatus: state machine, transitions, and authorized writers
documented in docstring; PENDING_RETRY added to enum
- Remove now-dead _STALE_SLOT_ERROR_PATTERN constant
Full projector architecture (ExecutionStateProjector, agent event emission,
projected_status shadow column) deferred — agents have no Redis access and
the restart-recovery design needs more thought before those land.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(public-chat): build context before storing user message to prevent duplication (#539) (#540)
* fix(public-chat): build context before storing user message to prevent duplication (#539)
In the public chat endpoint, the user message was persisted to the database
before build_public_chat_context read from it, causing the current message
to appear twice in every agent prompt — once in "Previous conversation:"
and once in "Current message:". Reordering the calls so context is built
first (from prior history only) then the user message is stored eliminates
the duplicate on every turn.
Adds unit tests that document both the old broken order (two occurrences)
and the corrected order (one occurrence), guarding against regression.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs(feature-flows): update public-agent-links with #539 context ordering fix
- Correct PUB-005 data flow: build_public_chat_context before add_public_chat_message
- Update backend implementation step ordering to match fixed code
- Add revision history entry for the bug fix
- Add #539 entry to feature-flows.md index
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(agent-runtime): guard content_block isinstance in process_stream_line (#542) (#543)
Prevents AttributeError crash when Claude Code stream-json emits a
string element inside a message content array. Guards both
process_stream_line (real-time path) and parse_stream_json_output
(batch path) using the same isinstance(block, dict) pattern already
used by the error_content loop.
Fixes #542
Co-authored-by: Claude <noreply@anthropic.com>
* docs(#411): Phase 1 canary harness design + catalog Phase 1 subset additions (#544)
* docs(#411): Phase 1 canary harness design + catalog Phase 1 subset additions
- New design doc at docs/planning/CANARY_HARNESS_PHASE_1.md scoping the
AC-required infrastructure (snapshot collector, canary_violations table,
canary agent template, fleet, alerts) for the three required invariants
(S-01, E-02, L-03).
- Catalog Phase 1 subset expanded 10 → 12: adds S-03 (slot TTL ≥ exec
timeout, catches #226) and E-05 (dispatched rows have session, catches
#106), since both bugs are cited in the catalog motivation but had no
Phase 1 detector.
* docs(#411): scope fleet to strict minimum for AC's 3 invariants
* docs(#411): expand design doc to cover full Phase 1 (12 invariants, snapshot format)
* feat(settings): add Remove buttons for stored API keys + Slack (#459) (#483)
Settings page lets admins save/test Anthropic API Key, GitHub PAT, and
Slack OAuth credentials, but exposed no UI to clear them once stored.
Only workaround was calling DELETE endpoints directly or editing the DB.
Adds Remove buttons next to Save in each row, conditionally rendered
when the value lives in settings DB (source === 'settings'). Env-var
fallbacks stay uneditable from UI. Confirm dialog before deletion
(reuses ConfirmDialog component + pattern from ApiKeys.vue).
Backend DELETE endpoints already existed — no backend work:
- DELETE /api/settings/api-keys/anthropic
- DELETE /api/settings/api-keys/github
- DELETE /api/settings/slack
Audit of other Settings sections: Trinity Prompt has clearPrompt,
Skills Library blanks via deleteSetting, MCP URL has resetMcpUrl,
GitHub Templates/Email Whitelist have inline remove. Agent Quotas are
config values, not secrets.
Closes #459.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(migrations): swallow duplicate-column race on cold start (#456) (#537)
* fix(migrations): swallow duplicate-column race on cold start (#456)
`_migrate_sync_health` (#389) used a check-then-act PRAGMA → ALTER
pattern that is not atomic across uvicorn workers. On cold start with
`--workers 2`, both workers passed the PRAGMA before either committed
the ALTER, and the loser crashed its child process with
`sqlite3.OperationalError: duplicate column name: auto_sync_enabled`.
Fix:
- Add `_safe_add_column` helper that swallows the duplicate-column
OperationalError (treats it as success — another worker won the race).
Future migrations should route ALTER TABLE ADD COLUMN through it.
- Refactor `_migrate_sync_health` to use the helper for both column
additions and switch the bare `CREATE TABLE` to `CREATE TABLE IF NOT
EXISTS` (atomic in SQLite).
Tests:
- `test_safe_add_column_swallows_duplicate_column_race` — drives the
exact production race via a PRAGMA-lying cursor proxy.
- `test_safe_add_column_propagates_other_errors` — non-duplicate errors
still raise.
- `test_safe_add_column_returns_true_when_added` — happy path.
- `test_migrate_sync_health_idempotent_under_race` — `_migrate_sync_health`
is now safe to re-run on already-migrated schemas, including under
the simulated race.
The other ~50 ALTER ADD COLUMN sites are untouched: they're already
applied on production DBs (run_all_migrations short-circuits via the
schema_migrations tracking table). The race only bites new migrations
on first cold-start; the helper is available for them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(migrations): route all ALTER ADD COLUMN through _safe_add_column (#456)
Mechanical sweep of every check-then-act `PRAGMA table_info` →
`ALTER TABLE ADD COLUMN` site through the `_safe_add_column` helper, so
new migrations on a fresh cold-start with `--workers N` are race-safe by
default — not just `_migrate_sync_health` (the originally reported case).
22 migrations refactored. Bare `try/except Exception` swallows in
`_migrate_chat_messages_source_column` and
`_migrate_agent_ownership_voice_prompt` are also replaced with the
helper, which catches only the duplicate-column error instead of every
exception class.
Verification:
- Schema dump (init_schema + run_all_migrations on fresh DB) is
byte-identical before and after the sweep — every column type,
default, FK, and index preserved.
- run_all_migrations is idempotent across runs and across fresh
connections (2nd/3rd runs print no add/create lines).
- 6-worker concurrent stress test (`threading.Barrier`-coordinated)
completes without any worker crashing; final schema is intact.
- tests/unit/test_migrations_concurrent.py +
tests/unit/test_migrations.py + tests/unit/test_guardrails.py:
72 pass, 0 fail.
`tests/unit/test_guardrails.py::test_migration_is_idempotent` updated
to also exec the `_safe_add_column` helper into its isolated
namespace, since the migration now delegates to it.
The two remaining bare `CREATE TABLE` calls in the file
(`_migrate_agent_sharing_table`, `_migrate_agent_skills_table`) are
one-time DROP+CREATE data-recreation migrations that already shipped
on every existing install; they are out of scope for this sweep.
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(git): UI Push no longer commits runtime state (#462)
Expands the platform .gitignore deny-list to cover all runtime files
(.env, .mcp.json, .credentials.enc, instance dirs, content/, Claude
Code state, temp files). Adds idempotent migration that updates
existing agents on next Push and calls `git rm --cached` for files
that are now tracked but newly ignored.
Closes #462
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(frontend): semantic status color tokens (#67) (#553)
Introduce 5 semantic status tokens (`status-success/warning/danger/info/urgent`)
in tailwind.config.js as direct aliases of the green/yellow/red/blue/orange
palettes, then migrate 9 frontend files (4 components, 2 panel-local helpers,
1 composable, 1 utility) from raw color classes to the new tokens. Visual
output is byte-equivalent — tokens compile to identical RGB values.
Add CI safety net: `npm run check:tokens` script verifies token-palette
equivalence and catches typo'd token references in source. Wired into a new
frontend-build.yml workflow that runs `npm ci → check:tokens → build` on PRs
touching `src/frontend/**`.
Drive-by fix: rename postcss.config.js → postcss.config.mjs to fix Node
ESM/CJS interop for local `npm run build` (production Docker build was
masking the issue).
70+ raw-color files remain for follow-up sweep (per autoplan phasing).
Fixes #67
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(files): FILES-001 outbound file sharing — MVP + Phase 1 hardening (#491)
* feat(files): FILES-001 outbound file sharing MVP (Steps 1-6)
Implements outbound file sharing per docs/drafts/amazing-file-outbound.md:
- Schema + migration (agent_shared_files table with FK cascade)
- Per-agent opt-in toggle + Docker publish volume (agent-{name}-public)
- Internal share endpoint with path/MIME/size/quota validation
- Public download endpoint (/api/files/{id}?sig=...) with token auth
- share_file MCP tool (agent-scoped)
- SharingPanel UI: toggle, list, revoke, copy URL
Live-verified on Slack: agent→share_file→URL→download end-to-end.
Unit tests: 33 passed (migration, mixin, mount-match).
Known limitations + production readiness plan:
docs/drafts/amazing-file-outbound-production-readiness.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(files): FILES-001 — requirements, architecture, feature-flow doc
- requirements.md §13.10 new entry marking FILES-001 Implemented (2026-04-24)
- architecture.md: add files.ts MCP module, agent_shared_files_service,
routers/files.py, the 5 new API endpoints + dedicated section, and the
agent_shared_files table schema + operational notes
- feature-flows.md: Recent Updates entry + Documented Flows index
- feature-flows/file-sharing-outbound.md: new full vertical-slice doc
(UI → store → router → service → DB → download) matching the template
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* security(files): cap filename length at 255 chars (C2)
ShareFileRequest.filename and ShareFileMcpRequest.filename get
Field(max_length=255, min_length=1). display_name same cap.
Prevents 10KB+ filename edge cases from agent or attacker.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* security(files): disk-space pre-check before write (C3)
New check_disk_space() helper using shutil.disk_usage('/data').
Refuses writes when /data has less than size_bytes + 500MB free
(HTTP 507 Insufficient Storage). Called before persisting.
Protects shared /data mount — SQLite DB, Vector logs, and log
archives live there too; letting an agent fill the disk causes
platform-wide outage, not just a failed share.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(cleanup): purge expired and old-revoked shared files (C4 / Step 7)
Adds delete_expired_and_revoked(revoke_grace_hours=24) to the DB ops
class (returns stored_filename list for disk unlink) + facade forward
+ wired into cleanup_service.py's 5-min tick.
Per cycle:
- SELECT rows where expires_at < now OR revoked_at < now - 24h
- DELETE them from DB
- unlink each /data/agent-files/{stored_filename}
- bumps CleanupReport.shared_files_purged
The 24h grace on revoked rows keeps them queryable for incident
diagnosis right after revocation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* security(files): dedicated rate-limit bucket for downloads (C5)
/api/files/{id} now uses _check_file_download_rate_limit which keys
redis by file_downloads:{ip} instead of sharing the public_link_lookups
bucket used by /api/public/chat and friends. Limits unchanged (60/min
per IP).
Prevents heavy download traffic from starving the rate-limit quota for
public chat or other /api/public/* endpoints on the same IP.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(files): HEAD handler mirroring GET validation (C6)
Link previewers (Slackbot, Twitterbot, Discordbot, facebookexternalhit)
HEAD-probe URLs before GET. Our endpoint was 405-ing those.
Extracted _validate_download_request() helper from GET; new HEAD
handler reuses it and returns Response(200) with the same headers
(Content-Disposition, nosniff, no-store, Content-Length) but no body,
no download counter bump, no audit row. Follows RFC 7231 §4.3.2.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* security(files): tighten list endpoint to owner+admin only (C7)
GET /api/agents/{name}/shared-files previously used can_user_access_agent
(owner/admin/shared). But the list response includes full download URLs
with signed tokens — so anyone able to see the list can reuse every
share. That's the same capability as share_file + revoke, both of which
already require can_user_share_agent.
Change to can_user_share_agent (owner + admin), 403 otherwise.
DELETE was already owner-only; no change needed there.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(prompt): agent nudge for share_file MCP tool (C8)
Add a new 'Sharing Files with Users' section to the system-wide
PLATFORM_INSTRUCTIONS between Collaboration and Operator Communication.
Tells every agent:
- write files to /home/developer/public/
- call share_file MCP tool with the relative filename
- return the URL as-is
This means new agents discover the capability without the user
needing to name the tool explicitly. Applies immediately to every
agent via compose_system_prompt() — no image rebuild needed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(pr-491): address validation findings — PII redaction + scope drift
PR #491 /validate-pr flagged two issues:
1. CRITICAL: pavshulin@gmail.com in two draft docs' Owner fields
(amazing-file-outbound.md, amazing-file-outbound-production-readiness.md).
Public repo + CLAUDE.md forbids real user emails.
→ Replaced with @pavshulin (GitHub handle).
2. WARNING: .claude/settings.json committed as new file — personal
Claude Code permission allowlist unrelated to FILES-001 scope.
→ Merged 8 allowlist entries into .claude/settings.local.json
(gitignored per .gitignore:67). Removed .claude/settings.json.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: fix test-ordering contamination from FILES-001 mixin fixture
Two adjustments surfaced by running the full unit suite:
1. test_file_sharing_mixin.py registered `sys.modules['db']` as a plain
module (no `__path__`), which poisoned `from db.X import Y` lookups in
sibling tests (e.g. test_fleet_sync_audit did `from db.schedules ...`
and hit `'db' is not a package`). Now we give our stub a `__path__`
pointing at the real db directory, and restore `sys.modules['db']` on
fixture teardown so no leakage remains.
2. test_start_agent_skip_inject.py didn't mock the new
`check_public_folder_mount_matches` import added by FILES-001 in
services/agent_service/lifecycle.py. The Mock container lacked
iterable `attrs["Mounts"]`, blowing up with TypeError. Stubbed the
whole `file_sharing` submodule and bound the check on `_mod`
per-test to return True by default.
Full unit suite now matches dev baseline: 17 pre-existing failures,
701 passing (+33 over dev, all from FILES-001).
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>
* fix(channels): deliver images as vision content blocks via stream-json (#562) (#566)
Replaces the broken base64 data-URI-in-text approach (where Claude Code
received images as opaque markdown strings) with proper vision content
blocks fed via --input-format stream-json stdin. Images sent through
Telegram (and other channel adapters) are now visible to the agent.
- message_router: _handle_file_uploads returns 4-tuple (added image_data);
image MIME files collected as {media_type, data} dicts instead of embedded
- task_execution_service: execute_task() accepts images param, forwards in payload
- agent_server models: ParallelTaskRequest.images field added
- agent_server chat router: passes images to runtime.execute_headless()
- claude_code: adds --input-format stream-json and builds JSON content-block
stdin payload when images present; stdout/stderr threads start before stdin
write to prevent pipe deadlock; write moved into executor (not event loop)
- runtime_adapter ABC + GeminiRuntime: images param added to prevent TypeError
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(frontend): add state, brand, accent token families (#555) (#561)
Extends the design-system token system from #67 with three additional families
for colors that don't fit the status taxonomy:
state-* agent operating modes (autonomous, locked)
brand-* third-party product identity (claude, gemini)
accent-* decorative highlights named after the literal color so future
accents (accent-green, etc.) join cleanly
New tokens (all alias full Tailwind palettes, identical visual output):
state-autonomous → amber (AutonomyToggle AUTO mode)
state-locked → rose (ReadOnlyToggle ON mode)
brand-claude → orange (RuntimeBadge for Claude Code)
brand-gemini → blue (RuntimeBadge for Gemini CLI)
accent-purple → purple (DashboardPanel widget badges)
Also extends scripts/check-design-tokens.mjs to validate the new families
via a KNOWN_FAMILIES map; the reference scanner now flags typos within any
of the four families (status/state/brand/accent), not just status-*.
Migrates the 4 components blocked by #67's status-only scope:
- RuntimeBadge.vue → brand-claude, brand-gemini
- AutonomyToggle.vue → state-autonomous
- ReadOnlyToggle.vue → state-locked
- DashboardPanel.vue → accent-purple slot in getStatusColors
Fixes #555
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(scheduler): agent-owned pre-check hook (#454) (#455)
* feat(scheduler): agent-owned pre-check hook (#454)
New optional contract: agents implement POST /api/pre-check in their
container; scheduler calls it before firing a cron-triggered chat.
Endpoint absent or any error → fire as usual (fail-open). fire=false
records a skipped execution. fire=true with a message overrides the
schedule.message for that invocation.
- docker/base-image/agent_server/routers/pre_check.py: new router that
dynamically loads /home/developer/.trinity/pre-check.py (template-
supplied) and calls its check() function
- agent-server main.py: mount pre_check_router
- scheduler/agent_client.py: pre_check() method with fail-open semantics
on 404/5xx/timeout/malformed-response
- scheduler/service.py: _run_pre_check + pre-check branch in
_execute_schedule_with_lock (cron only; manual triggers bypass)
- tests/scheduler_tests/test_pre_check.py: 12 tests covering client-
and service-level behavior; 161/161 scheduler suite passes
Zero schema change — reuses existing ExecutionStatus.SKIPPED and
create_skipped_execution. Closes the "wake agent on every cron tick"
cost gap noted in docs/planning/PR_REVIEWER_AGENT.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(#454): scheduler pre-check feature flow + arch + requirements
- feature-flows/scheduler-pre-check.md: new flow doc with contract,
fail-open semantics, error table, testing summary
- architecture.md: add /api/pre-check to agent-server endpoint list
and pre-check note to Scheduler Service row
- requirements.md: SCHED-COND-001 entry under §10 (Scheduling & Execution)
- feature-flows.md: index row
- docs/planning/PR_REVIEWER_AGENT.md: design doc from which this
feature was extracted — committed for traceability
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* review: address PR #455 review feedback
- pre_check.py: asyncio.get_event_loop() → get_running_loop() (deprecated in 3.10+)
- pre_check.py: oversized message override no longer dropped silently —
response now carries message_truncated="override dropped: N bytes exceeds
32000 cap" so scheduler/operator can see what happened; log escalated to
ERROR with size+limit details
- pre_check.py: module-level docstring expanded to note the security scope
of check() (full Python interpreter access, same sandbox as chat tools —
operators should review .trinity/pre-check.py like any executable template
file) and the intentional no-cache behavior
- tests/unit/test_pre_check_router.py: 15 new router/unit tests covering
oversized-message drop path and non-dict return → 500 (both previously
only exercised by inspection). Uses importlib to load pre_check.py
directly, avoiding python-multipart requirement from sibling routers
- feature-flows/scheduler-pre-check.md: document truncation behavior,
security scope expectation, and updated test summary (12 scheduler +
15 router = 176 total passing)
Lock-scope concern noted in review is not an issue: the skip path returns
from _execute_schedule_with_lock, and the outer _execute_schedule holds
the lock in a try/finally that covers the return. No leak.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(#454): docker exec instead of agent-server HTTP endpoint
Review feedback on #455 flagged that the HTTP-endpoint design introduced
a new system edge (scheduler → agent-server direct) and a novel code-
loading pattern (importlib in a router). Both broke with Trinity's
established convention that all "run something in an agent container"
flows go through `services/docker_service.execute_command_in_container`
— the same primitive used by:
- services/git_service.py (persistent-state allowlist, #384 S3)
- services/ssh_service.py (key provisioning)
- services/agent_service/terminal.py (web SSH)
- routers/system_agent.py (admin exec)
- adapters/message_router.py (Slack file ingest)
- routers/voice.py, monitoring_service.py
This commit swaps the design accordingly.
Changes:
- Delete docker/base-image/agent_server/routers/pre_check.py and its
router registration. No new HTTP surface on agent-server.
- Delete tests/unit/test_pre_check_router.py (router is gone).
- Add src/backend/routers/internal.py →
POST /api/internal/agents/{name}/pre-check. Runs the template-shipped
`.trinity/pre-check.py` via execute_command_in_container. Two-step:
`test -f` for existence, then `python3 .../pre-check.py`. Returns
{hook_present, exit_code, stdout, stderr}. Gated by existing
X-Internal-Secret header (C-003).
- Rewrite src/scheduler/service.py::_run_pre_check to call the backend
endpoint (scheduler no longer opens a direct edge to agent-server).
Translates …
5 tasks
5 tasks
11 tasks
vybe
pushed a commit
that referenced
this pull request
May 4, 2026
* fix(security): split Docker compose into platform and agent networks (#589) Redis at 172.28.0.0/16 was reachable from any agent container. AISEC scan 3aad5469 demonstrated end-to-end exfiltration / cross-user task injection from a legitimately deployed agent. Network segmentation is the strongest control — agents now physically cannot route to Redis. Topology: - trinity-platform (172.29.0.0/16, NEW) — Redis, scheduler, vector - trinity-agent (172.28.0.0/16, name preserved) — frontend, agents - Backend / mcp-server / otel-collector / cloudflared straddle both Agent-creation sites in services/agent_service/* and system_agent_service.py need zero changes because the agent-network external name is preserved. Dev: bind Redis host port to 127.0.0.1:6379 (was 0.0.0.0). Tests connect from the dev machine; LAN cannot. Auth lands in the next commit. Refs #589 — acceptance criterion #3 (network segment separation). * fix(security): mandatory Redis auth, ACL users, auth-aware healthcheck (#589) Both compose files now enforce two passwords (REDIS_PASSWORD admin / REDIS_BACKEND_PASSWORD runtime) with the fail-on-missing :? form. docker compose refuses to render without them. Per-user ACL via inline --user flags. Additive (start from zero, allow only what the runtime needs) — never +@ALL -X, which lets newly added dangerous commands through. backend + scheduler get standard data families plus scripting/transactions/pubsub minus -@dangerous, which covers FLUSHALL, CONFIG, SHUTDOWN, MIGRATE, REPLICAOF, MONITOR. Verified at runtime against redis:7-alpine: PING/SET/GET work for the backend user, FLUSHALL and CONFIG GET return NOPERM, unauth requests return NOAUTH. REDIS_URL on backend + scheduler now embeds the backend ACL user. mcp-server: REDIS_URL and depends_on:redis dropped in prod compose (zero Redis imports in src/mcp-server/). Healthcheck pings as the backend ACL user so a typo'd ACL keeps redis unhealthy and gates dependent services. depends_on:redis switches to service_healthy so backend/scheduler don't race the ACL load. Refs #589 — acceptance criteria #1, #2, #5. * fix(scheduler-test-rig): mirror Redis auth posture (#589) Without this, scheduler container fails fast on startup against the rig because src/scheduler/config.py requires creds in REDIS_URL after #589. No ACL or network split here — this is a 2-service standalone debugging rig, not the production posture. * fix(security): fail-fast on REDIS_URL missing credentials (#589) Backend (src/backend/config.py) and scheduler (src/scheduler/config.py) now raise RuntimeError at import time if REDIS_URL is unset or lacks credentials. Removed the splicing fallback in backend config that papered over an unauth REDIS_URL by joining REDIS_PASSWORD into the URL — single source of truth (compose) eliminates silent drift. Tests that import backend modules need a creds-bearing REDIS_URL in their environment; tests/conftest.py will set a dummy one in the test commit. Refs #589 — acceptance criterion #5. * fix(webhooks): use REDIS_URL for rate-limit client (#589) Webhooks rate-limit was the one Redis client that bypassed REDIS_URL — it used redis.Redis(host="redis", port=6379) and would silently fail-open under requirepass. Switching to redis.from_url(REDIS_URL) picks up the credentialed URL like every other client. Also: distinguish auth/ACL errors (logged at ERROR with exception class) from transient errors (WARN). Fail-open behavior preserved so a Redis blip doesn't 500 legitimate webhooks, but a misconfigured deploy now surfaces in alerts instead of via a webhook abuse incident. Drops the now-unused REDIS_HOST/REDIS_PORT env reads. * feat(deploy): auto-generate Redis passwords on fresh installs (#589) start.sh ensure_redis_passwords matches the existing CREDENTIAL_ENCRYPTION_KEY pattern, with one safety guard: - Fresh install (no redis-data volume) → generate both passwords with openssl rand -hex 24 and append to .env. One-command boot keeps working. - Existing volume + missing password → refuse with a loud error pointing at docs/migrations/REDIS_AUTH.md. Re-keying a populated Redis would lock the backend out of its own data; ops needs to follow the explicit upgrade path. Idempotent — second run is a no-op when both passwords are already set. * docs(security): add Redis auth migration guide + architecture notes (#589) - docs/migrations/REDIS_AUTH.md: operator upgrade guide. Covers fresh installs (auto-generated by start.sh), live upgrades (down --remove-orphans + docker network rm + add passwords), production, and verification commands. - docs/memory/architecture.md: new "Network Topology (Issue #589)" section above Container Security. Documents the two-network split, service membership table, the "agents NEVER on platform network" rule, and the three Redis ACL users + their access patterns. * test(security): network isolation, ACL, fail-fast, webhook rate-limit (#589) tests/conftest.py: top-level autouse env stub for backend imports. Backend config now raises at import-time if REDIS_URL lacks credentials; without this, every test that transitively imports backend modules breaks. Real Redis tests under tests/security/ override via their own conftest from .env. Adds the `integration` marker. tests/unit/test_config_fail_fast.py (new): backend refuses to import without creds-bearing REDIS_URL. 3 cases — missing env, unauth URL, URL with creds. tests/security/test_redis_network_isolation.py (new): 5 integration tests covering acceptance criteria #1-#3: - agent-network container has no route to redis (BLOCKED) - unauth client gets NOAUTH on platform network - backend ACL user can PING with creds - backend ACL user FLUSHALL → NOPERM (no admin) - backend ACL user CONFIG GET → NOPERM (no requirepass leak) tests/security/conftest.py (new): session-scoped fixture loads real .env values for the integration tests; skips the suite if missing. tests/integration/test_webhook_rate_limit.py (new): regression for the from_url switch in webhooks.py. Self-contained — creates agent + schedule + webhook token inline, hits 11×, expects 429 on the 11th. Catches the silent fail-open if Redis auth ever regresses. tests/run-integration.sh (new): pytest -m integration runner. Excluded from run-smoke.sh per the smoke runner's ~30s no-Docker contract. * docs(security): detach agents before network rm (#589) Trinity-managed agent containers are created via the Docker SDK outside compose, so they store the agent network's UUID, not its name. After `docker network rm trinity-agent-network` (step 3 of the upgrade procedure), any later `docker start <agent>` fails: Error response from daemon: failed to set up container networking: network <old-uuid> not found Compose-managed services don't hit this — they're recreated with fresh network refs on `up`. Agent containers aren't, so they keep the stale UUID until disconnected. Add an explicit detach loop as step 2, before the network removal. Verified against a populated install with one running and four stopped agents: all five reattach cleanly to the new network on next start. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(security): CSO OBS-1/2/3 follow-ups — webhook rate-limit + healthcheck hardening (#589) Resolves three observations from the CSO audit (docs/security-reports/cso-2026-05-04-589-diff.md): OBS-1 — webhook rate-limit fail-open + connection-per-request DoS amplifier: * Added in-process secondary rate limiter (3x primary, per-worker) in src/backend/routers/webhooks.py. Bounds blast radius during a Redis outage without breaking the documented fail-open philosophy. * Cached the Redis client at module level under threading.Lock with double-checked init. _check_webhook_rate_limit resets the cache on inner exceptions so stale connections rebuild cleanly. Without caching, a flood would open a fresh TCP per request and exhaust Redis maxclients — turning the rate limiter into the DoS amplifier. OBS-2 — tightened _TOKEN_RE from {20,60} to {43} matching secrets.token_urlsafe(32) (verified against db/schedules.py:524). OBS-3 — switched all three compose healthchecks from `redis-cli -a $$PASS` to `REDISCLI_AUTH="$$PASS" redis-cli` so the password no longer appears in /proc/<pid>/cmdline. Additional #589 hardening (caught while resolving OBS-1): * src/backend/config.py + src/scheduler/config.py: tightened the REDIS_URL credential check from `"@" in url` substring to urlparse validation. Catches redis://@redis:6379, redis://user@redis:6379, etc. * src/scheduler/main.py: redact password from REDIS_URL before logging (was leaking via Vector log aggregator). Tests: * tests/unit/test_webhook_rate_limit_inprocess.py — 7 new tests covering cap, window expiry, token isolation, runtime-error fallback, regex shape, cache hit, cache reset. * tests/unit/test_config_fail_fast.py — 4 new parametrized cases for malformed-credential URL rejection. * 15/15 unit tests pass. * Live Redis healthcheck verified — trinity-redis reports healthy with the new REDISCLI_AUTH form; `redis-cli ping` returns PONG. Also adds .gstack/ to .gitignore so future skill artifacts stay local. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Jul 17, 2026
… terminal (#1578) (#1680) * docs(requirements): system-emitted task completion events (#1578) Rule #1 — document the new capability before implementing it. Adds §17.2a beside EVT-001: backend emits agent.task.completed/failed at every CAS-won terminal, matching-sub gated, reserved namespace + recursion-break, EVT-001 dispatch (best-effort to a running subscriber), inert by default. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(events): event_dispatch_service — extracted delivery + system emit helper (#1578) New services/event_dispatch_service.py: - trigger_subscription / _interpolate_template / _get_internal_token moved verbatim out of routers/event_subscriptions.py so a service (TES) can reuse the EVT-001 dispatch primitive without importing a router (Invariant #1). - emit_task_terminal_event + spawn_task_terminal_event (#1578): the single shared helper the backend fires at every CAS-won terminal — matching-sub gated (empty => no row, no dispatch), recursion-break on triggered_by='event', flat interpolable payload incl. fan_out_id/loop_id, fail-open. - trigger_subscription stamps the reserved-namespace loopback with X-Event-Trigger so the spawned task persists triggered_by='event'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(events): reserved-namespace guards + recursion-break wiring (#1578) - event_subscriptions.py: import the extracted event_dispatch_service (delete the moved defs); reject agent emits into agent.task.* on both emit routes (400); block reserved-namespace self-subscription on create AND update (400); dispatch via event_dispatch_service.trigger_subscription. - chat.py: read X-Event-Trigger on /task; a reserved-namespace dispatch persists triggered_by='event' (threaded via triggered_by_override) so the emit helper suppresses that task's own terminal event — the recursion-break. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(events): emit agent.task.* at every task_execution terminal (#1578) - apply_result SUCCESS won path -> agent.task.completed (sanitized_resp, cost, duration); covers both #1083 paths (inline sync + async result-callback both converge here). - apply_result FAILURE won path -> agent.task.failed (envelope.status carries failed/cancelled; salvage cost). - _write_terminal_and_gate gains agent_name and emits agent.task.failed on won — covering the timeout / budget-exhausted / unexpected-exception (+ inline circuit-open/capacity/ephemeral) terminals that never reach apply_result, the exact wedge case the feature exists for. agent_name threaded from all 4 execute_task call sites. All emits are CAS-won only, fire-and-forget, fail-open. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(events): emit agent.task.failed at the #1083 lease-reaper terminals (#1578) _sweep_stale_slots' two per-execution CAS-won sites (async lease expired with no result callback) now emit agent.task.failed — waking a subscribed orchestrator on the exact wedge the feature exists for. Fire-and-forget + fail-open; the bulk watchdog sweeps stay a documented residual (no per-row context). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(events): emit agent.task.* at the pull-sink terminal (#1578) apply_task_result's CAS-won path now emits agent.task.completed/failed. Dark until a pull pilot is enabled (#1081) but wired so a pilot doesn't silently regress report-back. CAS-won only (replay/late report short-circuits or loses the CAS), fire-and-forget + fail-open. Coordinate with obasilakis (his file). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(mcp): note reserved agent.task.* namespace in events.ts (#1578) Invariant #13 doc-only: emit_event notes agent.task.* is reserved/backend-emitted (rejected if emitted); subscribe_to_event notes you can subscribe to a source agent's agent.task.completed/failed for an automatic report-back task instead of polling, with the payload shape and the no-self-subscription rule. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(events): unit coverage for system-emitted task completion events (#1578) 27 tests, all green: - Emit helper: fired (success/failed/cancelled), not-fired-no-sub (AC #1/#5), recursion-break, status-as-.value (#1085 footgun), payload shape (fan_out_id/loop_id), duration/cost row fallback, truncation, fail-open. - Every CAS-won terminal writer spawns on won / not on lost CAS: apply_result inline path, _write_terminal_and_gate (timeout class — the critical pin), the #1083 lease-reaper, and the pull sink. - Both #1083 terminal paths GENUINELY (dossier §5): inline apply_result call AND the async result-callback endpoint driving the REAL apply_result — distinct entry points, not two tests on the same inline call. - Reserved-namespace guards: emit 400, create self-sub 400, PUT self-sub 400, cross-agent subscribe allowed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(feature-flows): task-completion-events flow + index + cross-links (#1578) New feature-flows/task-completion-events.md: end-to-end report-back path, the terminal-writer coverage table, system- vs agent-emitted split, 3-layer loop safety, honest best-effort delivery note, content-trust note, test map. Adds the Recent Updates row + category index entry (feedback: always add the index row), and cross-link pointers from task-execution-service.md and agent-event-subscriptions.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(architecture): Task Completion Events cross-cutting block (#1578) - New 'Task Completion Events (#1578)' subsystem block: system- vs agent-emitted, shared event_dispatch_service helper, CAS-won terminal-writer coverage table, 3-layer loop safety, honest best-effort delivery note. - Pointer from Fire-and-Forget Dispatch (#1083); services-catalog row for event_dispatch_service.py; reserved-namespace note on the events.ts MCP row + the agent_events DDL. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(target-arch): reconcile §Async-First — completion events implemented (#1578) Mark the agent.task.completed/failed contract IMPLEMENTED, note the divergence (EVT-001 subscription dispatch, best-effort to a running subscriber; durable pull-queue delivery deferred; not the WS event_bus) and the pull-sink coverage. Add a '├─ Bankable win 3' #1578 row under the #1081 umbrella. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(events): authenticate recursion-break header + sanitize task-event summary (#1578) Two review findings on the #1578 completion-event path, both pre-merge: 1. Spoofable recursion-break. The reserved-namespace loopback was suppressed purely on an `X-Event-Trigger` header, so an external `/task` caller could spoof it to suppress a real agent's completion event. The header is now honored ONLY with a valid backend-internal `X-Internal-Secret` (C-003): `trigger_subscription` stamps both, `execute_parallel_task` verifies via `event_dispatch_service.verify_internal_dispatch_secret` (constant-time) before persisting `triggered_by="event"`. 2. Un-sanitized failure summary. Success/pull terminals sanitize upstream, but the failure error strings (`envelope.error` / `str(exc)`) reached the emit helper raw. `emit_task_terminal_event` now credential-sanitizes at the single chokepoint over a 2x-cap window BEFORE the final truncation, so a secret straddling the `TASK_EVENT_SUMMARY_MAX` boundary can't leak an un-redactable head fragment. Tests: +7 (spoof-guard at the helper AND the router call-site; sanitize incl. the boundary-straddle case) -> 37 pass. Docs: architecture.md delivery note + feature-flow recursion-break/second-terminal notes updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
vybe
pushed a commit
that referenced
this pull request
Jul 18, 2026
…) (#1687) Follow-up 5/5 to the #964 display-name track, building on the ent#181 display_label foundation (#1676): the label + immutable slug, the `utils/agentName.js` resolver (`agentDisplayName`/`hasDistinctLabel`/ `agentNameTooltip`), and the `agent_label_changed` WS handler already shipped. This spreads the label to the surfaces #1676 didn't touch. Decision (AC #1): resolve slug -> label on the FRONTEND off the loaded agents, not by adding a mutable `display_name` to operator/monitoring/executions payloads. Those endpoints are high-volume; duplicating a presentation value across each invites staleness and N sources of truth. Two store getters do it from one place and stay live via the existing WS handler: - `displayNameForSlug(slug)` -> label or slug (prose) - `agentRefForSlug(slug)` -> agent object or bare slug (feeds the tooltip) An unloaded slug falls back to itself, so a cold surface never regresses. Render rule by surface class (#964): - Dense operational tables keep the SLUG primary, label as a hover tooltip: ExecutionsPanel, operator QueueCard/QueueList/QueueItemDetail/ResolvedCard/ NotificationsPanel, MonitoringPanel (alert + health rows), MobileAdmin ops + notif rows, RoleMatrix (100px column, slug-only + tooltip). - Prose / toasts use the label alone: Agents.vue toasts, Settings.vue agent confirm + subscription option, PortalConversation header/switcher/placeholder, PortalFilesPanel headings/toast. Comma-joined lists (GitHub-PAT propagation) stay slug — long labels make them unreadable. - Collaboration graph: network.js carries `display_label` on the agent and system-agent node data + a live `agent_label_changed` handler; AgentNode renders the label but keeps `data.label` (the slug) as the action key for router.push / toggleAutonomy / toggleAgentRunning. AgentAvatar stays on slug. - Router tab titles resolve the label on warm SPA nav, fall back to the slug on a cold direct load (store not fetched yet); next nav self-heals. - Reviewed against the rule: ExecutionDetail breadcrumb + source-agent attribution (label + slug link), PermissionsPanel target rows, ReplayTimeline lane, FoldersPanel consumer/source rows, AgentWorkspace + AgentBrainOrb headers. Terminal header stays slug (machine-facing). Requirements §1.3.1 FR-6 records the decision. No backend change. All 20 touched SFCs compile via @vue/compiler-sfc; the 3 JS files pass node --check. Related to #1643 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Jul 18, 2026
…se#162) (#1686) Let each user store a personal GitHub PAT so a non-admin can create an agent from a repo the shared admin token can't see. Agent creation now resolves per-agent -> owner's per-user (live) -> global, and each agent carries its creator's scope instead of the fleet-wide admin token. Shape A (create-time convenience): - users.github_pat_encrypted (AES-256-GCM, Invariant #12) + four accessors with DatabaseManager delegations; column kept out of the general user dict so it never rides /api/users/me. - resolve_github_pat(agent_name, owner_id) in settings_service returns (pat, tier); both crud.py create sites use it. Persist as the #347 per-agent PAT ONLY for tier in {per_user, fork}, never global, so a global-fallback agent keeps NULL and still receives admin rotation. - get_github_pat_for_agent relocated into settings_service (kills the service->router import, Invariant #1) and stays 2-tier per-agent -> global. The recreate/restart ladder never re-derives the per-user tier, so adding a PAT in Settings can't force-recreate a running agent (the #1560/#1557 recreate-storm class). - Self-service GET/PUT/DELETE /api/users/me/github-pat: token never echoed on read; honest 400 (GitHub rejected) vs 503 (unreachable); create's bare 500 becomes an actionable 400. - UserGitHubPatPanel.vue on the personal MCP Keys settings tab. - users column registered in rotate-credential-key.py; dual-track migration (SQLite + Alembic 0025 on head 0024). Out of scope: public-repo-no-PAT (#123), OAuth/GitHub App (#177), rotation fan-out to existing agents (fast-follow). Tests: real migrated-DB + real facade — resolution ladder, owner isolation, persist-only-user-tier, live column + encrypted round-trip, facade delegation, read-never-echoes; plus the load-bearing recreate-no-storm guard (proven to fail if the ladder goes 3-tier). Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
vybe
pushed a commit
that referenced
this pull request
Jul 19, 2026
…ne auth wiring (#1310) (#1683) * docs(auth): INV-8 shared imperative-guard family — auth.md §2.7 + Invariant #8 (#1310) Requirements-driven first step (Rule #1) for the INV-8 auth-dependency consolidation. Documents the five leaf helpers (assert_admin, assert_agent_access, assert_agent_owner, assert_owns_or_admin, assert_owns), the preserve-403 rationale (access-first = self-uniform per #186; #1445 precedent), the imperative-vs-path-dependency rule, the assert_agent_owner != delete-authorization footgun, and the permanent exceptions (nevermined/reports intentional-404, sessions compound-404, slack.py deferred). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(auth): characterization suite locking current inline-auth behavior (#1310) Stage 3 — behavioral lock BEFORE any migration (must stay green after). Real-DB db_harness drives the real db.can_user_* predicates (never a mocked auth check — a wholesale-Mock db false-greens a broken gate); only non-auth resource lookups are stubbed. Covers every in-scope site: admin gates, access gates, owner gates, Shape-F owns-or-admin (admin admitted + anti-IDOR binding), public.py strict-self (the admin-non-owner-403 access-widening guard), the agent_files anti-exfil sibling, the schedules #1445 access-before-404 ordering, and the loops composite gate. Asserts exact status + detail + admitted-principal set per site. Also hardens test_186 tier4 to compare the bound access dependency by name, not object identity: the unit conftest pops `dependencies` between tests while already-imported router modules persist, so a co-resident test importing agent_config in an earlier generation left the annotation bound to a stale (no-longer-`is`-equal) helper object. Name comparison proves the same invariant without the re-import fragility. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(auth): add INV-8 imperative-guard family + relocate narrow_to_agent (#1310) Stage 2 — the five leaf helpers in dependencies.py (assert_admin, assert_agent_access, assert_agent_owner, assert_owns_or_admin, assert_owns), each raising 403 and access-first (self-uniform); the agent-name helpers run _enforce_connector_scope first to match the path-dependencies' second fence. No new import edge (dependencies.py already imports db) and no new DB accessor. Also relocate _narrow_to_agent from routers/executions.py to services/agent_service/helpers.py as narrow_to_agent (removing a router->router import edge; reports.py + executions.py repointed). Semantics unchanged. Part B helper-level truth tables added to the characterization suite: connector fence, owner-or-admin, strict-self (no-admin bypass) + owner_id int/int type parity. 39/39 green. Sites are migrated onto these helpers in the next commits. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(auth): consolidate inline admin gates onto assert_admin/require_admin (#1310) Bucket 4a (admin). Replaces inline `role != "admin"` raises with assert_admin (avatar generate-defaults, nevermined settlement-failures/retry, schedules scheduler-status) and deletes the four router-local `require_admin` imperative dupes (subscriptions/system_agent/ops/settings — ~67 call sites swapped to assert_admin). logs.py's local `require_admin` Depends is replaced by the shared dependencies.require_admin. Exact 403 "Admin access required" preserved (custom avatar detail preserved); assert_admin/require_admin additionally reject connector principals — a no-op tightening (connectors are fenced upstream). _check_sdk() ordering before the nevermined retry admin gate preserved. Characterization suite (39) stays green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(auth): consolidate inline access/owner gates onto assert_agent_* (#1310) Bucket 4b (access + owner). Replaces inline db.can_user_access_agent →403 with assert_agent_access (agent_config resources/capacity, avatar identity, public_memory, schedules create, notifications ×3, subscriptions auth-status, loops composite fallback) and inline db.can_user_share_agent →403 with assert_agent_owner (agent_config ×6, agent_files ×3, subscriptions ×2, event_subscriptions ×2, messages). Exact 403 + per-site detail preserved. Load-bearing orderings/siblings preserved verbatim: * schedules.create_schedule — access-403 stays BEFORE the is_agent_live 404 and the #929 timeout read (#1445 anti-enumeration order); * public_memory — the execution-404 / execution.agent_name-403 binding stays; * agent_files.share — the anti-exfil agent-scoped-key sibling check kept; * loops._check_loop_access — admin + initiator allow-returns kept, only the can_user_access fallback migrated. Characterization suite (39) stays green; only slack.py (deferred) + the 4 Shape-F voice/chat sites (next bucket) remain flagged by the guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(auth): consolidate Shape-F + strict-self session gates (#1310) Bucket 4c. Migrates the strict-self-or-admin session gates to assert_owns_or_admin (voice_stop / get_voice_panel / chat get-session-detail / close-session) and the public-link session gate to assert_owns (id-only, NO admin bypass — the access-widening guard: an admin who is not the session owner stays 403; mapping it to assert_owns_or_admin would flip that to 200). Anti-IDOR bindings preserved verbatim (the `session.agent_name != name` / `preview.agent_name != name` 403 on the line above each owner gate — the cross-agent-session-read defense, #600). The WS-dict close(4003) handler (voice_ws) and the admin FILTER branches (chat history/session listing) are correctly left inline — they never raise HTTPException. Characterization suite (39) stays green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(auth): static AST guard against inline auth-wiring drift + slack markers (#1310) The durable risk reduction: test_1310_auth_wiring.py forbids, in routers/*.py, a db.can_user_*_agent() call whose negation guards a `raise HTTPException`, and an inline `role != "admin"` deny-If — the shapes the shared dependencies helpers now own. Precise AST matcher (self-tested against planted violations AND the benign shapes: assignments, role=="admin" allow/filter branches, WS close(4003) dict handlers). Per-(file, function) allowlist for the intentional-404 designs (reports.get_report, nevermined._require_*_access) and a `# noqa: inv8` line-exemption for the deferred slack.py sites — so NEW inline auth added to slack.py still trips the guard. slack.py's 10 inline-auth sites carry the `# noqa: inv8` marker (deferred follow-up, coordinate with the channel-adapter owner). Guard is green over the live tree. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(auth): update existing tests for the consolidated auth mechanism (#1310) The INV-8 consolidation moved the inline checks behind dependencies.py helpers, which (a) now read current_user.connector_agent via the connector fence and (b) resolve db.can_user_* through the dependencies module. Existing tests that predated these need small, behavior-neutral fixups: * test_929 / test_1018 / test_117 / test_subscription_reassign — complete the User fakes with connector_agent=None (a real User always carries it — Optional[str]=None; the fakes just predate the field, same convention as the existing agent_name=None). test_subscription_reassign additionally points assert_agent_owner's own `db` global at its fake_db (the owner gate reads dependencies.db, not routers.subscriptions.db). * test_subscription_bola (#182) — asserts the new gate (assert_agent_owner on assign/clear, assert_agent_access on read-only auth-status) instead of the now-moved raw db-call; the behavioral shared-reader-denied case is covered by test_1310_auth_consolidation. No production behavior changed; these lock the same #182/#929/#1445 invariants against the consolidated shape. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(auth): complete more User fakes + dependencies stub for INV-8 (#1310) The consolidated settings admin handlers now flow through assert_admin (which reads current_user.connector_agent). Complete the settings-test User fakes with connector_agent=None (test_85_brain_orb_settings, test_retention_floor, test_1638) — same field a real User always carries. test_telegram_webhook_backfill stubs `dependencies` in sys.modules; add the five new helper names so a router that imports them against the stub resolves. No production behavior changed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(feature-flows): sync role-model + permissions flows for INV-8 (#1310) Fix role-model.md's stale dependencies.py line numbers (ROLE_HIERARCHY 174→503, require_role 177→506, require_admin 158→486, hierarchy check 190→522), add the _reject_connector_principal line to the require_role snippet, and add §1b + a symbol-table row documenting the five imperative auth-guard helpers. agent- permissions.md gains a pointer distinguishing the consolidated router gates from the untouched service-layer inline checks. Feature-flows index gets its Recent-Updates row. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(security): CSO diff report for INV-8 auth consolidation (#1310) PASS — behavior-preserving refactor; no new bypass, no widening, no enumeration oracle. Documents the two-axis analysis (status + admitted-principal set), the public.py strict-self widening guard (assert_owns, no admin bypass), the connector-fence parity tightening, the intentional-404 exceptions, and the static AST drift guard as the durable control. Full unit tier green modulo one pre-existing failure (test_1474, untouched db/schedules.py summary path). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(auth): pin real service modules in char-test against sibling sys.modules leak (#1310) The consolidation suite drives router handler bodies that lazy-import start_agent_internal from services.agent_service (subscriptions admit path). A sibling unit file (test_inject_assigned_credentials) installs a fake services.agent_service package at its own import that persists for the session, so under an adverse collection/execution order the real module was shadowed and the owner-admit path tripped a bare ImportError escaping _raised. Capture the genuine services / services.agent_service / services.docker_service at collection time (evicting any cached fake subtree first) and re-pin all three per test via an autouse monkeypatch.setitem fixture (auto-restores the sibling's stub afterward, so this file stays a well-behaved sys.modules citizen). Verified: 88 passed with test_inject_assigned_credentials collected first. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(feature-flows): correct role-model.md imperative-family line range (#1310) The INV-8 imperative auth-guard family spans dependencies.py 740-801 (assert_admin@740 … assert_owns body end@801), not the stale 744-786. ROLE_HIERARCHY@503 / require_role@506 / require_admin@486 already correct. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(auth): reconcile INV-8 guards with dev after merge (#1310) Post-merge integration fixes for the origin/dev merge: - test_1310_auth_consolidation.py: adopt the linter-recognized `_STUBBED_MODULE_NAMES` + `_restore_sys_modules` escape hatch for the import-time module-eviction (a module-level `sys.modules.pop` no monkeypatch fixture can reach), so `tests/lint_sys_modules.py` passes without a new baseline entry. Behavior unchanged (same three real service modules re-pinned per test). - test_1310_auth_wiring.py: allowlist `chat.execute_parallel_task` — dev's #1083 resume-session check is a compound `role != "admin" AND NOT db.resume_session_belongs_to_user(...)` raising an intentional 404 (session-id enumeration safety, Invariant #8). No shared helper fits (assert_owns_or_admin takes an owner_id and raises 403), so it stays inline like reports.get_report / nevermined._require_*. - test_1578_task_completion_events.py: the update-route owner check moved into dependencies.assert_agent_owner (#1310), so the #1578 reserved-namespace guard test now no-ops the owner gate via the router's own imported binding (module-identity-robust) instead of patching the router db alone. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
vybe
pushed a commit
that referenced
this pull request
Jul 19, 2026
…ored sync seam (#1632) (#1682) * docs(requirements): §26.7 operator-queue ingestion caps (OPS-001-CAPS, #1632) Requirements-before-impl (Rule #1) for the operator-queue create-path rate/depth/size caps: per-agent DB depth cap (primary, Redis-independent), per-agent + fleet rate limits (fail-open), truncate-with-marker field hygiene, reserved-id guard, leader-locked sync, and the platform exemption. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(operator-queue): DB depth-count helper + generous sink belt (#1632) - count_pending_for_agent(agent) -> int: DB-measured per-agent pending depth, the Redis-independent primary bound for the ingestion cap (delegated through database.py). - create_item generous belt: reject title>4KiB / question>16KiB / context>64KiB / id>512 (ValueError the sync loop quarantines) — an order of magnitude past the service caps, so platform/clamped items never trip it (#1525 validate-at-boundary-AND-at-sink). - Fix non-dict context AttributeError in execution_id extraction (a str/list context used to raise and hot-loop the sync). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(operator-queue): ingestion depth/rate/size caps at the sync seam (#1632) Bound a hostile (not just runaway) agent at the one agent-authored create seam (_sync_agent): - DEPTH cap (primary, DB-measured => Redis-independent): stop ingesting once pending+admitted >= OPERATOR_QUEUE_MAX_PENDING_PER_AGENT (25). - RATE cap (burst smoothing, fail-open): per-agent + fleet rate_limiter.check at the create point; denied => hold + break. - Field hygiene (_clamp_ingested_item, total, inside the #1525 try/except): truncate-with-marker title/question; context/options over cap => marker (validated execution_id only); non-dict context => {}; created_at normalized to ingest time; priority validate-only. - Reserved-id guard: reject agent ids with a platform-reserved prefix so an agent can't pre-suppress its own flood alarm / the #1402 poison alert; malformed/oversize ids rejected. - Per-cycle scan bounds: skip an oversized file wholesale; cap scan at 500. - Leader lock (opqueue:leader, mirror monitoring #1464): only the leader worker syncs => no double-charge / double-broadcast / double-scan. - Flood alert: one per episode, un-guessable id, cooldown, emit-failure safe. - validation_service._notify_operator_on_failure => direct DB create (exempt by construction) + fixes the pre-existing bare-list schema bug. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(operator-queue): #1632 ingestion-cap suite + #1525 fake-db fix 45 pure/mocked tests covering: depth cap (admit-to-cap-then-hold, no infinite drip, break bounds exists() probes); rate cap (allow-then-deny break, per-agent short-circuits fleet, one real in-process-fallback run); total clamp (truncate-with-marker, context/options markers, non-dict context, non-serializable, boundary lengths, never-raises, no-mutate); reserved/malformed id guard; flood alert (direct create, un-guessable id, cooldown, emit-failure-safe, oversize-file skip); leader lock (election, fail-open, own-lease release, non-leader poll no-op); DB belt + count helper. Registered in tests/registry.json. #1525 _fake_db: stub count_operator_queue_pending_for_agent → 0 so the new depth gate is a no-op (a MagicMock default misfired the >= cap check). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(architecture,feature-flow): operator-queue ingestion caps (#1632) - architecture.md: Operator Queue subsystem gains the ingestion-caps paragraph (depth/rate/fleet + reserved-id guard + leader lock + exemption + DB belt + flood alert); Background-Services row notes the leader-gated sync; Redis keys (opqueue:leader, operator_queue_create:*); rate_limiter reuse-consumer list updated. - feature-flows/operating-room.md: Sync Service Ingestion-caps section + poll-cycle leader gate + Agent Protocol field limits / held semantics / reserved ids / created_at normalization + queue-flood-* side effect. - feature-flows.md: Recent Updates row + Documented Flows row. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(operator-queue): drop unused import (ruff F401) (#1632) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * config(operator-queue): document + plumb ingestion-cap env knobs (#1632) Add the 13 OPERATOR_QUEUE_* tunables to .env.example and wire them through both docker-compose.yml and docker-compose.prod.yml with ${VAR:-default} so the .env levers actually reach the backend container (the #1039 packaging discipline — an unplumbed lever is inert in prod). Defaults match the code getenv defaults exactly (depth 25, rate 60/60s, fleet 300, field caps). Mirrors the REPORT_RATE_LIMIT (#918) shape. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(operator-queue): floor leader-lease TTL at 30s + pin platform producer (#1632) _leader_ttl is refreshed once at the top of each _poll_cycle, so it must outlast one worst-case cycle (a slow _sync_agent read + write_file, ~10s) plus the inter-cycle sleep. The bare poll_interval*3 (15s at the 5s default) had ~zero headroom — a single slow-writing agent could expire the lease mid-cycle, a sibling would grab it, and leadership would flap, double-emitting the flood alert (its id carries utc_now_iso(), so on_conflict can't dedup it). Floor the TTL at 30s so one slow write can't drop leadership; the *3 rule still governs at larger poll intervals. Also pin the concrete platform producer: validation_service._notify_operator_on_failure was converted to a direct db.create_operator_queue_item (bypasses the agent-file sync caps, reserved `val_` id an agent can't pre-create, best-effort) — the new tests assert the direct create + reserved id + create-failure swallowing. Docs (architecture.md, operating-room.md) and the test registry updated for both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(operator-queue): first flood alert suppressed on fresh-boot uptime + test isolation (#1632) Two root causes surfaced by the CI backend-unit-test regression diff (10 new HEAD-only failures in test_1632, order-dependent across pytest-randomly seeds; not reproducible locally because macOS vs Linux collection order differs under the same seed). 1. PRODUCTION BUG — flood-alert cooldown suppressed the FIRST alert on a freshly-booted process. `_maybe_emit_flood_alert` defaulted "never alerted" to `0.0` and checked `now - last < COOLDOWN`. `time.monotonic()` is seconds from an arbitrary reference and can be < COOLDOWN (300s) right after boot, so `now - 0.0 < 300` was true and the first-ever alert for every agent was dropped for the first 5 minutes of backend uptime. Fixed with a `None` sentinel so the first alert always emits; the cooldown still gates repeats. (This is why the flood tests failed only when they happened to run within the first 300s of the CI runner's uptime — a real defect, not just test flake.) 2. TEST ISOLATION — the TestDbBelt engine stub used string-path `monkeypatch.setattr("db.operator_queue.get_engine", ...)`. Under a full-suite ordering where a prior test left a stand-in in sys.modules that re-exports the real class, the string patch lands on the stand-in while the real method reads the original module dict (module-identity gotcha) → the real engine runs. New `_patch_engine` helper patches the method's __globals__ via monkeypatch.setitem, immune to the swap. Verified against a simulated stand-in. Added a regression test pinning the low-monotonic case. Operator-queue suite green across all 3 CI seeds; ruff + sys.modules lint clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
vybe
pushed a commit
that referenced
this pull request
Jul 19, 2026
#1296) (#1694) * docs(requirements): agent self-reminders §10.14 (#1296) Requirements-first per Rule of Engagement #1: document the new agent self-reminders capability (durable one-shot deferred self-trigger) before implementing it — AC 1-7, storage fork, scheduler fire home, at-least-once delivery, autonomy hold, and retention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(db): agent_reminders table — 5-track migration (#1296) Durable one-shot deferred self-trigger storage. Byte-consistent DDL across all five coordinated edits (Invariant #3): - db/schema.py: TABLES entry + two indexes (agent + partial active) - db/migrations.py: _migrate_agent_reminders_table + MIGRATIONS tail - migrations/versions/0028_agent_reminders (off head 0027) - db/tables.py: SQLAlchemy Core Table (byte-matched to DDL) - db/agent_cleanup.py: AGENT_REFS CASCADE (rename cascade + purge + soft-delete filter; source_agent_name left unregistered per the agent_loops precedent) schema-parity, alembic-parity, and cleanup-parity all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(executions): wire triggered_by="reminder" into all three trigger constants (#1296) A new triggered_by value must hit all three sites or reminder executions silently vanish from the Overview chart, fail to filter, or misclassify their failure alert: - _TRIGGER_BUCKETS + _BUCKET_ORDER (db/schedules.py) — first-class "Reminders" bucket (mirrors the #1150 loops precedent) - _AUTONOMOUS_TRIGGERS (task_execution_service.py) — a reminder fires unwatched, so a FAILED reminder earns an operator alert - _VALID_TRIGGERS (routers/executions.py) — the fleet Executions ?triggered_by= filter accepts it Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(reminders): backend create/list/cancel surface (#1296) Three-layer backend for agent self-reminders (Invariants #1/#2/#8/#14/#18): - models.py: ReminderCreate (fire_at XOR delay_seconds validator) + Reminder/ReminderSummary + env-tunable abuse-bound constants - db/reminders.py: RemindersOperations (SQLAlchemy Core, tenant-scoped by-id ops, CAS cancel, pending/daily counts, retention prune+count) - database.py: facade delegation - services/reminder_service.py: thin create — resolved-window bound, timeout clamp to agent cap (#929 parity), pending + durable daily caps, provenance, relative→absolute, insert - services/idempotency_service.py: derive_reminder_key over RAW input - routers/reminders.py: self-only gate (current_user.agent_name==name) + _reject_connector_principal; create-idempotency (header wins else raw-input key; terminal rows excluded from replay; fail() only pre-insert); tenant-scoped cancel (200/409/404); main.py registration Self-only auth mirrors reports; connector keys stay off the auth-entry allowlist AND are explicitly rejected. Verified against a real migrated SQLite DB; models-centralized + enumeration-uniformity green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(scheduler): arm/fire/reconcile agent self-reminders (#1296) The standalone scheduler (single-instance) owns the reminder fire path — a near-clone of the RETRY-001 one-shot DateTrigger machinery: - models.py: Reminder dataclass (naive-UTC fire_at/firing_at) - database.py: committed CAS methods (claim_reminder_firing pending→ firing, mark_fired/failed, release_to_pending, set_execution) + get_active_reminders (pending∪firing JOIN agent_ownership, filters deleted_at + autonomy_enabled=1) + _row_to_reminder via parse_scheduler_ts - service.py: _schedule_reminder_job (DateTrigger), _execute_reminder (at-least-once: CAS claim → create real execution row (__manual__, triggered_by=reminder) → dispatch via _call_backend_execute_task → timeout=assume-dispatched no-force-FAILED / clean-failure=status- guarded FAILED + bounded retry), _reconcile_reminders (fail-open, arms pending + reclaims stale firing) wired into initialize() (own try after _recover_pending_retries), _sync_schedules() (own try, NOT under the cron try — Codex C5), and reload_schedules() (Codex C6) - config.py: MAX_REMINDER_FIRE_ATTEMPTS (default 3) All 207 existing scheduler tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(mcp): set_reminder/list_reminders/cancel_reminder tools (#1296) Third surface (Invariant #13) mirroring loops.ts 1:1: - tools/reminders.ts: createReminderTools → {setReminder, listReminders, cancelReminder}; reuses the inline getClient + resolveAgentName self-scoping (an agent-scoped key defaults to its bound name); Zod carries the abuse bounds (message.max(4000), delay 60..2592000, fire_at ISO string). Idempotency is enforced server-side over the raw input, so a naive retry dedupes without a client-supplied key. - client.ts: setReminder/listReminders/cancelReminder thin request wrappers - server.ts: import + register in the tools array No agent-server mirror (backend-only scheduling primitive). tsc build clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(retention): agent_reminders retention sweep + #1644 guard wiring (#1296) A durable status table needs a retention story (the AGENT_REFS CASCADE only cleans on agent delete; terminal rows on a LIVE agent accumulate). Decision: wire the FULL #1644 blast-radius guard (lightweight, mirrors agent_reports exactly, honors #1638/#1644 discipline): - settings_service.py: agent_reminders_retention_days in OPS_SETTINGS_ DEFAULTS (90, wide/safe), OPS_SETTINGS_DESCRIPTIONS, and RETENTION_OPS_KEYS (surfaced at GET /api/settings/retention, protected from /ops/reset, logged at boot). NOT a community-floor key. - cleanup_service.py: _sweep_agent_reminders_retention — DELETE terminal (fired/cancelled/failed) rows past the window (pending/firing never deleted), chunked, gated via _guard_allows + _after_guarded_prune (default MAX_ROWS_PER_SWEEP floor); CleanupReport.agent_reminders_pruned. retention-guard suite green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reminders): backend + scheduler coverage (#1296) Backend (tests/unit/test_1296_reminders.py, 33 tests): migration + tables.py live-select accessor guard (the guard schema-parity misses), the three trigger constants (parametrized), ReminderCreate XOR, derive_reminder_key raw-input stability, db round-trip + tenant-scope + cancel CAS states, retention (terminal pruned / pending+firing kept), reminder_service bounds (min/max window, timeout>cap, pending 429, daily 429), and router self-gate ATTACK (sibling 403, connector 403), tenant-scope 404, idempotency edges (dup replay, in-flight 409, cancel-then-recreate-fresh), cancel outcomes, list default-pending. Scheduler (tests/scheduler_tests/test_1296_reminders.py, 17 tests): committed single-fire CAS + multi-connection contention (exactly one wins, Codex C8), _execute_reminder outcomes (claim-loss / success / timeout=assume-dispatched-no-force-FAILED / clean-failure-retry / bounded-failed), _reconcile (arm-once, past-due, stale-firing reclaim, Z-suffix no-raise, missing-table no-op, soft-delete/autonomy-off filtered), reload path (Codex C6). Scheduler suite 224 green; backend reminder+parity+guard sweep 124 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(architecture): agent self-reminders subsystem (#1296) - New 'Agent Self-Reminders (#1296)' subsystem section (sibling to Sequential Agent Loops): storage fork, scheduler fire home, at-least-once delivery, self-only auth + abuse bounds, autonomy hold + AGENT_REFS cascade + retention - agent_reminders DB schema block (dual-track migration, CASCADE) - router/service catalog entries (reminders.py, reminder_service.py) - MCP tools table row (reminders.ts × 3) - Scheduler Service + Cleanup Service background-service rows updated - API Endpoints section for the 3 reminder endpoints Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(feature-flows): agent-self-reminders flow doc + index (#1296) New feature-flow doc mirroring run-agent-loop.md: the two design forks, MCP/backend/scheduler layers, single-fire + at-least-once delivery semantics, the 5-track migration, retention, and the three trigger constants. Adds a Recent Updates row + a Core Agent Features category row to the index (per the always-add-an-index-row rule). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(config): wire agent-reminder tunables into compose + .env.example (#1296) The six REMINDER_*/MAX_*REMINDER* caps are backend os.getenv() levers with working code defaults, so the feature ships functional — but without compose wiring the .env levers are inert on deploy (the #1056/#1039 packaging class), mirroring how REPORT_RATE_LIMIT (#918) is wired into all three files. Adds the block to .env.example, docker-compose.yml, and docker-compose.prod.yml backend.environment: with defaults matching models.py/routers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: resolve .env.example conflict left in the previous merge commit (keep both #1296 REMINDER_* and #1632 OPERATOR_QUEUE_* blocks) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
dolho
added a commit
that referenced
this pull request
Jul 20, 2026
…ent#180)
An exposed agent's card advertises every `template.yaml capabilities[]` tag, and
the well-known discovery route is unauthenticated by design — so today that full
capability list is world-readable for any exposed agent. This lets an operator
choose what the outside is told.
**A disclosure control, and the code says so.** The card's `skills[]` is
advertisement: inbound `message/send` dispatches free-form text via
`execute_task(triggered_by="a2a")`, with no per-skill routing. Filtering changes
what an orchestrator SEES, never what it may ASK for. That's stated in the
requirement, the seam docstring and the filter itself, because a filter
operators mistake for an invocation gate is a control that looks like security
and isn't. A real boundary (constraining a2a-triggered runs via
allowed_tools/guardrails) is separate work with its own threat model.
Extends the existing ent#157 seam rather than adding a module — same shape as
the inbound allow-list, second provider:
provider.exposed_skills(agent_name) -> Optional[List[str]]
- No provider (OSS) → identity function; the card is byte-identical to before,
by construction. The enterprise module owns the config, storage and UI.
- `None` = no opinion = advertise all: the unconfigured default, so exposure
(already opt-in, default OFF) keeps every existing card unchanged on upgrade.
- `[]` ≠ `None`: an explicit "advertise nothing".
- Stale ids are inert — the selection only subtracts; `template.yaml` stays the
source of truth for what exists.
- Fail-open on provider error (advertise all + WARNING): consistent with the
seam's availability bias and the advertise-all default. Honest only *because*
this isn't a security boundary — failing closed would silently empty a card
and break discovery invisibly.
Both card surfaces (public well-known + authenticated per-agent) go through one
router helper, so they can't disagree and a future third surface gets the filter
by default rather than by remembering. `generate_a2a_card` stays pure — the
provider lookup lives in the helper.
Writing the tests found a real gap: a provider returning a str (a defect) would
iterate into single characters, match no id, and silently empty the card —
fail-CLOSED, the opposite of the contract, and invisible. Malformed returns now
take the same fail-open path as a raised error.
Requirements §32.4 written before the code (CLAUDE.md rule #1); public docs
describe the generic seam only, per the standing enterprise-docs rule.
Related to trinity-enterprise#180
vybe
pushed a commit
that referenced
this pull request
Jul 24, 2026
…nterprise#124) On a genuinely fresh install, auto-deploy a bundled starter fleet once, right after first-time setup — the multi-agent generalization of the Cornelius seeder (ent#107), driven through the resilient deploy path (ent#125). - Extract the deploy orchestration verbatim from routers/systems.py into services/system_service.py::deploy_manifest (Invariant #1; router is a thin HTTP wrapper). Default create_agent_fn lazily resolves the routers/agents ws-broadcasting facade so agent_created broadcasts are preserved on every deploy path. - New services/system_seed_service.py: ensure_first_run_seeded() resolves a PERSISTED first-run verdict (first_run_fresh — computed once BEFORE Cornelius provisions; cornelius_seeded forces not-fresh) and runs both seeders under that one decision, closing the cross-pass count-poisoning bug. Durable default_system_seeded flag + seed-info JSON, SETNX lock, reserved-name existence backstop (the deploy path suffixes collisions instead of 409ing), flag policy deployed/partial=set failed/error=retry, operator-queue alerts (system-seed-* ids, #1632 reserved prefix) on partial/failed/error/unreadable-override/interrupted-converge. - cornelius_agent_service.ensure_seeded gains an optional precomputed fresh verdict (legacy count when None). - Bundled config/manifests/default-system.yaml: the in-tree acme trio (local:scout/sage/scribe) as the coherent team its content assumes — no schedules, no prompt. TRINITY_DEFAULT_SYSTEM_MANIFEST env override / disable sentinels; baked into the image AND dir-mounted in both compose files (the dev src mount shadows image COPYs). e2e stack pins the disabled sentinel to keep its zero-agent baseline. Refs trinity-enterprise#124 (closed manually at release — closing keywords don't cross repos). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5 tasks
AndriiPasternak31
added a commit
that referenced
this pull request
Jul 26, 2026
* chore(.claude): bump dev-methodology submodule — Product Quality Bar
Points .claude at trinity-dev 57c8b5c: adds a canonical Product Quality Bar
section to DEVELOPMENT_WORKFLOW.md (six adoption/ease-of-use principles) and
hooks it into the dev pipeline — /create-issue (acceptance criteria),
/autoplan (scope calibration → taste decisions), /implement (build-time
checklist), and /review (new 4.15 catch-in-diff check).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(announcements): v0.8.0 announcement record (#1548)
* chore(.claude): bump dev-methodology submodule — Product Quality Bar
Points .claude at trinity-dev 57c8b5c: adds a canonical Product Quality Bar
section to DEVELOPMENT_WORKFLOW.md (six adoption/ease-of-use principles) and
hooks it into the dev pipeline — /create-issue (acceptance criteria),
/autoplan (scope calibration → taste decisions), /implement (build-time
checklist), and /review (new 4.15 catch-in-diff check).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(announcements): v0.8.0 release announcement record (all channels)
---------
Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(avatar): prevent thinking-token truncation dropping the color-scheme block
Avatar refinement runs each identity prompt through GEMINI_TEXT_MODEL to append
a fixed technical block (background #1a1f2e/#111827, indigo rim #6366f1,
head-and-shoulders framing, 85mm lens, 5600K key) — the sole source of avatar
color scheme and cross-generation consistency.
The refinement call capped maxOutputTokens at 512. Since #1130 the default model
is gemini-3.5-flash, a *thinking* model whose reasoning phase draws from the
output budget. Reasoning consumed ~491 of 512 tokens, truncating the refined
prompt (finishReason=MAX_TOKENS) to a ~90-char fragment that dropped the entire
technical block. Avatars were then generated from a bare subject description —
wildly inconsistent and off-palette.
Fix in _call_gemini_text:
- Disable thinking (thinkingConfig.thinkingBudget=0) — refinement is a
deterministic rewrite that needs no chain-of-thought.
- Raise maxOutputTokens 512 -> 4096 as headroom even when thinking stays on.
- Retry once WITHOUT thinkingConfig on HTTP 400: a thinking-mandatory model
(e.g. gemini-2.5-pro) rejects budget=0, and refine_prompt silently falls back
to the raw prompt on error, so an unhandled 400 would re-break avatars.
Verified end-to-end against the live model: all 6 technical-block tokens now
survive refinement (was 0/6). Adds 3 regression tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(channels): count inbound client messages on the Sharing-tab roster (#1533) (#1554)
* fix(channels): count inbound client messages on the Sharing-tab roster (#1533)
The roster showed `message_count = 0` for every external client, `last_active`
was frozen at `/login` time, and clients who never ran `/login` never appeared
at all. All three share one cause: `get_or_create_chat_link` and
`increment_message_count` (Telegram + WhatsApp) had zero callers outside the
`database.py` facade. The shared inbound path writes `public_chat_sessions`, a
different table, so `*_chat_links` rows were only ever created by
`set_*_verified_email` — the `/login` flow.
Note: the issue's Context paragraph states that `last_active` "is touched on
inbound traffic via `get_or_create_chat_link`". It is not — that function was
dead, and returns early for an existing row without writing anything.
Revive the write path behind a default-no-op `ChannelAdapter.record_inbound_activity`
hook, called once per delivered DM from `ChannelMessageRouter._handle_message_inner`
at step 5c: after the access gate, so an unauthenticated stranger who messages the
bot cannot create unbounded chat-link rows, and skipped for groups, since a chat
link is keyed by (binding, user) and counting group traffic would list members who
never DM'd the agent. Telegram and WhatsApp override it; Slack and VoIP inherit
the no-op (Invariant #9). The call is best-effort — a counter write never blocks
message processing.
Replace `get_or_create` + `increment` with one atomic `INSERT … ON CONFLICT DO
UPDATE` (`record_inbound`), which removes a cross-worker SELECT-then-INSERT race
under `--workers 2`, halves the writes per message, and refreshes a stale display
name via `COALESCE(excluded, existing)`. Since `increment_message_count` was
`last_active`'s only live writer, one call fixes the count and the timestamp
together. Delete the now-dead methods, their facade delegations, and the
`_row_to_chat_link` helper they alone called.
Historical counts are not backfilled; the roster's "Messages" header says so.
Tests: dual-backend (SQLite + PostgreSQL) roster read-back 0->1->2, `last_active`
advance, username backfill, and `/login` interplay through the real `db_backend`
harness; the real `_handle_message_inner` for the DM, group, access-denied and
counter-failure paths; and the adapter override bodies executed for real —
mutation-verified, a typo'd metadata key turns the suite red. A facade-delegation
guard covers the wholesale-mock blindness recorded in docs/memory/learnings.md.
Verified against a live instance: real Telegram webhook payloads through the real
transport drive the roster 0->1->2; a group message reaches the router and is not
counted; an access-denied message creates no row.
Follow-up #1552 records the read-time-derivation reframe (deriving the roster from
public_chat_messages) that this tactical fix deliberately defers.
Fixes #1533
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(channels): use an example.com placeholder email in the #1533 access-denied case
The public-repo rule in CLAUDE.md calls for `user@example.com`-style
placeholders. `a@b.com` was copied from the neighbouring access-gate test and
was also inconsistent with the rest of this file, which already uses
`alice@example.com`. No behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(ci): auto-trigger frontend-e2e (nightly + frontend paths), keep ui opt-in (#1526) (#1551)
The frontend-e2e Playwright suite ran ONLY on `ui`-labeled PRs, so most PRs never
triggered it and the suite rotted silently — specs broke against structural/UX
changes and sat red on `dev` for weeks (four documented instances: #1134, #1508,
#1378). The gate itself was the root cause.
Add automatic triggers so a red suite is always visible:
- Nightly `schedule` (07:00 UTC) on `dev` — the durable signal. A red nightly
opens/updates a single tracking issue (new `report` job, label
`frontend-e2e-nightly`), à la #1185; a green nightly closes it.
- Auto-run on any PR that touches `src/frontend/**` (dependency-free `changes`
job via `gh pr diff`), so frontend authors see their own breakage.
- The `ui` label still works as a manual opt-in on non-frontend PRs and to force a
run (preserved for the heavier @visual/@interactive tiers, #596).
- `workflow_dispatch` for on-demand runs.
DECISION (AC): advisory, NOT a required merge gate. The recurring failure class is
modal/overlay flake on a fresh zero-user-agent stack, so a hard gate like #715's
unit gate needs a flake budget first (#596). Recorded in the workflow header.
Per-job least-privilege permissions; concurrency keyed per-PR (cancel) vs per-ref
(schedule, no cancel). Only @smoke runs in CI, unchanged.
Related to #1526
* feat(mcp): make per-agent MCP connector OSS-core (trinity-enterprise#118 Part A) (#1555)
* feat(mcp): make per-agent MCP connector OSS-core (trinity-enterprise#118 Part A)
Relocate the per-agent MCP connector (ent#46/#55/#51, shipped v0.8.0 entitlement-
gated as `mcp_connector`) from the private enterprise submodule into OSS core, and
drop the entitlement gate front and back. Decision (Eugene, 2026-07-09): sharing
agents via individual MCP connectors is a platform-adoption surface, not a paid
module.
Backend (router → service → db, Invariants #1/#2/#14):
- routers/connector.py — /api/agents/{name}/connector* (config, mint/regenerate/
revoke key, playbooks), mounted unconditionally in main.py; no requires_entitlement.
- services/connector_service.py — snippet builder + playbook resolution.
- db/connector.py (ConnectorOperations) — config CRUD + scoped-key mint/revoke
into mcp_api_keys (scope='connector'); facade delegators on database.py.
- models.py — ConnectorConfigUpdate/Status/KeySecret/Playbook/ClientSnippet.
Schema: enterprise_connectors table re-homed onto OSS dual-track (db/tables.py,
db/schema.py, db/migrations.py:enterprise_connectors_table + Alembic
0015_enterprise_connectors). Name kept so existing enterprise installs adopt their
data with zero migration (CREATE TABLE IF NOT EXISTS, no duplicate-table drift).
Delete/rename cascade via an enterprise_connectors AGENT_REF.
Frontend: ConnectorChannelPanel un-gated in SharingPanel.vue (dropped the
isEntitled('mcp_connector') v-if + the now-unused enterprise store wiring).
The MCP proxy tools (connector.ts), the connector-scope auth fence
(dependencies.py), and ExposedToolsPanel.vue were already OSS and edition-agnostic.
`mcp_connector` is removed from the entitlement registry by the paired enterprise
PR (deletes register_module).
Docs: requirements mcp.md §7.5 + feature-flows/mcp-connector.md + architecture/index.
Tests: tests/unit/test_118_mcp_connector_oss.py (11) — service helpers, config CRUD,
key mint/regenerate/revoke, scope='connector' validate contract.
Part B (email-auth onboarding, #848) deferred pending design sign-off.
Related to trinity-enterprise#118
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(submodule): bump enterprise to post-#121 so the connector isn't double-mounted (#118)
#1555 makes the per-agent MCP connector OSS-core. The enterprise submodule was
pinned at 630cca9e, which still ships backend/mcp_connector/ and registers it via
register_enterprise() — so an enterprise build would double-mount the connector
router alongside the new OSS-core one. Bump the pin to f5d69be6 (enterprise main
post trinity-enterprise#121), where the private module is removed.
This forward-integrates two already-on-enterprise-main commits into the pin:
- trinity-enterprise#121 — removes backend/mcp_connector/ (the intended pair)
- trinity-enterprise#120 — ENTERPRISE_LOCAL_DEV docs (docs only)
- trinity-enterprise#106 — client-portal umbrella (already on enterprise main)
OSS-only CI is unaffected (submodule is update=none / "boots without enterprise").
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
* docs(enterprise): explain the private feature catalog + runtime verification
ENTERPRISE.md documents the open-core seam mechanism (correct) but never
said WHERE the feature-level OSS/enterprise split lives or WHY it isn't
here. Add a short "Why there's no feature catalog here" note: the standing
rule trinity-enterprise#45 (enforced by enterprise-docs-guard.yml) keeps
the paid-feature catalog private, and entitled customers find it in the
private enterprise repo. Point readers at the runtime source of truth for
"what's enabled on this instance" (GET /api/version + feature-flags).
Mechanism-only, no named features — enterprise-docs-guard grep verified green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(announcements): webinar video announcement record 2026-07-10
Record of the "sovereign, isolated AI agents per client" webinar
video announcement (https://youtu.be/8w98dA6hDew) sent via /announce
to Discord, Slack, Telegram, Twitter/X (trinity + default), and
GitHub Discussions.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(deps-dev): bump @types/node (#1531)
Bumps the patch-and-minor group in /src/mcp-server with 1 update: [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node).
Updates `@types/node` from 26.1.0 to 26.1.1
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node)
---
updated-dependencies:
- dependency-name: "@types/node"
dependency-version: 26.1.1
dependency-type: direct:development
update-type: version-update:semver-patch
dependency-group: patch-and-minor
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* fix(reliability): clear per-agent Redis runtime state across the agent lifecycle (#1560) (#1568)
* fix(reliability): clear per-agent Redis runtime state across the agent lifecycle (#1560)
`agent:circuit:{name}` (transport breaker) is keyed by agent NAME, not container
identity, and carries no TTL. Nothing in the lifecycle cleared it, so a container
replaced under the same name inherited its predecessor's `dormant` verdict and
fast-failed every execution with "Agent circuit breaker open — agent is
unhealthy" — without the backend ever contacting the agent.
Adds `services/agent_runtime_state.py` as the single enumeration point for every
name-keyed per-agent Redis keyspace (the Redis-side twin of the `AGENT_REFS`
registry in `db/agent_cleanup.py`), with two entry points whose blast radii
differ by whether a container is running:
clear_agent_breakers heartbeat + transport circuit + dispatch breaker
(safe on a live container)
clear_agent_runtime_state the above + execution slots
(teardown paths only — force_clear_slots would drop
capacity accounting for an in-flight #1083 execution)
Wired into six lifecycle points. The issue's acceptance criteria named delete,
rename and create; none is the reachable path:
* create is unreachable — `is_agent_name_reserved` sees soft-deleted rows and
409s, locking the name for the whole retention window;
* the name only unlocks at the retention purge, which cleared no Redis state;
* the reachable path is `start_agent_internal`, whose `needs_recreation` branch
replaces the container on any config drift (subscription switch, resource
change, auth-token rotation), so one fleet-wide rotation resurrects every
stale verdict.
So start/recreate, purge, and the `trinity-system` bootstrap were added beyond the
written criteria. The start-path clear runs before the recreate, since
`containers_run(detach=True)` brings the replacement up, and is guarded on
`needs_recreation or not was_already_running` so a no-op start cannot reset a
breaker protecting a wedged agent.
Tests: a bidirectional parity guard fails CI when a new `agent:*` keyspace ships
unregistered; wiring tests pin all six call sites and that lifecycle.py never
clears slots; an integration test exercises the real Lua (fakeredis has no
EVALSHA) plus an opt-in leg driving the full recreate path over HTTP. Both guards
are mutation-tested.
Complementary to #1561, which removes the source of breaker poisoning; this
removes the inheritance. Both are needed.
Fixes #1560
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(1560): stop bare sys.modules mutations in the new test files (#762)
The `lint (sys.modules pollution check)` CI gate flagged 7 new violations.
They were real, not a lint technicality: the integration test replaced
`sys.modules["services"]` with a bare stub module and never restored it,
so every later test in the same session that imported the real `services`
package would have seen the stub — the exact cross-file pollution class
#762 introduced this gate for.
- Unit files: drop the `sys.modules[mod_name] = module` registration in
`_load` entirely. `agent_runtime_state.py` is a stdlib-only leaf with no
`@dataclass` needing `sys.modules[cls.__module__]` to resolve annotations,
so the registration bought nothing and risked pollution. Per-test stubs
already go through `monkeypatch.setitem`, which restores itself.
- Integration file: keep the bindings (they are what makes the production
lazy imports resolve to the real modules against real Redis — monkeypatch
cannot reach an import performed inside the function under test) and add
the sanctioned `_STUBBED_MODULE_NAMES` + autouse `_restore_sys_modules`
snapshot/restore pair, per tests/unit/test_telegram_webhook_backfill.py.
Verified: `python tests/lint_sys_modules.py` clean; running the integration
file followed by a suite that imports the real `services` package passes.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(ui): prettify Agent Permissions Matrix — bigger labels, no grid jump (#1563)
The fleet permissions matrix was hard to read (text-xs, 32px cells) and
the grid jumped on every grant/revoke: the toast rendered inline between
the toolbar and the grid, so it pushed the grid down on appear and
snapped it back on the 3.5s auto-dismiss.
- Bump base font (text-xs → text-sm), cells (w-8 h-8 → w-11 h-11),
checkmark (text-base), row/column agent labels (font-medium), corner
hints (10px → text-xs), and column-label headroom (8rem → 10rem).
- Move the toast into a reserved min-h-[2.5rem] slot so appear/dismiss
no longer reflows the grid.
Presentation-only — grant/revoke logic and endpoints unchanged.
Grant/revoke verified end-to-end via the backend API.
Related to #1562
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(sync-health): exclude soft-deleted agents from list_git_enabled_agents (#1561) (#1565)
`list_git_enabled_agents()` selected from agent_git_config on sync_enabled
alone, without joining agent_ownership or excluding deleted_at. Soft delete
keeps the git_config row, so the 60s SyncHealthService poller kept issuing
HTTP calls to removed containers forever — each httpx.ConnectError poisoned
the transport circuit breaker, eventually driving it DORMANT and emitting a
bogus circuit_breaker_dormant operator-queue alert for a nonexistent agent.
GET /api/fleet/sync-audit (same accessor) also listed dead agents.
- Join agent_ownership + filter deleted_at IS NULL, mirroring the #834
hardening on list_all_enabled_schedules(). Fixes both consumers at once.
- git_service.get_git_status: bare print() → logger.warning (structured).
- Regression tests: soft-deleted agent is excluded from the accessor and
never polled (zero HTTP, no sync_state row, no operator-queue entry).
Audit sweep (per AC): list_git_enabled_agents was the sole background-loop
accessor missing the deleted_at filter. Other per-agent HTTP pollers
(monitoring, operator-queue) enumerate from Docker, so removed containers are
inherently excluded; scheduler already filters (#834); capacity drain is
DB-only (no agent HTTP). Companion breaker-inheritance defect tracked in #1560.
Related to #1561
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(canary): execution-row integrity invariants E-03, G-03, E-04, G-04 (Phase 4) (#1497)
* test(canary): unblock canary_invariants fixture broken by #1472 merge
Two pre-existing breakages landed via #1472 (367a5e12, merged today) in
root-level tests/test_canary_invariants.py — a file the CI unit job
(tests/unit/ only) does not run, so they merged undetected:
1. Duplicate `CREATE TABLE agent_schedules` in the `canary_db` fixture DDL
— the E-06 work added a second definition next to the pre-existing one,
so `executescript` raised "table agent_schedules already exists" and
every fixture-dependent canary test errored. Removed the older, unused
block (its `message`/`owner_id` columns have no consumer; `_add_schedule`
and the E-06/L-03 collectors use only the kept block's columns).
2. `test_run_invariants_all` expected registry set omitted "E-06" (added to
the invariants registry by the same PR). Added E-06 to the expected set
and asserted it is green on a clean platform.
Restores a green baseline (92 passed) so the #1450 B-01 work can be verified.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(canary): B-01 queue-status coherence — backend-consistent Side B + confirm-re-read (#1450)
B-01 compared two reads that were neither temporally atomic nor
backend-consistent (both latent while the canary is default-OFF and SQLite
makes the two reads hit one file):
(a) Temporal non-atomicity — a concurrent enqueue/backlog-drain landing
between the two reads produced a transient count mismatch → spurious
critical + green→red Slack alert.
(b) Backend divergence (#300/#1093) — Side A (`db.get_queued_count`) honors
`get_engine()`/DATABASE_URL; Side B read raw sqlite3 at DB_PATH. On
Postgres those are two different databases, so B-01 compared Postgres
truth to a stale/absent SQLite file (fatal under the Postgres direction +
SQLite EOL, #1278).
Production-side residue of #1446 (PR #1452), which fixed only the test-harness
`sys.modules` leak and deferred these two gaps.
Fix (localized to B-01):
- New `_collect_queued_ids_via_engine` reads B-01's Side B through the SAME
`get_engine()` seam as the accessor, on a dedicated `queued_ids_via_engine`
snapshot field. Independent code path (SELECT id/literal 'queued' vs
COUNT(*)/the QUEUED enum) — shares a database, not a code path, so a
cache/status-filter regression still surfaces (non-tautology, AC #3). No
cache / second count of the queue (AC #4).
- The collector performs one confirm-re-read on a mismatch: a transient race
self-heals; a persistent drift survives and fires (AC #2). An engine-read or
unconfirmable confirm degrades to a B-01 skip — it never compares an engine
count against the raw-sqlite id-set (the blocker the reviews surfaced).
- `queued_exec_ids` (raw sqlite) stays untouched for B-02/E-02, so blast
radius is B-01-only and the sibling #1077 merge stays clean.
The running-side / known-agents reads remain raw-sqlite (half-migrated
collector); the collector-wide migration + a dark-canary tripwire are a filed
follow-up.
Tests (test_canary_invariants.py): retarget the synthetic B-01 tests at
`queued_ids_via_engine`; add the AC #5 regression net — a diverged-backend
proxy (raw ≠ engine temp files) that false-fires pre-fix and is green
post-fix, a transient-race confirm-absorb + persistent-drift-still-fires pair,
and an engine-read-failure→skip test. New split fixtures (`canary_db_split` /
`reload_canary_split`) model the diverged backend without a live PG. Convert
the file to the sanctioned `_STUBBED_MODULE_NAMES` + `_restore_sys_modules`
escape hatch so its import-time stubs no longer leak (the #1446 mechanism) and
the reimport `del`s are lint-clean.
Docs: architecture.md Canary B-01 row updated for the engine-backed read path
+ confirm-re-read.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(feature-flows): add #1450 canary B-01 Recent Updates row
Sync-feature-flows: canary-internal change, no dedicated flow doc
(architecture.md is canonical); append one Recent Updates row.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(canary): fix duplicate agent_schedules DDL + reconcile E-06 in registry test
The canary_db fixture had two `CREATE TABLE agent_schedules` statements in one
executescript (a #1472 merge artifact), so it raised `table already exists` and
reddened the whole file. Merge them into one definition carrying every column
the canary reads (next_run_at/enabled/deleted_at + agent_name). Add
duration_ms/queued_at/backlog_metadata to schedule_executions and extend
_add_execution for the #1077 E-03/E-04 collector work. Reconcile E-06 (already
registered) into test_run_invariants_all's expected key-set.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(canary): terminal-row collector for E-03/G-03 (#1077)
Add _collect_terminal_rows(window_seconds) + Snapshot.terminal_rows. Windowed on
started_at (not completed_at, so E-03 can see NULL-completed_at rows), scoped to
success/failed/cancelled via a local _E03_TERMINAL_STATUSES subset that excludes
skipped (which legitimately has no completed_at/duration_ms). PRAGMA guard skips
the source entirely when completed_at/duration_ms are absent (column-absent !=
value-NULL) rather than false-firing. Window = max per-agent timeout + 300s;
bounded ORDER BY started_at DESC LIMIT 5000 with a logged sampled flag (no
(status,started_at) index over 90-day retention; tripwire, not backfill audit).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(canary): register E-03 (completed_at populated) + G-03 (clock sanity) (#1077)
E-03 (A/major): terminal rows must have completed_at NOT NULL. Predicate is
completed_at-only — the catalog's + duration_ms clause false-fires on healthy
queue-terminated rows (cancel/fail/expire set completed_at but never
duration_ms). G-03 (A/minor): started_at <= completed_at with a ~1s cross-worker
clock-skew tolerance, UTC-aware parsing (E-06 _to_utc shape) so a #1474 mixed
naive/Z pair compares without raising. Both are leading-edge tripwires over the
shared terminal-row collector and catch all producers incl. the standalone
scheduler's raw-SQL writers a unit test never exercises.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(canary): E-03/G-03 synthetic + collector + end-to-end coverage (#1077)
Per-invariant synthetic tests (holds-clean, fires-on-violation), collector tests
(started_at window in/out, NULL-completed_at still collected, skipped-status
excluded, column-absent DDL -> unavailable, LIMIT cap + sampled flag), and
end-to-end collect_snapshot tests through the real collector: the C1
cancelled-from-queue holds-clean guard (completed_at set, duration_ms NULL ->
zero E-03), half-written fires E-03, bad-clock fires G-03, sub-second skew does
not, and the #1474 naive-vs-Z compare-without-raising case.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(canary): document E-03/G-03 Phase 4 invariants (#1077)
architecture.md canary table gains E-03/G-03 rows + Phase 4 lookup-key line;
requirements/infrastructure.md §31 gains a Phase 4 bullet (and reconciles the
stale 'Phase 2 deferred' note now that #882/#1472 shipped);
orchestration-invariant-catalog.md annotates E-03/G-03 as shipped with explicit
registry-id mapping, notes the E-03 completed_at-only predicate deviation and
G-03 started_at<=completed_at reduction, and flags the catalog-id vs registry-id
E-06 drift (catalog #129 != registry #1472).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(feature-flows): index row for canary Phase 4 E-03/G-03 (#1077)
Canary is an internal invariant harness (no user-facing flow / dedicated flow
doc); follows the #1446 precedent of a Recent Updates row pointing at
architecture.md.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(canary): queued-row metadata collector for E-04/G-04 (#1077)
Capture queued_at + backlog_metadata for status='queued' rows in the
existing _collect_executions query, keyed by execution_id in a new
AgentSnapshot.queued_meta map. Both columns are PRAGMA-guarded (added by
BACKLOG-001): when either is absent on an older/minimal DDL the map is
left empty so E-04/G-04 skip those eids (older-image fail-open). Scoped
STRICTLY to queued rows — never terminal — so #1449's deferred
terminal-row backlog_metadata NULL-out cannot make E-04 false-fire.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(canary): register E-04 (queued metadata) + G-04 (no creds in metadata) (#1077)
E-04 (Tier A, major): every status='queued' row has queued_at NOT NULL
AND a non-NULL, JSON-parseable backlog_metadata — the
backlog_service.drain_next replay contract. A malformed blob raises
JSONDecodeError and stalls the FIFO. Reports only the failed-predicate
reason code + ids, never the raw metadata (may carry credentials;
violations persist to canary_violations).
G-04 (Tier A, critical): a queued row's backlog_metadata matches no
known secret prefix (sk-/ghp_/gho_/ghs_/ghu_/github_pat_/xoxb-/xoxp-/
AKIA/AIza/sk_live_), word-boundary anchored so common substrings don't
false-fire. Rides E-04's collected bytes. Reports only the matched
pattern NAME + ids, one violation per row (stops at first match) — never
the secret, surrounding bytes, or raw metadata.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(canary): E-04/G-04 synthetic + collector + end-to-end coverage (#1077)
Collector test proves queued_meta is populated for queued rows only (a
terminal row carrying backlog_metadata is excluded — #1449-safe). E-04:
holds on valid rows; fires with the right reason code on NULL queued_at,
NULL backlog_metadata, and non-JSON metadata; skips an eid absent from
queued_meta (older-image fail-open); e2e over a real temp DB. G-04:
holds on benign metadata (incl. "task-" substring that must not
false-fire); fires on github_pat / openai / slack / aws exemplars; skips
NULL metadata (E-04 owns it). Every G-04/E-04 test asserts the secret /
raw metadata bytes appear NOWHERE in the persisted violation record. The
runner test now expects E-04 + G-04 in the registered invariant set.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(canary): document E-04/G-04 Phase 4 invariants (#1077)
architecture.md lookup-key table gains E-04 + G-04 rows and the Phase 4
line now lists all four (E-04/G-04 stacked on #1450). requirements
infrastructure.md Phase 4 bullet expands to the full four-predicate set
with the credential-safety note. Catalog flips E-04/G-04 from
"gated on #1450" to SHIPPED, records the json.loads-vs-json_valid
implementation note, the queued-only scope, older-image fail-open, and
the report-reason/pattern-name-only security discipline; G-04 notes the
implemented check covers the backlog half of the title (log-line
scanning out of scope for #1077).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(feature-flows): index row for canary Phase 4 E-04/G-04 (#1077)
Also corrects the header/separator ordering left malformed by the
earlier E-03/G-03 index-row commit (a data row had slipped above the
|---| separator).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
* fix(config): forward OPERATOR_INTAKE/DO_NOT_TRACK, DISPATCH_BREAKER_ENABLED, PUBLIC_ACCESS_REQUESTS_ENABLED through compose (#1485) (#1493)
* fix(config): honor cross-tool DO_NOT_TRACK convention for operator intake (#1486)
The operator-intake kill switch only disabled the outbound POST when
DO_NOT_TRACK was one of {"1","true","True"}, so DO_NOT_TRACK=yes|on|2|TRUE
leaked despite the obvious opt-out intent — breaking the very
consoledonottrack.com convention config.py's own comment cites.
Flip to a tracking-allowed whitelist: any value not in {0,"",false}
(case/space-insensitive) disables intake. Unset -> "0" -> tracking
allowed (unchanged). Add tests/unit/test_1486_do_not_track_truthiness.py
so a revert to the exact-tuple check fails loudly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(config): forward operator-intake, dispatch-breaker & public-access levers through compose (#1485)
Same packaging-gap class as PR #1067 (VOIP vars): a lever read by
src/backend/config.py that neither compose file forwards into the backend
container, so setting it in .env silently no-ops (compose reads .env only
for ${VAR} interpolation, and neither backend service uses env_file).
Forward in both docker-compose.yml and docker-compose.prod.yml backend
environment: blocks, mirroring the existing PUBLIC_CHAT_URL idiom:
- PUBLIC_ACCESS_REQUESTS_ENABLED (#1488) — secure default false
- OPERATOR_INTAKE_ENABLED / DO_NOT_TRACK / OPERATOR_INTAKE_URL (#1486)
privacy kill switch. OPERATOR_INTAKE_URL keeps its FULL non-empty
default — a bare :- would arrive set-but-empty and shadow the code
default (#1076 set-but-empty class).
- DISPATCH_BREAKER_ENABLED (#1487) — global gate for the #526 dispatch
breaker; without it the owner-facing PUT .../circuit-breaker toggle
silently no-ops (two-tier gating needs both flags on).
.env.example: add the missing DISPATCH_BREAKER_ENABLED block (documenting
the two-tier gate) and a settable DO_NOT_TRACK=0 line. Backend-only —
src/scheduler reads none of these five vars.
Fixes #1486
Fixes #1487
Fixes #1488
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(feature-flows): note DISPATCH_BREAKER_ENABLED compose-forwarding fix (#1487/#1485)
The dispatch-circuit-breaker flow already documented the DISPATCH_BREAKER_ENABLED
env var, but before this PR that var never reached the container (the
#1039/#1067 packaging-gap class), so a reader following the doc would set it and
the owner toggle would silently no-op. Add a one-line Config-surface accuracy
note + a Recent Updates index row.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
* fix(scheduler): serialize timestamps as UTC 'Z' so non-UTC browsers show correct relative times (#1474) (#1496)
* fix(scheduler): serialize timestamps as UTC with explicit 'Z' (#1474)
The standalone scheduler wrote execution/schedule timestamps via
datetime.utcnow().isoformat() (no 'Z'), while the backend writes through
utc_now_iso() (Z-suffixed). In a non-UTC browser, JS new Date(naive) parses
the naive string as local time, shifting schedule-triggered rows by the
viewer's UTC offset. Data on disk was fine; only display was wrong.
Vendor a byte-parity mirror of the backend timestamp helpers into
src/scheduler/utils.py (same regenerate-from-backend discipline as
failure_classifier.py): utc_now_iso / to_utc_iso emit 'Z'; parse_scheduler_ts
reads tolerantly and returns naive UTC.
Write + read land together (atomic): once writes emit 'Z',
datetime.fromisoformat("...Z") returns a tz-aware value on 3.11+, and the
duration math (datetime.utcnow() - started_at) would raise aware-naive.
parse_scheduler_ts converts-then-strips so the historical naive model type is
preserved and the subtraction stays naive-naive. All 24 read-parses route
through it; every write site (started_at/completed_at/last_run_at/next_run_at/
retry_scheduled_at/validated_at + process-schedule variants) emits 'Z'.
Updates the #1472 next_run_at comments to note the mapper now returns
naive-UTC (instant preserved), so both compare branches stay correct.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(executions): normalize naive timestamps to UTC 'Z' at read boundary (#1474)
The summary/list readers returned raw dict(row) values straight from the DB,
so a scheduler-written naive started_at/completed_at serialized naive out of
Pydantic and JS new Date(naive) parsed it as local time — the reported
schedule-triggered relative-time shift. This fixes historical rows for all
consumers at the source (unit-testable, unlike the frontend layer).
Reuse the existing parse_iso_timestamp helper (assume-UTC for naive) + the
Z-emitting to_utc_iso — the same normalization the already-correct sibling
readers (_row_to_execution) apply — via a small _norm_ts shim:
- db/schedules.py: get_agent_executions_summary (TasksPanel),
get_fleet_executions (ExecutionsPanel), get_agent_schedules_summary
last_run_at (Overview/Schedules).
- db/activities.py: _row_to_activity / _mapping_to_activity
(UnifiedActivity) started_at/completed_at/created_at.
None passes through untouched. No data migration.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): parse backend timestamps as UTC in the 5 execution panels (#1474)
Defense-in-depth for the read side (covers WebSocket-pushed timestamps and any
un-audited endpoint, alongside the backend read-boundary fix). Each panel
hand-rolled `new Date(backendStr)`, which parses a naive (no-'Z') string as
*local* time. Replace only the internal parse with the shared idempotent
`parseUTC` (appends 'Z' when no tz indicator present → correct for legacy-naive,
new-'Z', and '+00:00' alike); `new Date()` "now" calls are left untouched and
per-panel display formats are unchanged.
- TasksPanel: formatRelativeTime + inline log-detail timestamp
- SchedulesPanel: formatRelativeTime, isOverdue, formatOverdue, formatDateTime
- ExecutionsPanel: timeAgo
- UnifiedActivityPanel: formatTime
- OverviewPanel: fmtDateTime
Established pattern (stores/network.js, ReplayTimeline.vue).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: record #1474 scheduler Z-suffix + read-boundary under Invariant #16
Extend Architectural Invariant #16 (the #476 ISO-Z rule) with its write-side/
read-boundary cousin: the scheduler vendors src/scheduler/utils.py (Z-suffixed
writes, naive-UTC tolerant reads); the leaking backend read boundaries normalize
via parse_iso_timestamp; the 5 panels parse via parseUTC. Notes honestly that
next_run_at stays mixed-format across writers (safe — Python-compared only) and
that main.py SchedulerStatus.last_check is out of scope.
Update the #1472 learnings entry: the scheduler DB mapper now returns naive-UTC
(convert-then-strip, instant preserved), so the "schedules store AWARE
next_run_at" premise is true of the backend writer, not the scheduler read path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(feature-flows): document #1474 scheduler Z-suffix timestamp contract
Add a "Timestamp serialization contract (#1474)" subsection to
scheduler-service.md (vendored utils.py, Z writes, naive-UTC tolerant reads,
backend read-boundary + panel normalization, next_run_at mixed-format caveat)
plus a Revision History row; add the index Recent Updates row.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(scheduler): cover _update_business_status validated_at Z-suffix (#1474)
The service-layer VALIDATE-001 write path (service.py::_update_business_status)
is the one #1474 timestamp write site outside database.py. Add sibling-path
coverage per the incomplete-fix rule: assert validated_at is stored 'Z'-suffixed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
* docs(user-docs): add Trinity FAQ — 264 grounded Q&As across 14 topic pages
One page per topic (getting started, agents, chat/sessions, credentials,
scheduling, collaboration, channels, MCP/API, operations, sharing,
deployment, security, advanced, troubleshooting) plus a generated
question index. Answers derived from user docs and verified against
code; .claude pointer picks up the generate-user-docs FAQ step.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(user-docs): clarify PAT scope — git transport vs gh CLI/REST API authentication
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(helper-mcp): standalone Trinity docs Q&A MCP server (#1459)
New src/helper-mcp/ package (@abilityai/trinity-docs-mcp): an npx-runnable
stdio MCP server exposing the public ask-trinity docs Q&A endpoint
(DOCS-QA-001) to any MCP client — no Trinity instance or API key required.
- Tools: ask_trinity (multi-turn; detects the endpoint's silent session
reset and warns when context was lost) + get_agent_requirements (agent
guide fetched live from GitHub, quick-reference fallback)
- Guards: 4k question cap, 50s abort timeout, no auto-retry,
redirect:"error", non-JSON guard, structured error text; session_id
handled as opaque string (live values exceed 2^53)
- Deps: official @modelcontextprotocol/sdk + zod only; console.error-only
logging (stdout is the JSON-RPC channel); Node >=18 launcher guard
- Tests: 26 unit (mocked fetch) + pack-and-run stdio smoke test asserting
JSON-RPC stdout purity; CI workflow helper-mcp-test.yml
- Publish: publish-helper-mcp.yml (npm provenance; documented one-time
manual first-publish bootstrap for trusted publishing)
- Corpus: sync-docs-to-vertex.yml now also indexes the agent guide;
feature flow corrected (user-docs/FAQ were already indexed) and extended
with the verified endpoint contract (no citations field, silent session
expiry)
- Docs: requirements/mcp.md entry, README + user-docs install blocks,
package README
Fixes #1459
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(agents): ephemeral ghost agents — budgeted hard-discard lifecycle + spawn provenance (trinity-enterprise#69)
Disposable "ghost" agents: created with a hard budget (max_executions and/or
TTL — expiry ALWAYS stamped, ceiling 24h default), volume-less (container
writable layer; ghosts never recreate), hard-discarded at budget with no
soft-delete/retention/name-reservation. Creation is entitlement-gated
(ephemeral_agents; fail-closed — inert until the enterprise module registers);
all lifecycle mechanics are edition-agnostic OSS primitives.
Lifecycle:
- Schema: 5 additive agent_ownership columns (is_ephemeral, budget, expiry,
spawned_by_agent/key_id); dual-track migration (SQLite + Alembic 0016)
- Creation gates (crud.py): entitlement 403 → ephemeral-caller refusal
(chain-spawn kill) → per-parent spawn rate limit → TTL ceiling 400 →
server-suffixed name (hex8) → atomic per-owner Redis quota (INCR-with-cap,
NX reseed, DB fallback); ghosts skip volume/avatar/cred-injection/auto-sync,
default max_parallel_tasks=1
- Budget: gate at the TOP of CapacityManager.acquire (terminal+active >= max
or expired ⇒ EphemeralBudgetExhausted → 410 Gone / FAILED
ephemeral_exhausted; covers every admission surface); post-CAS-win
apply_result hook (backgrounded, fail-open) triggers discard at budget
- Hard discard (services/agent_service/ephemeral.py): SETNX-locked,
crash-convergent — intent marker → CAS-fail non-terminal rows
(ghost_discarded) → force-remove container → clear Redis state BEFORE purge
→ cascade purge (executions KEEP) → audit. DELETE routes ghosts here before
the container lookup (half-discarded state force-discardable)
- GC (cleanup_service._sweep_ephemeral_agents): DB pass + Docker-as-truth
orphan pass with 15-min newborn grace; capped per cycle
Part 2 — spawn provenance + parent control:
- Any agent-spawned creation persists spawned_by_agent/key_id and auto-grants
the agent_permissions parent→child edge (created_by="spawn:{parent}") so a
parent can immediately chat/list/info the child it spawned
- BEHAVIOR CHANGE: agent-scoped keys may start/stop/delete ONLY agents they
spawned (name AND key-id match; interim until #948); sharing, permission
grants, rename, and credential ops are now human-only (403 for agent keys)
- Ghost-key containment fence at the single auth entry point: a ghost's own
key reaches only heartbeat/result-callback/reports/notifications/self-info
Fleet hygiene: heartbeat watch + fleet health exclude ghosts; operator-queue
polling keeps them; exec/cost stats stay inclusive; schedules on ghosts → 400;
AgentStatus.ephemeral surfaced + GHOST badge.
Tests: tests/unit/test_69_ephemeral_agents.py (40, db_harness real-engine) —
accessors, facade delegations, acquire-gate matrix, key-fence matrix, Part 2
guard matrix, budget hook, discard idempotency/crash-convergence, atomic
quota. Full unit suite verified; residual order-dependent flakes reproduced
on clean dev (pre-existing, documented in learnings.md).
Refs abilityai/trinity-enterprise#69 (Phase 0 record on the issue; closed
manually at release — cross-repo keywords don't auto-close).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(69): patch _REAL_MODULES objects directly, never string targets — fixes seed-12345 order flake
CI's regression-diff (head seed 12345) caught the discard test flaking:
string-form monkeypatch targets resolve through sys.modules at PATCH time,
which under some pytest-randomly orderings is a sibling test's leaked stale
entry — the patch lands on the wrong module object while discard's call-time
import (under the _own_real_modules pin) resolves the real one, so
get_agent_container fell through to the real function (container=None,
removal skipped). Same hazard removed from the gated_capacity and
ghost_fence fixtures (bare fixture-time imports), and the audit assertion
moved to an instance-method patch on the pinned singleton.
Verified: full suite green under all three CI seeds (12345/67890/99999),
3775 passed each.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(learnings): string-target monkeypatch resolves stale sys.modules entries — patch by object
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(tests): pin real modules in TestAcquireCeilingClamp to kill sys.modules-leak flake (#1582) (#1584)
The trio (test_acquire_clamps_above_ceiling / test_get_slot_state_clamps /
test_get_all_states_clamps_each) failed order-dependently under
pytest-randomly on clean dev (5/7 observed runs), passed in isolation and
under -p no:randomly.
Root cause (the #762/#1446 family): `_patch_ceiling` patched
`get_max_parallel_tasks_ceiling` on the settings_service resolved via
`from services import settings_service`, but `capacity_manager.acquire`
imports `clamp_to_ceiling` at call time from `services.settings_service`.
When a sibling's module-level stub (e.g. test_fleet_status_resilience
installing a fresh `services` package with a real `__path__`) leaves two
module objects for `services.settings_service` in play, the patch lands on
one and `clamp_to_ceiling.__globals__` reads the other — the patch is
silently never hit and the clamp assertion sees the unpatched (default)
ceiling.
Fix (the pattern documented for this leak family):
- Capture the real `services` / `services.settings_service` /
`services.capacity_manager` at collection time (this file sorts before
every known leaker, so the import is leak-free).
- Autouse fixture re-pins them into sys.modules per test (monkeypatch
auto-restores) so the code-under-test's call-time import and the test's
patch target resolve to the SAME object — last-write-wins over any leak.
- `_patch_ceiling` now patches the captured real module object directly,
never a bare re-import that could resolve a leaked stub.
Verification: 25/25 in the file; 4/4 trio; 32 passed across 6 random seeds
with test_fleet_status_resilience co-resident; 111 passed / 0 ceiling
failures across 5 full-unit randomized seeds.
Related to #1582
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(reliability): decouple agent autonomy from the circuit breaker (#1557) (#1571)
Disabling autonomy called force_circuit_dormant, parking the transport
circuit breaker dormant. The execute_task gate consults that breaker for
every trigger, so a healthy paused agent fast-failed all inbound chat
(manual/Telegram/Slack/public) with "circuit breaker open — agent is
unhealthy" — never contacted. Autonomy governs proactive work only; it
now acts solely via set_schedule_enabled and never touches the breaker.
#631's flood protection is unaffected (the breaker's own failure-driven
dormant path + #1464 leader lock + #1121 monitoring-default-off).
Also splits the misleading fast-fail message to name the breaker that
fired (transport = unreachable, dispatch = auth-dead).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
* feat(voice): voice replies v2 — per-message capability, agent-level config, runtime ElevenLabs key (trinity-enterprise#117) (#1549)
* feat(voice): voice replies v2 — per-message capability, agent-level config, runtime ElevenLabs key (trinity-enterprise#117)
Rework outbound voice replies (ElevenLabs TTS) along three lines:
1. Voice is a per-message capability, not a hard rule. Channel replies are text
by default; a reply becomes a voice note only when the agent explicitly calls
the new send_voice_reply MCP tool during the turn. The backend resolves the
channel destination from the execution (new schedule_executions.source_channel*
columns), gates on TTS availability + agent enable + per-channel flag, wraps
delivery in effect_guard (#1084), and reuses each channel's send primitive.
The always-voice adapter path (_maybe_send_voice) is removed.
2. Voice config moves to agent Settings (enable + voice selection, one place);
channel panels keep only a per-channel on/off flag
(agent_ownership.tts_voice_{telegram,slack,whatsapp}_enabled, default ON).
GET/PUT /api/agents/{name}/voice-replies extended with channels + effective voice.
3. ElevenLabs API key + platform default voice are runtime-configurable in admin
Settings (GET/PUT /api/settings/elevenlabs), key stored AES-256-GCM encrypted
and surfaced as configured:bool only; resolved via
settings_service.get_elevenlabs_api_key() (stored setting -> env), no restart.
New tts_available feature flag.
Dual-track migrations (SQLite + Alembic 0015/0016). New send_voice_reply MCP tool
(voice.ts) + POST /api/agents/{name}/voice-reply + voice_reply_service. Capability
advertised in the platform prompt only when voice is enabled for the agent and the
current channel's flag is on. OSS-core.
Related to trinity-enterprise#117
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(voice): forward source_channel* through the db facade create_task_execution (trinity-enterprise#117)
DatabaseManager.create_task_execution (the `db` facade wrapper) didn't forward the
new source_channel / source_channel_chat_id / source_channel_thread kwargs to
ScheduleOperations, so a channel-triggered task raised
"unexpected keyword argument 'source_channel'" → 500. Add the passthrough.
Related to trinity-enterprise#117
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(voice): allow send_voice_reply on channel turns via --allowedTools (trinity-enterprise#117)
Channel turns run headless with a restricted `--allowedTools` (default
WebSearch,WebFetch), which blocks every MCP tool — so an agent could not call
send_voice_reply even when the capability was advertised in its prompt. When the
voice capability is advertised for a channel turn, also append
`mcp__trinity__send_voice_reply` to the channel allowed-tools list so the agent
can actually act on it.
Related to trinity-enterprise#117
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(migrations): stage the Alembic re-chain — 0017/0018 revision ids + down_revision onto 0016_agent_ownership_ephemeral
The dev-merge commit renamed the files but the id/down_revision edits were left unstaged, leaving two heads off 0015 (pg-migrations 'Multiple head revisions' failure).
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
* chore(dev-skills): bump .claude — /cso v1.1 Trinity-shape refresh
Pointer bump to trinity-dev 2bdd362: fixes stale audit checks (setup-token
removal ent#49, Redis ACL model, dual-track DB / two-network stack facts)
and adds checks for post-skill surfaces (agent-key self-boundaries
#307/#1083/#918, backend→agent auth #1159, webhook HMAC ent#77, vendored
parity Invariant #5, enumeration uniformity #186, MCP description leak
#846, backlog_metadata G-04 class, enterprise-docs-guard ent#45).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(security): add CSO full-codebase audit report (2026-07-13)
Full-codebase Chief Security Officer audit (all phases, daily 8/10 gate).
No CRITICAL or exploitable-HIGH findings; prior CRITICAL (unauthenticated
agent-server, #1159) and both HIGH supply-chain items resolved. Ceiling is
MEDIUM: shared-user privilege tier, npm-ci lockfile bypass, unenforced
CODEOWNERS, plaintext backlog_metadata, unpinned deps, enterprise-doc
disclosure the guard can't see, python-multipart CVE. Two candidate findings
(orb.js XSS, Slack code takeover) downgraded to LOW by adversarial verifiers.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(portal): hide the platform Help widget on client-portal routes (#1588)
The global "Trinity Help" chat widget is fixed at bottom-right (z-50) and
overlapped the client portal chat composer's Send button. It's an operator/
platform-docs widget (gated on platform auth) and is meaningless on the
standalone client portal, so gate it off there via a `hideHelpWidget` route
meta on the two portal routes (`ClientPortalPublic` /portal and
`EnterpriseClientPortal`). App.vue now renders it only when
`authStore.isAuthenticated && !route.meta.hideHelpWidget`.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(portal): attach/upload files directly in the customer portal chat (ent#144) (#1587)
A client can now attach files in the portal chat composer. The backend already
had everything — `POST /client-portal/agents/{name}/documents` (writes to the
client inbox), and `portal_chat`'s `_collect_inbox_for_turn` attaches inbox
images as vision + lists documents — and the store already had
`uploadDocument`/`fetchUploads`. The only missing piece was the composer UI.
- `PortalChat.vue`: adds an `uploadDocument` prop and a paperclip attach control
(hidden multi-file input). Picked files upload to the inbox immediately and
render as chips with state (uploading / done / error) and a remove button.
On send, the done attachments' (server-sanitized) filenames are appended as
`[Attached: …]` so `_collect_inbox_for_turn` attaches images as vision THAT
turn (it keys on the filename / image-intent) and lists documents. Files-only
turns are allowed; send is blocked while an upload is in flight; a 25 MB
client guard mirrors the backend cap (415/quota surfaced as a chip error).
- `Portal.vue`: passes `:upload-document="(name, file) => store.uploadDocument(name, file)"`.
- The attach control hides when no upload handler is wired (operator-preview
`ClientPortal.vue` renderer), so nothing changes there.
Public-repo (gated Vue) change only — no backend/submodule change (reuses the
shipped enterprise endpoint + inbox consumption).
Refs Abilityai/trinity-enterprise#144
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(access): restore the per-recipient proactive-messaging toggle (#1577) (#1590)
The `allow_proactive` toggle (the only UI for #321/#376) was silently dropped
when the #1317 Access-tab redesign replaced the Sharing tab's Team Sharing rows
— leaving no way to opt a recipient into proactive messages except a raw API
call. Backend was fully intact; this is a UI-only restore.
- `AccessPanel.vue`: each operator row gets a Proactive toggle bound to the
`allow_proactive` the `/access` roster already returns (no extra fetch). On
change it persists via the store and reflects the server's confirmed value,
reverting + surfacing the error on failure (honest status, no optimistic-only
flip). Pending invites are toggleable too (the flag rides on the
`agent_sharing` row, which exists pre-resolution) with a tooltip on timing. A
static note states the owner is always allowed (owners aren't in the roster,
so there's no misleading owner toggle).
- `stores/agents.js`: new `setProactive(name, email, allow)` →
`PUT /api/agents/{name}/shares/proactive`.
- `feature-flows/proactive-messaging.md`: updated — it still documented the
removed SharingPanel markup/line numbers.
- `tests/unit/test_1577_proactive_toggle_guard.py`: static regression guard (3)
asserting the toggle + endpoint wiring survive future panel refactors (the
frontend has no JS unit runner; a full render e2e is a `ui` follow-up).
Related to #1577
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(github): wire the managed agent PAT for the gh CLI + REST API, not just git (#1574) (#1591)
Trinity injected the resolved GitHub PAT as GITHUB_PAT (authenticating git via
the origin URL) but not the `gh` CLI or REST API, which read GH_TOKEN/GITHUB_TOKEN
— so agents had to prefix every command with `GH_TOKEN="$GITHUB_PAT" gh …`, and
`gh` wasn't even installed. This makes the SAME token cover both. No new token,
endpoint, or UI — wiring only.
Expose GH_TOKEN + GITHUB_TOKEN = the resolved PAT at every point GITHUB_PAT is
set today, gated identically (only when a repo + PAT resolve — never an empty
token that makes `gh` look logged-in but broken):
- create (`crud.py`) and recreate (`lifecycle.py`) bake them into the container env;
- the no-restart `.env` propagation (`github_pat_propagation_service._patch_env_github_pat`)
now patches/adds all three keys, kept in sync;
- `startup.sh` exports them from GITHUB_PAT so child processes (agent server,
terminal shells) auto-authenticate — also covering older baked images that
only carry GITHUB_PAT, without a recreate.
Install the `gh` CLI in the agent base image (`Dockerfile`) from GitHub's official
apt repo (base image must be rebuilt for existing agents to get the binary; the
env vars are harmless on older images — git keeps working).
Honest surfaces: the `set_agent_github_pat` MCP tool description and
`docs/.../github-pat-setup.md` now state the managed token covers `gh`/REST too,
keeping the scope caveat (wiring makes the token available; it can't grant scopes
the token lacks). The credential sanitizer's `.*TOKEN.*`/`GITHUB_.*` patterns
already mask GH_TOKEN/GITHUB_TOKEN (asserted).
Tests: `tests/unit/test_1574_gh_token_wiring.py` — the .env patcher mirrors the
PAT onto all three keys (replace-in-place / append, lookalike-key-safe, value
mirrored); static guards that create/recreate/startup/Dockerfile each wire the gh
vars; sanitizer covers the new vars.
Related to #1574
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(operator-queue): quarantine persistently-failing creates to stop the sync hot-loop (#1525) (#1589)
The `created_at` KeyError in `create_item` was already made defensive on dev
(#1426 — `.get(...) or utc_now_iso()` + `on_conflict_do_nothing`). This closes
the remaining #1525 gap: the sync loop still re-attempted ANY failing create on
every ~5s cycle forever (the row never persists → `operator_queue_item_exists`
stays False → retry + ERROR-log indefinitely), so a DB error or any other
persistent create failure — not just the fixed field case — still hot-loops.
- `operator_queue_service._sync_agent`: track per-request consecutive
create-failure counts; after `MAX_CREATE_ATTEMPTS` (3) skip that request
(one WARN at the quarantine threshold instead of an unbounded ERROR stream).
A create that later succeeds clears the counter; an in-memory safety valve
caps the map so it can never grow without bound.
- `db/operator_queue.create_item`: the last hard-indexed field (`id`) now uses
`.get` and raises a clear `ValueError` (which the caller quarantines) instead
of an opaque `KeyError`. Also hardened the WS-broadcast `item["id"]`.
Tests: `test_1525_operator_queue_quarantine.py` (4, pure/mocked) — create is
attempted at most the cap (not once per cycle), success clears the counter, a
healthy request leaks no quarantine state, and the id-guard raises ValueError.
Related to #1525
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(mcp): one-click Copy connection config on the Expose-via-MCP panel (#1575) (#1585)
* feat(mcp): one-click Copy connection config on the Expose-via-MCP panel (#1575)
Surfaces a ready-to-paste external-client config — with a least-privilege,
agent-scoped, revocable API key already embedded — from the #846 exposure
panel, so an external MCP client connects in one flow (enable → copy → paste).
Direction: reuse the existing per-agent MCP connector (ent#46 → OSS #118)
rather than mint a duplicate credential system. The connector already provides
exactly the hard parts #1575 asks for — a scoped `scope='connector'` key,
owner-selected playbooks exposed as tools, and `build_snippets` producing
per-client `.mcp.json`/CLI blocks with the key embedded — but only on the
Sharing tab. This wires that capability onto `McpExposedPanel.vue` (Settings →
Expose via MCP), shown when `mcp_exposed` is on.
- Frontend-only, no new backend endpoint/key type: reuses
`GET/POST/DELETE /api/agents/{name}/connector[/key]` (owner-only,
`OwnedAgentByName`) + `ExposedToolsPanel` for the playbook allow-list.
- One-click "Copy connection config": mints (or regenerates) the scoped key and
copies the `.mcp.json` in a single action via the robust `utils/clipboard`
helper — the only moment the live secret exists.
- Copy-once integrity: with an existing key, "Copy config" copies the
placeholder config and offers "Regenerate & copy" for a fresh live key;
"Revoke" severs any connected client. Key already lists in Settings → MCP Keys.
- Safe default: the section only appears once the agent is MCP-exposed; a plain
warning marks the config as a live secret.
Docs: mcp-connector.md gains the 2nd-surface row; architecture #846 block notes
the connect surface.
Verification: `McpExposedPanel.vue` compiles clean via @vue/compiler-sfc
(full `vite build` blocked by a pre-existing unrelated missing `mermaid` dep in
AgentWorkspace.vue). Live click-through / ui-labeled e2e needs the running
frontend+backend stack (not up locally).
Not changed (documented scope): a 422 on an invalid playbook name — the picker
only offers live user_invocable playbooks, so the invalid path is UI-unreachable;
left to the connector's existing read-time filter.
Related to #1575
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(mcp): flash a "Copied!" animation on the connection-config copy buttons (#1575)
Adds an inline copied-state affordance to the connect-config copy actions
(Copy connection config / Copy config / per-client snippet Copy): on a
successful clipboard write the button swaps to a green "✓ Copied!" with a
scale-pop checkmark for ~1.6s, then reverts. One shared timer (cleared on
unmount); only fires when the copy actually succeeded; honors
prefers-reduced-motion.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(mcp): make the copied state transform the button itself (#1575)
Stronger, unmissable copy feedback: on a successful copy the primary
"Copy connection config" button fills solid success-green, gains a
ring + glow, bumps to a bolder "Copied to clipboard!" with a larger
checkmark, and plays a one-shot pop + outward ring-pulse
(copied-btn-flash) before settling. The existing-key "Copy config"
button gets the same solid-green ring treatment. Reverts after ~1.6s;
honors prefers-reduced-motion.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(mcp): animate the connector snippet Copy buttons too (#1575)
The MCP connector panel (Sharing tab, ConnectorChannelPanel) is the other
surface that shows per-client copy snippets; its plain "Copy" links gave no
feedback. Give them the same copied-state treatment as the exposure panel:
on a successful copy the button flashes solid success-green with a ring +
pop + "Copied!" checkmark, reverting after ~1.6s (honors reduced-motion).
Also route its copy() through the robust utils/clipboard helper instead of
raw navigator.clipboard.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(tests): conftest no longer deletes real agents named test-* (#1558) (#1586)
* fix(tests): stop conftest from deleting real agents named test-* (#1558)
The session-scoped `api_client` fixture deleted EVERY agent whose name began
with `test-` on whatever `TRINITY_API_URL` pointed at — silent, unprompted
data loss that destroyed a developer's real `test-agent-2` (bound Telegram
bot, chat history, GitHub sync). Any `test-*` agent on a staging/prod instance
was one test invocation from deletion.
Fix — fail-closed on every axis:
- Dedicated prefix: the suite now names every agent it creates
`pytest-ephemeral-*` (a namespace no human uses), never the broad `test-`.
Renamed all conftest agent generators (test_agent_name, module_agent_name,
stopped_agent, shared_agent).
- Session registry: created names are registered (`register_created_agent`)
and reclaimed by name at session end (belt over each fixture's own teardown).
- Startup leftover-sweep is now OPT-IN (`TRINITY_TEST_CLEANUP_SWEEP`), refuses
any non-localhost target (`is_local_target`), and only removes provably
suite-owned names (`select_sweepable_agents` — pure, fail-closed). Default:
no sweep at all.
- `cleanup_test_agent(require_suite_owned=True)` refuses (no stop/delete call)
a name the session can't prove it created — used by every sweep path.
- Docstring + tests/README now state plainly that the suite mutates the target
instance and must point at localhost.
…
Open
5 tasks
vybe
added a commit
that referenced
this pull request
Jul 28, 2026
…ocal:default (#1759) (#1824) * test(agents): failing tests for absent local: template (#1759) TDD RED first. Proven failing on the pristine cut base f7aff46: 18 failed, 13 passed — T1 fails with the exact defect, "Failed: DID NOT RAISE HTTPException". - test_1759_local_template_not_found.py — T1 (400 + no side effects), T1b/T1c (no path/root disclosure, one identical message), T2/T3 (curated + deploy-local roots still create), T4 (dir without template.yaml), T5 (empty/non-dict), T6 (unparseable YAML), T7 (Blank Agent unaffected), T8 (traversal barrier keeps precedence), T10 (/template bind source, incl. the empty-env case). Every test monkeypatches crud._LOCAL_TEMPLATE_ROOTS to real tmp dirs, never template_service (MagicMocked in the #1484 harness). - test_1759_template_root_parity.py — T9: create resolver vs listing surface, deliberately NOT using the _load_crud mock harness. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(agents): reject an absent local: template with a named 400 (#1759) A well-formed `local:<name>` whose directory (or template.yaml) did not exist under either root fell through `_resolve_local_template` with no `else` — `template_data == {}`, the same HTTP 200 as success, and a blank container with no instructions. Sibling of the unprefixed case #843 made loud. - Gate: `LOCAL_TEMPLATE_NOT_FOUND` 400 raised inside `_resolve_local_template`, the same pre-side-effect band as FORK_REQUIRES_GITHUB_TEMPLATE. Nothing between it and the docker try-block allocates a resource, so no rollback is needed and the 4xx is not flattened to a 500. `create_agent_internal` gains ZERO lines (#1484: 118 SLOC before and after). - ONE identical message whichever root missed, with no filesystem path and no root name: deploy-local templates (#950) are named after AGENT names, so a root-distinguishing error would let a creator-role caller probe another user's deploy-local agents (#186 adjacency). - D5: an empty / non-dict / unparseable template.yaml produced the identical blank-agent outcome via `except Exception: logger.warning`. Narrowed to (OSError, yaml.YAMLError) -> 400 LOCAL_TEMPLATE_INVALID, plus an isinstance guard. The listing path already rejected both, so create was strictly less strict than the surface advertising the template. Field-level malformation still degrades gracefully. - Repo-relative curated root (`parents[4]`, hand-rolled — importing template_service would be satisfied by the #1484 harness MagicMock), so the gate is not inert wherever the test suite runs (#1638 accidental-green). Both seams take it: the resolver AND the /template bind, whose base is now `os.getenv(...) or _default_host_templates_base()` — an empty HOST_TEMPLATES_PATH made `Path("")/name` a bare name, i.e. an empty NAMED VOLUME at /template: this bug class, one seam over. Container path is byte-identical (compose always sets HOST_TEMPLATES_PATH). Create-time only — rebuild/recreate reads template.yaml off the persisted volume, so no running agent is affected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(templates): ship a real hidden `local:default` template (#1759) AC#2. `local:default` is referenced 46× in tests/test_systems.py (~19 agent-creating deploys), in test_subscriptions.py and test_agent_lifecycle.py, and in ~35 lines across 6 doc files — but no `default` directory has ever existed. Once create hard-fails on an absent template those all 400. Shipping the template honours the already-published contract; repointing ~85 refs is mechanical churn across three live-server suites for zero product value. - `hidden: true` (house convention: test-*, demo-*, sleep-echo, trinity-system): it stays resolvable by id and creatable, but never enters the user-facing picker, where it would duplicate the UI's own "Blank Agent" option. - template.yaml declares NO `resources:` — nor type/tools/runtime/ shared_folders/credentials.mcp_servers. `_resolve_local_template` applies a template's `resources` OVER the caller's, so a copy-pasted block would neutralise the ent#125 partial-deploy test's deliberate `cpu: "3"` failure vector and flip it from `partial` to `deployed`. Every existing fixture declares `resources`, so the copy-paste path lands straight on this landmine — hence the in-file warning comment. - CLAUDE.md ships too: ent#239 codifies "a template.yaml with no CLAUDE.md deploys an agent with no instructions" as a quality failure. - `default` added to the listing guard's explicit `forbidden` set: it matches neither the test-/demo- prefix convention nor any other check, so dropping `hidden: true` would leak it to every user with no CI signal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(systems): export a resolvable template for template-less agents (#1759) `export_manifest` built each entry as `agent.get('template', 'local:business-assistant')`, broken twice: 1. `dict.get(key, default)` returns the default only when the key is ABSENT. Every Blank Agent's dict carries `"template": None` (routers/agents.py), so the fallback was unreachable DEAD CODE and blank agents have always exported `template: null`. As `SystemAgentConfig.template` is a non-Optional `str`, redeploying that manifest already failed Pydantic validation. 2. `config/agent-templates/business-assistant` has never existed, so even a reachable fallback named a template that now 400s. So this fixes a PRE-EXISTING broken round-trip on the platform's most common agent type — the create-time gate does not cause it. Uses `or` (the same footgun as the HOST_TEMPLATES_PATH `os.getenv` default, twice in one change) and `local:default`, which is truthful for a template-less agent where a product template would fabricate provenance. A `logger.warning` names the inferred agents: `export_manifest` returns a bare YAML string, so a structured field would change the response contract. Also adds the AC#3 test: the create-time 400 reaches the operator through ent#125's per-agent `failed[]` (status_code 400, reason = the self-contained sentence, sibling agent still deployed, manifest not aborted). No new plumbing — the test proves the machinery already works, and pins that `_failure_reason` drops `detail["code"]`, which is why the sentence must stand alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(templates): document the LOCAL_TEMPLATE_NOT_FOUND contract (#1759) Docs delta decided at plan time (Trinity Rule #1 — tiered updates). - requirements/core-agent.md §4.1: a bug fix is normally commit-message only, but this ADDS a named error code to a documented create-path contract, and §4 already documents sibling codes (FORK_TO_OWN_REQUIRED, the `local:` rejection). Records the ordering against INVALID_LOCAL_TEMPLATE_NAME, the Blank-Agent carve-out, the one-message-per-root disclosure rule, and the ent#125 failed[] path. - feature-flows/system-manifest.md: 5 `local:business-assistant` examples repointed to `local:default`. That directory has NEVER existed, so those copy-pasteable manifests would now 400. - onboarding: the 3 UI-template-PICKER mentions of `local:default` now name visible starters (scout/scribe). `default` ships `hidden: true`, so it never appears in the Create-Agent picker those lines describe. The ~30 remaining manifest/API `local:default` examples stay valid — that is the payoff of shipping the template. - learnings.md 2026-07-23: annotated CLOSED; that entry ended "the loud-reject follow-up is tracked as its own bug" — this is it. - architecture.md deliberately NOT changed: its crud.py entry is a <=2-line catalog entry per that file's own editorial rules, and a named 400 inside one helper is below that granularity. Recorded as a decision, not an oversight. Two test fixups the fix makes necessary: - test_systems.py TestResilientDeploy docstring asserted the BUG as fact ("a well-formed but absent local: template does NOT fail") and would have become a lie in the exact place the next engineer reads. - test_agent_lifecycle.py `if response.status_code in [200, 201]` was tolerating a template that never existed; with `local:default` real and absence now fatal, that tolerance only hid failures. Tightened to assert_status_in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(feature-flows): sync template-processing with the #1759 gate /sync-feature-flows tail step. - template-processing.md "Local Templates" was materially stale: it showed a single-root lookup with an inline `templates_dir` fallback and no `_safe_local_template_path`, i.e. the pre-#950 shape. Replaced with the real two-root resolution, the CodeQL barrier, the #1759 failure-contract table (which condition yields which of the three 400 codes, and that Blank Agent never enters the branch), the pre-side-effect / zero-orchestrator-lines placement, the one-message disclosure rule, and the two-seam note incl. the empty HOST_TEMPLATES_PATH named-volume trap. - feature-flows.md: Recent Updates row (the index row this tail step reliably drops). Note: the Recent Updates table carries 52 rows against its own stated "newest ~20" cap (#1360) — pre-existing drift, deliberately not trimmed here (out of scope for a P2 bug fix). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(models,tests): correct two comments that named the old behaviour (#1759) - models.py:571 `SystemAgentConfig.template` cited `local:business-assistant` as its canonical example — a template that has never existed on disk and now 400s on create. This is the exact field the exporter fix hinges on, so the example being wrong is the worst place for it. Repointed to a shipped starter. - test_ent124_default_system_seed.py: the bundled-manifest guard's docstring asserted "an absent local: template creates a BLANK agent" as present-tense fact. Updated to say #1759 closed that and why the check is still worth keeping (collection-time failure with a precise message vs a runtime seed failure landing in the operator queue, and it still covers the ent#239 CLAUDE.md half the create gate never checks). Verified by hand: every `local:` reference in config/ and src/ resolves to a real template directory — the three shipped manifests (default-system, acme-consulting, research-network) and the Cornelius first-run seed are all safe. Only the two comments above referenced the nonexistent one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(templates): make the path-disclosure guard actually guard (#1759) The `_leaked_paths` helper is the general backstop behind the plan's single binding security rule: ONE identical message whichever root missed, echoing no resolved path — because deploy-local templates (#950) are named after AGENT names, so "which root missed" is itself a #186 enumeration oracle. The guard did not enforce that rule. `(?:^|\s)(/...)` only sees a path preceded by whitespace, so it was blind to a path wrapped in ANY punctuation — including `!r`, which is the quoting style these very messages already use for `config.template`. A future `f"...{template_path!r}..."` emits `'/data/deployed-templates/x'` and sailed straight through. Executed: 8 of 9 leak shapes were MISSED (repr-quoted, double-quoted, parenthesised, colon-prefixed, backticked, relative, ...). Worse, no assertion in the file covered the RELATIVE form. t1b's full pre-fix assertion set — all five substring checks plus `_leaked_paths` — passes a message reading "no template.yaml under data/deployed-templates/acme-payroll-agent", which discloses another user's deploy-local agent name exactly as the absolute form would. Two changes, both test-only: - Negative lookbehind `(?<![A-Za-z0-9_.\-/])` instead of a leading-whitespace anchor, so a `/` is "absolute" whenever it does not continue a relative path. `config/agent-templates/` stays clean (its `/` follows `g`); `/api/...` remains the one allowed `/`-rooted form. - `_SENSITIVE_ROOT_TOKENS` — naming either root is banned with or without a leading slash. `agent-templates` is deliberately NOT listed; it is the public, intentional remedy. Verified: all 9 leak shapes now caught, 0 false positives on the three real messages, and an end-to-end mutant that leaks `{template_path!r}` fails t1b. POSIX-only is deliberate, not an oversight — every path reaching these messages is a `pathlib.Path` built inside the Linux backend container, so a Windows branch would be dead regex. Also covers seam 2's CONTAINER branch, which had no test at all: every other test runs on a source checkout where `/agent-configs/templates` is absent, so only the repo-fallback branch was exercised. D1-A3's justification for widening the bind seam is "the delta in the verified container path is provably zero" — that claim now has a test. Pointing the module global at a real directory exercises the in-container branch without a container. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(templates): name the template in the partially-applied-config warning (#1759) #1759 narrowed the template.yaml PARSE (OSError/YAMLError -> a named 400) but deliberately kept a broad `except Exception` around the FIELD mutation that follows. That decision is right and is left standing: the file parsed, the agent does get its template files, and only some `config` mutations are skipped. Tightening it to a 400 would reject templates that deploy successfully today, which is outside this issue's acceptance criteria. What was not right is that the swallow was unfindable. The mutations run in order, so a raise part-way through leaves a PARTIALLY applied template — e.g. `credentials: "a string"` applies type/resources/tools and then silently skips mcp_servers/runtime/shared_folders. The message was `f"Error loading template config: {e}"`: it named neither the template nor the agent, so an operator holding a subtly-wrong agent had nothing to grep for. Log-only. No HTTP status, no control flow, and no config value changes. Adds a regression test that pins two things the swallow must never lose: 1. It stays a DEGRADE. The handler runs inside an `except`, so a bug in the handler itself converts a graceful degrade into a 500 on the create path. Proven load-bearing by mutation: replacing `config.name` with a non-existent attribute makes the test fail, and restoring it makes it pass. 2. The warning names both the template and the agent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(learnings): record the decorative-disclosure-guard class (#1759) The #1759 review found `_leaked_paths` — the backstop behind the fix's one binding security rule — missing 8 of 9 leak shapes, including the `!r` form those same messages already use. Records the durable rule so a future plan writes the adversarial table before trusting the guard. Same family as the #1644 "a guard that fails open manufactures confidence" and #1638 "a test asserting the defective behaviour ships it green" entries, one level up: a guard that never fires at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(templates): explain why the parents[4] fallback is unreachable (#1759) `_repo_local_templates_dir` indexes `parents[4]`, which the container layout (`/app/services/agent_service/crud.py`, 4 parents) cannot satisfy. Record why that is latent rather than live: the fallback runs only when `/agent-configs/templates` is missing, and both compose files always bind it on `backend`, so `_LOCAL_TEMPLATE_ROOTS[0]` resolves first. The defensive `len(parents) <= 4` guard is a flagged follow-up. Comment only — no behaviour change, so the shipped artifact stays byte-equivalent to the one /verify-local exercised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(submodule): drop accidental .claude pointer bump (#1759) The branch carried .claude at 3dbde4bf, a commit that exists only on an unmerged trinity-dev branch — pinning the public repo's submodule there would break 'git submodule update' for every clone if that branch is rebased or deleted. Reverts the pointer to de7fc716, the trinity-dev main tip that dev already points at. No other content change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Eugene Vyborov <1073874+vybe@users.noreply.github.com>
vybe
added a commit
that referenced
this pull request
Jul 28, 2026
…ocal:default (#1759) (#1824) * test(agents): failing tests for absent local: template (#1759) TDD RED first. Proven failing on the pristine cut base f7aff46: 18 failed, 13 passed — T1 fails with the exact defect, "Failed: DID NOT RAISE HTTPException". - test_1759_local_template_not_found.py — T1 (400 + no side effects), T1b/T1c (no path/root disclosure, one identical message), T2/T3 (curated + deploy-local roots still create), T4 (dir without template.yaml), T5 (empty/non-dict), T6 (unparseable YAML), T7 (Blank Agent unaffected), T8 (traversal barrier keeps precedence), T10 (/template bind source, incl. the empty-env case). Every test monkeypatches crud._LOCAL_TEMPLATE_ROOTS to real tmp dirs, never template_service (MagicMocked in the #1484 harness). - test_1759_template_root_parity.py — T9: create resolver vs listing surface, deliberately NOT using the _load_crud mock harness. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(agents): reject an absent local: template with a named 400 (#1759) A well-formed `local:<name>` whose directory (or template.yaml) did not exist under either root fell through `_resolve_local_template` with no `else` — `template_data == {}`, the same HTTP 200 as success, and a blank container with no instructions. Sibling of the unprefixed case - Gate: `LOCAL_TEMPLATE_NOT_FOUND` 400 raised inside `_resolve_local_template`, the same pre-side-effect band as FORK_REQUIRES_GITHUB_TEMPLATE. Nothing between it and the docker try-block allocates a resource, so no rollback is needed and the 4xx is not flattened to a 500. `create_agent_internal` gains ZERO lines (#1484: 118 SLOC before and after). - ONE identical message whichever root missed, with no filesystem path and no root name: deploy-local templates (#950) are named after AGENT names, so a root-distinguishing error would let a creator-role caller probe another user's deploy-local agents (#186 adjacency). - D5: an empty / non-dict / unparseable template.yaml produced the identical blank-agent outcome via `except Exception: logger.warning`. Narrowed to (OSError, yaml.YAMLError) -> 400 LOCAL_TEMPLATE_INVALID, plus an isinstance guard. The listing path already rejected both, so create was strictly less strict than the surface advertising the template. Field-level malformation still degrades gracefully. - Repo-relative curated root (`parents[4]`, hand-rolled — importing template_service would be satisfied by the #1484 harness MagicMock), so the gate is not inert wherever the test suite runs (#1638 accidental-green). Both seams take it: the resolver AND the /template bind, whose base is now `os.getenv(...) or _default_host_templates_base()` — an empty HOST_TEMPLATES_PATH made `Path("")/name` a bare name, i.e. an empty NAMED VOLUME at /template: this bug class, one seam over. Container path is byte-identical (compose always sets HOST_TEMPLATES_PATH). Create-time only — rebuild/recreate reads template.yaml off the persisted volume, so no running agent is affected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(templates): ship a real hidden `local:default` template (#1759) AC#2. `local:default` is referenced 46× in tests/test_systems.py (~19 agent-creating deploys), in test_subscriptions.py and test_agent_lifecycle.py, and in ~35 lines across 6 doc files — but no `default` directory has ever existed. Once create hard-fails on an absent template those all 400. Shipping the template honours the already-published contract; repointing ~85 refs is mechanical churn across three live-server suites for zero product value. - `hidden: true` (house convention: test-*, demo-*, sleep-echo, trinity-system): it stays resolvable by id and creatable, but never enters the user-facing picker, where it would duplicate the UI's own "Blank Agent" option. - template.yaml declares NO `resources:` — nor type/tools/runtime/ shared_folders/credentials.mcp_servers. `_resolve_local_template` applies a template's `resources` OVER the caller's, so a copy-pasted block would neutralise the ent#125 partial-deploy test's deliberate `cpu: "3"` failure vector and flip it from `partial` to `deployed`. Every existing fixture declares `resources`, so the copy-paste path lands straight on this landmine — hence the in-file warning comment. - CLAUDE.md ships too: ent#239 codifies "a template.yaml with no CLAUDE.md deploys an agent with no instructions" as a quality failure. - `default` added to the listing guard's explicit `forbidden` set: it matches neither the test-/demo- prefix convention nor any other check, so dropping `hidden: true` would leak it to every user with no CI signal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(systems): export a resolvable template for template-less agents (#1759) `export_manifest` built each entry as `agent.get('template', 'local:business-assistant')`, broken twice: 1. `dict.get(key, default)` returns the default only when the key is ABSENT. Every Blank Agent's dict carries `"template": None` (routers/agents.py), so the fallback was unreachable DEAD CODE and blank agents have always exported `template: null`. As `SystemAgentConfig.template` is a non-Optional `str`, redeploying that manifest already failed Pydantic validation. 2. `config/agent-templates/business-assistant` has never existed, so even a reachable fallback named a template that now 400s. So this fixes a PRE-EXISTING broken round-trip on the platform's most common agent type — the create-time gate does not cause it. Uses `or` (the same footgun as the HOST_TEMPLATES_PATH `os.getenv` default, twice in one change) and `local:default`, which is truthful for a template-less agent where a product template would fabricate provenance. A `logger.warning` names the inferred agents: `export_manifest` returns a bare YAML string, so a structured field would change the response contract. Also adds the AC#3 test: the create-time 400 reaches the operator through ent#125's per-agent `failed[]` (status_code 400, reason = the self-contained sentence, sibling agent still deployed, manifest not aborted). No new plumbing — the test proves the machinery already works, and pins that `_failure_reason` drops `detail["code"]`, which is why the sentence must stand alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(templates): document the LOCAL_TEMPLATE_NOT_FOUND contract (#1759) Docs delta decided at plan time (Trinity Rule #1 — tiered updates). - requirements/core-agent.md §4.1: a bug fix is normally commit-message only, but this ADDS a named error code to a documented create-path contract, and §4 already documents sibling codes (FORK_TO_OWN_REQUIRED, the `local:` rejection). Records the ordering against INVALID_LOCAL_TEMPLATE_NAME, the Blank-Agent carve-out, the one-message-per-root disclosure rule, and the ent#125 failed[] path. - feature-flows/system-manifest.md: 5 `local:business-assistant` examples repointed to `local:default`. That directory has NEVER existed, so those copy-pasteable manifests would now 400. - onboarding: the 3 UI-template-PICKER mentions of `local:default` now name visible starters (scout/scribe). `default` ships `hidden: true`, so it never appears in the Create-Agent picker those lines describe. The ~30 remaining manifest/API `local:default` examples stay valid — that is the payoff of shipping the template. - learnings.md 2026-07-23: annotated CLOSED; that entry ended "the loud-reject follow-up is tracked as its own bug" — this is it. - architecture.md deliberately NOT changed: its crud.py entry is a <=2-line catalog entry per that file's own editorial rules, and a named 400 inside one helper is below that granularity. Recorded as a decision, not an oversight. Two test fixups the fix makes necessary: - test_systems.py TestResilientDeploy docstring asserted the BUG as fact ("a well-formed but absent local: template does NOT fail") and would have become a lie in the exact place the next engineer reads. - test_agent_lifecycle.py `if response.status_code in [200, 201]` was tolerating a template that never existed; with `local:default` real and absence now fatal, that tolerance only hid failures. Tightened to assert_status_in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(feature-flows): sync template-processing with the #1759 gate /sync-feature-flows tail step. - template-processing.md "Local Templates" was materially stale: it showed a single-root lookup with an inline `templates_dir` fallback and no `_safe_local_template_path`, i.e. the pre-#950 shape. Replaced with the real two-root resolution, the CodeQL barrier, the #1759 failure-contract table (which condition yields which of the three 400 codes, and that Blank Agent never enters the branch), the pre-side-effect / zero-orchestrator-lines placement, the one-message disclosure rule, and the two-seam note incl. the empty HOST_TEMPLATES_PATH named-volume trap. - feature-flows.md: Recent Updates row (the index row this tail step reliably drops). Note: the Recent Updates table carries 52 rows against its own stated "newest ~20" cap (#1360) — pre-existing drift, deliberately not trimmed here (out of scope for a P2 bug fix). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(models,tests): correct two comments that named the old behaviour (#1759) - models.py:571 `SystemAgentConfig.template` cited `local:business-assistant` as its canonical example — a template that has never existed on disk and now 400s on create. This is the exact field the exporter fix hinges on, so the example being wrong is the worst place for it. Repointed to a shipped starter. - test_ent124_default_system_seed.py: the bundled-manifest guard's docstring asserted "an absent local: template creates a BLANK agent" as present-tense fact. Updated to say #1759 closed that and why the check is still worth keeping (collection-time failure with a precise message vs a runtime seed failure landing in the operator queue, and it still covers the ent#239 CLAUDE.md half the create gate never checks). Verified by hand: every `local:` reference in config/ and src/ resolves to a real template directory — the three shipped manifests (default-system, acme-consulting, research-network) and the Cornelius first-run seed are all safe. Only the two comments above referenced the nonexistent one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(templates): make the path-disclosure guard actually guard (#1759) The `_leaked_paths` helper is the general backstop behind the plan's single binding security rule: ONE identical message whichever root missed, echoing no resolved path — because deploy-local templates (#950) are named after AGENT names, so "which root missed" is itself a #186 enumeration oracle. The guard did not enforce that rule. `(?:^|\s)(/...)` only sees a path preceded by whitespace, so it was blind to a path wrapped in ANY punctuation — including `!r`, which is the quoting style these very messages already use for `config.template`. A future `f"...{template_path!r}..."` emits `'/data/deployed-templates/x'` and sailed straight through. Executed: 8 of 9 leak shapes were MISSED (repr-quoted, double-quoted, parenthesised, colon-prefixed, backticked, relative, ...). Worse, no assertion in the file covered the RELATIVE form. t1b's full pre-fix assertion set — all five substring checks plus `_leaked_paths` — passes a message reading "no template.yaml under data/deployed-templates/acme-payroll-agent", which discloses another user's deploy-local agent name exactly as the absolute form would. Two changes, both test-only: - Negative lookbehind `(?<![A-Za-z0-9_.\-/])` instead of a leading-whitespace anchor, so a `/` is "absolute" whenever it does not continue a relative path. `config/agent-templates/` stays clean (its `/` follows `g`); `/api/...` remains the one allowed `/`-rooted form. - `_SENSITIVE_ROOT_TOKENS` — naming either root is banned with or without a leading slash. `agent-templates` is deliberately NOT listed; it is the public, intentional remedy. Verified: all 9 leak shapes now caught, 0 false positives on the three real messages, and an end-to-end mutant that leaks `{template_path!r}` fails t1b. POSIX-only is deliberate, not an oversight — every path reaching these messages is a `pathlib.Path` built inside the Linux backend container, so a Windows branch would be dead regex. Also covers seam 2's CONTAINER branch, which had no test at all: every other test runs on a source checkout where `/agent-configs/templates` is absent, so only the repo-fallback branch was exercised. D1-A3's justification for widening the bind seam is "the delta in the verified container path is provably zero" — that claim now has a test. Pointing the module global at a real directory exercises the in-container branch without a container. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(templates): name the template in the partially-applied-config warning (#1759) deliberately kept a broad `except Exception` around the FIELD mutation that follows. That decision is right and is left standing: the file parsed, the agent does get its template files, and only some `config` mutations are skipped. Tightening it to a 400 would reject templates that deploy successfully today, which is outside this issue's acceptance criteria. What was not right is that the swallow was unfindable. The mutations run in order, so a raise part-way through leaves a PARTIALLY applied template — e.g. `credentials: "a string"` applies type/resources/tools and then silently skips mcp_servers/runtime/shared_folders. The message was `f"Error loading template config: {e}"`: it named neither the template nor the agent, so an operator holding a subtly-wrong agent had nothing to grep for. Log-only. No HTTP status, no control flow, and no config value changes. Adds a regression test that pins two things the swallow must never lose: 1. It stays a DEGRADE. The handler runs inside an `except`, so a bug in the handler itself converts a graceful degrade into a 500 on the create path. Proven load-bearing by mutation: replacing `config.name` with a non-existent attribute makes the test fail, and restoring it makes it pass. 2. The warning names both the template and the agent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(learnings): record the decorative-disclosure-guard class (#1759) The #1759 review found `_leaked_paths` — the backstop behind the fix's one binding security rule — missing 8 of 9 leak shapes, including the `!r` form those same messages already use. Records the durable rule so a future plan writes the adversarial table before trusting the guard. Same family as the #1644 "a guard that fails open manufactures confidence" and #1638 "a test asserting the defective behaviour ships it green" entries, one level up: a guard that never fires at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(templates): explain why the parents[4] fallback is unreachable (#1759) `_repo_local_templates_dir` indexes `parents[4]`, which the container layout (`/app/services/agent_service/crud.py`, 4 parents) cannot satisfy. Record why that is latent rather than live: the fallback runs only when `/agent-configs/templates` is missing, and both compose files always bind it on `backend`, so `_LOCAL_TEMPLATE_ROOTS[0]` resolves first. The defensive `len(parents) <= 4` guard is a flagged follow-up. Comment only — no behaviour change, so the shipped artifact stays byte-equivalent to the one /verify-local exercised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(submodule): drop accidental .claude pointer bump (#1759) The branch carried .claude at 3dbde4bf, a commit that exists only on an unmerged trinity-dev branch — pinning the public repo's submodule there would break 'git submodule update' for every clone if that branch is rebased or deleted. Reverts the pointer to de7fc716, the trinity-dev main tip that dev already points at. No other content change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Eugene Vyborov <1073874+vybe@users.noreply.github.com>
AndriiPasternak31
added a commit
that referenced
this pull request
Jul 28, 2026
…talog
One template whose `credentials:` — or `credentials.mcp_servers` — was a list,
a string, or **null** raised an uncaught AttributeError out of
`_build_local_template` -> `get_local_templates()` -> `GET /api/templates`:
HTTP 500 with ZERO templates listed. One bad template hid every good one.
The `github:` builder was worse: no `isinstance` guard at all on untrusted repo
metadata (`_fetch_template_yaml` returns `safe_load(...) or {}`, and a top-level
list is truthy). Deeply nested YAML escaped the parse handler entirely as
`RecursionError` — a `RuntimeError`, not a `yaml.YAMLError`.
Worse than the crash because it was silent: `env_file: "OPENAI_API_KEY"` — an
ordinary typo, the list dash forgotten — was iterated character by character
into the generated `.env`, emitting fifteen single-letter variables, never
writing the real credential, with no error, no warning and no crash. The agent
booted and its MCP server failed at first use with nothing pointing at the
cause.
And `credentials.config_files[].path` was joined onto the staging directory and
opened for write with no normalization, so an absolute path or a `..` escape
was an arbitrary-file-write primitive — reachable by any authenticated user,
since `deploy_local_agent_logic` accepts an uploaded template archive.
Four tolerant readers in `template_service.py` are now the only way into the
block, with two deliberately opposite contracts:
* Read paths never raise. The catalog degrades the derived field to empty,
attaches `credential_errors` to the entry, logs one WARNING naming the
template id — and the template STILL LISTS. `get_local_templates` also fences
each per-template build, so a future unguarded field cannot regress the
property the readers buy.
* The write path fails loud. `generate_credential_files` validates first and
raises the HTTP-free `CredentialDeclarationError`, which its only caller maps
1:1 to 400 `INVALID_CREDENTIAL_DECLARATION` (Invariant #1: services hold no
HTTP concerns). It is called before the docker try/except, so the 400 is not
flattened to a 500.
`config_files[].path` is rejected at the parse boundary AND re-checked at the
write sink (`crud._safe_cred_file_path` -> 400 `INVALID_CREDENTIAL_FILE_PATH`),
using the same resolve + `is_relative_to` CodeQL barrier as
`_safe_local_template_path`.
Absent / null / `{}` all stay a valid zero-credential contract — the ent#124
starter trio ships exactly that, and a commented-out block must not acquire a
spurious warning.
`credentials.env_file` stays a names-only list. The enriched per-variable
declaration lands under its own top-level key (PR-B) precisely so an older
Trinity reading a newer template is structurally untouched.
Tests: `test_ent128a_catalog_resilience.py` — 33 pass here, 31 fail on
`origin/dev`, the two headline ones with the exact bug signatures
(`AttributeError: 'list' object has no attribute 'get'` and the literal
`O=\nP=\nE=\nN=` output). Includes a parity test so a malformed BUNDLED template
fails CI instead of a user's fresh install.
No DB change, so the dual-track migration rule (#9) does not apply.
PR-A of trinity-enterprise#128 (AC #5). PR-B — the enriched declaration schema —
re-gates after this merges.
Refs Abilityai/trinity-enterprise#128
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Jul 29, 2026
…cold boundary (#1816) (#1867) * docs(system-agent): base-image adoption semantics for trinity-system (#1816) Rule #1 — documentation before implementation. - architecture.md: #1560 lifecycle-clearing wording ("never evaluated" → "never acted on"), the 3-state split, and the structural AC2 gate; the system_agent_service catalog entry. - feature-flows/internal-system-agent.md: startup diagram gains the drift branch, plus a Base-image adoption section carrying the convergence invariant, the three boundaries, the AC2 gate and the consequences (writable-layer loss, no TRINITY_BACKEND_URL, operator-triggered on the canonical upgrade path). - feature-flows/agent-lifecycle.md: 3-state core + boolean wrapper, system-aware capabilities predicate + recreate override, restart-policy carry-forward. - requirements/infrastructure.md: new 8.5b ADOPT-001..005. - feature-flows.md: Recent Updates row. Refs #1816 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(agents): 3-state base-image check + system-aware capabilities predicate (#1816) T1 — `check_base_image_matches` split into a 3-state `check_base_image_state` core (`match` | `drift` | `unknown`) plus an unchanged boolean wrapper (`state != "drift"`). Every WARNING is kept verbatim and the recreate path still consumes only the boolean, so #1809's behaviour is byte-identical: `unknown` and `match` both fail open to True. The 3-state exists because the staleness alarm this issue adds cannot be built on a boolean whose True means both "the image is current" and "the check could not run" — alarming on that would recreate the #1809 symptom one layer up. T2 — `check_full_capabilities_match` is system-aware. `trinity-system` runs FULL_CAPABILITIES by contract (package installation), not by the fleet default this predicate compares against, so pinning its `trinity.full-capabilities` label alone would, on any install with `agent_full_capabilities=false`, produce a mismatch that can never converge — a recreate on every start, forever. Both route through one shared `is_system_agent_name()` so the checker, the recreate override and the AC2 gate can never disagree. Deliberately a NAME test rather than `db.is_system_agent`: it must be unfailable (a DB error that flips this answer would either recreate the orchestrator or leak full capabilities) and it must not widen the exemption to any `is_system`-flagged row. Verified: tests/unit/test_1809_image_drift_recreate.py (19) plus test_start_agent_skip_inject, test_subscription_auto_switch_no_cred_import, test_inject_assigned_credentials, test_agent_readiness_probe, test_1560_breaker_cleared_on_lifecycle, test_1811_recovery_container_parity, test_base_image_allowlist, test_1484_create_agent_characterization — 102 passed. Refs #1816 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(system-agent): converge creation on the recreate path's contract (#1816) T3 + T-A. `_create_system_agent` left two of the eight config predicates PERMANENTLY false: * `check_agent_auth_token_env_matches` (#1159) — the env dict never wrote TRINITY_AGENT_AUTH_TOKEN; its only writers were crud.py and the two lifecycle recreates. * `check_full_capabilities_match` — the container runs with cap_add=FULL_CAPABILITIES but carried no `trinity.full-capabilities` label, and a missing label reads as 'false' against a fleet default of true. That is not cosmetic. `recreate_container_with_updated_config` resolves the image from the container's own Config.Image *tag*, so every config recreate is also an image adoption — a permanently-false predicate means the first `POST /api/agents/trinity-system/start` after any fresh provision replaces a RUNNING orchestrator and swaps its image mid-operation, which is precisely what AC2 forbids. (Convergent: a recreate writes both values, so only the first start was affected — which is why this survived so long.) The token derive is fail-closed on an unset AGENT_AUTH_SECRET, accepted deliberately: the install now fails to CREATE the system agent rather than creating one the backend can never talk to. ensure_deployed catches → `create_failed`, and main.py's lifespan catch keeps boot alive. Deliberately NOT added: TRINITY_BACKEND_URL. It gates the agent-side heartbeat loop, and authorize_heartbeat accepts only scope='agent' keys — the system agent's is scope='system', so arming it is a permanent 5s 403 loop. Pinned by an AST guard (key-level, not substring — the code documents the omission and a text search would fire on its own rationale). tests/unit/test_1816_system_agent_convergence.py drives `_create_system_agent` for real and builds the fixture from the `environment`/`labels` kwargs it actually passes to containers_run — a hand-built fake carrying both values would assert only that a correct container is correct. Red before this commit on ['agent_auth_token'] + both pins; 11 passed after. Refs #1816 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(agents): restart-policy carry-forward, capability override, AC2 gate (#1816) T4 — three lifecycle fixes the adoption path depends on: * **Restart policy is carried onto the replacement.** `old_host_config` was extracted at the top of `recreate_container_with_updated_config` and then never read, so `unless-stopped` silently vanished from EVERY recreated agent. `trinity-system` is created with it, so one recreate downgraded the platform orchestrator to "stays down after a crash or host reboot". `_provision_folders_and_run_agent_container` takes a keyword-only `restart_policy` and forwards it only when it names a policy, so every pre-#1816 caller is byte-identical. Read null-safely — the key can exist with a null value and `.get` on None would abort the recreate after the old container is already gone. * **`full_capabilities` override.** `None` (every existing caller) resolves to the fleet default for a regular agent and unconditionally True for trinity-system, via the same `is_system_agent_name` the predicate exempts on — writer and checker cannot disagree. * **No `TRINITY_BACKEND_URL` for the system agent.** It gates the agent-side heartbeat loop and `authorize_heartbeat` accepts only scope='agent' keys; the system agent's is scope='system'. #1816 makes this recreate a routine path for it, so arming it would newly create a permanent 5s 403 loop. T5 (AC2) — `start_agent_internal` gains the structural gate: a RUNNING trinity-system is never recreated, and the caller is told `recreate_deferred="system_agent_running"` rather than left to infer it. The gate covers the WHOLE `needs_recreation` block, not just the image predicate: the recreate resolves the image from a tag, so any predicate that fires is also an image adoption — gating one would leave AC2 open through the other eight. Surfaced through `routers/agents.py`'s whitelisted response dict and its audit details (a field added to the internal dict alone dies at the router — #1809's own learning). Two harness stubs updated: a bare `Mock()` auto-creates `is_system_agent_name` returning a truthy Mock, which reads as "every agent is the system agent" and silently suppressed every recreate in test_start_agent_skip_inject. Verified per-file (all green): test_start_agent_skip_inject 9, test_1809_image_drift_recreate 19, test_1816_system_agent_convergence 11, test_agent_readiness_probe 5, test_inject_assigned_credentials 8, test_subscription_auto_switch_no_cred_import 1, plus the 12 other helpers-stubbing files (91 tests). A cross-file sys.modules ordering flake in TestCheckBaseImageMatches (9 tests) reproduces IDENTICALLY on pristine origin/dev @64845458 — pre-existing, not a regression. Refs #1816 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(system-agent): adopt a rebuilt base image at the cold boundary (#1816) The core of the issue. `ensure_deployed` returned `action: none` the instant the container reported `running`, without evaluating a single drift predicate. Combined with `restart_policy: unless-stopped` and a canonical upgrade path (build-base-image.sh → start.sh) that never touches agent containers, that made the platform orchestrator the most-stale agent in every fleet — indefinitely, and silently. T6 — three boundaries, honestly separated: * RUNNING → READ-ONLY. Reports `base_image_state` (`current`|`stale`|`unknown` — an enum only, never image ids, mirroring the /health clone_status contract #1439), WARNs naming the remedy, and never recreates. Source-pinned by slicing the branch between two named anchors, so a refactor that merely relocates a recreate call cannot pass. * STOPPED → delegates to `start_agent_internal` instead of a bare `container_start`. This is the cold boundary where the #1809 image gate fires, and it inherits #1560 clear-before-recreate ordering, the 409/NotFound race hardening, the post-recreate handle re-lookup and every future predicate — rather than forking a second lifecycle for one agent, which is the bug class that produced this issue. * no container → unchanged. Every early return sets both `action` and `message`: main.py's lifespan indexes them directly, so an omission would raise KeyError inside the boot log line. T7 — AC1 on the canonical upgrade path. Since the system agent is RUNNING after every canonical upgrade, the read-only branch is the one that fires, so detection without notification would tell no one. An edge-triggered operator-queue alarm follows the sync_failing/git_bloat idiom (reserved id prefix registered in #1632's anti-spoof guard, priority high, 6h cooldown, emit-failure-safe) and is raised on `stale` ONLY — a fail-open probe must never manufacture an alert, which is exactly why the 3-state split exists. R1 pre-flight: a recreate REMOVES the old container before running the replacement, so a run failure leaves the platform with no orchestrator. When the agent network is missing or the ssh port is bound, the adoption is declined and a plain start runs instead (costing one stale boot — the pre-#1816 status quo — rather than the orchestrator). Fail-open on an unreadable probe; a start failure raises a critical alarm. T8 — /restart delegates (an explicit stop makes it a cold start, so it is the operator's remedy for the alarm) and re-fetches the container for its response, because a recreate replaces the handle. /status gains the 3-state `base_image_state`, reported only while running. /reinitialize deliberately unchanged, pinned by a test. T10 — 49 behavioural cases + the source pins, and the test_1560 create-path pin repaired: `str.index` is first-occurrence, so it stayed green while silently ceasing to pin `_create_system_agent` the moment anything above it grew a `clear_agent_breakers(SYSTEM_AGENT_NAME)` call. Proven by simulation — the old assertion PASSES with the create-path clear removed, the repaired one fails with "the clear must live INSIDE _create_system_agent". Verified: test_1816_system_agent_adoption 49, test_1816_system_agent_convergence 11, test_1560_breaker_cleared_on_lifecycle 16, test_1632_operator_queue_caps 49 — 118 passed. tests/lint_sys_modules.py: no new violations. Refs #1816 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(agents): name the shared helper correctly (is_system_agent_name) (#1816) The docs written ahead of implementation used `is_system_agent()`, which collides conceptually with the existing DB-backed `db.is_system_agent`. Names the real helper and states why it is a name test rather than the DB one. Refs #1816 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(tests): sync the test catalog and the two remaining feature flows (#1816) /update-tests + /sync-feature-flows tail steps. - .claude/agents/test-runner.md: both new unit files catalogued under Operations & Observability, a dated Recent Test Additions block (including the test_1560 pin repair and the two harness-stub fixes), and the totals. - feature-flows/async-docker-operations.md: the new `network_get` wrapper. - feature-flows/operating-room.md: `base-image-stale-` registered in the reserved-prefix enumeration (all 3 sites) and named in the platform-create exemption list. agent-lifecycle.md and internal-system-agent.md were already synced in the docs-first commit; feature-flows.md index is 430 lines. Refs #1816 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(system-agent): make the state-label map public, merge the duplicate gate (#1816) Self-review polish, no behaviour change: - `_BASE_IMAGE_STATE_LABELS` is consumed by `routers/system_agent.py`, so a leading underscore was wrong — renamed `BASE_IMAGE_STATE_LABELS`. - `get_system_agent_status` had two consecutive `if status == "running":` blocks; folded the health fetch into the first. It keeps its own try/except, so `base_image_state` is still set when the agent is unreachable. Verified: test_1816_system_agent_adoption + test_1816_system_agent_convergence — 60 passed. Refs #1816 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(system-agent): pass the container into the adoption pre-flight (#1816) The stopped branch already holds the handle; re-fetching it inside `_preflight_ok_for_delegated_start` was a wasted Docker round-trip on the boot path and a needless TOCTOU window. Still null-safe throughout, so a partially populated handle degrades to "no port to check" rather than raising. Verified: 60 passed (adoption + convergence). Refs #1816 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(system-agent): make the #1816 suites CWD-independent (#1816) Caught by verify-local, not by any local run: `_create_system_agent` resolves its template through a CWD-RELATIVE fallback (`./config/agent-templates`, used whenever `/agent-configs/templates` is absent, i.e. off-container), and verify-local runs pytest from `tests/`. From there creation died on a missing template BEFORE reaching the token derive, so `test_creation_without_agent_auth_secret_fails_closed_without_blocking_boot` was asserting the wrong failure — green from the repo root, red from `tests/`. Both suites now pin the CWD to the repo root with the reason stated, so they hold wherever pytest is invoked from. Verified from BOTH cwds: 60 passed each. A test bug, not a code bug — the behaviour under test is unchanged. Refs #1816 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(system-agent): state the staleness alarm's per-worker reach (#1816) `ensure_deployed` runs once per worker lifespan and the cooldown cursor is per-process, so a stale boot files one operator-queue item per worker (`--workers 2` ⇒ 2). Deliberate rather than overlooked — a cross-worker cursor puts Redis or a DB read on the boot path for an advisory alarm, and the un-guessable timestamped id (what stops an agent pre-creating and silencing it, per #1632) is inherently undedupable by `on_conflict_do_nothing`. Said plainly in both the code and the flow doc rather than left for a reviewer to discover. Refs #1816 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(system-agent): fence the delegated start, gate the operator remedy (#1816) Review + CSO follow-ups on the base-image adoption work. - recreate_missing_container refuses trinity-system (409). start_agent_internal falls through to it when the container lookup returns None, and #1816 newly reaches that from the boot path and /restart — ensure_deployed runs in every uvicorn worker with no leader lock, so a concurrent recreate can null the lookup mid-flight. That path reconstructs a REGULAR agent: it deactivates the system-scoped MCP key and mints an agent-scoped one (plaintext unrecoverable, so the orchestrator irreversibly loses its permission bypass), drops trinity.is-system, the /template bind and unless-stopped, and arms the scope-403 TRINITY_BACKEND_URL. ensure_deployed's create branch rebuilds it correctly on the next boot; the race itself is #1817. - /restart and /reinitialize are human-only. assert_admin rejects connector principals but not agent ones, and get_current_user hands an agent-scoped key its owner's role — so on a default admin-owned install any non-ephemeral agent's TRINITY_MCP_API_KEY passed it. Tolerable while /restart was a stop+start; not once it replaces the container. trinity-ops-agent#232 precedent; no-op for JWT / user-scoped / system-scoped callers. - Sanitize the start-failure alarm's interpolated exception string. It lands in operator_queue.question — durable, operator-visible state — from a path that builds env dicts holding OAuth tokens and PATs. The staleness alarm was built to carry no identifiers; this one wasn't. - Bound that alarm across processes via a bucketed id. A per-process cursor cannot bound a failure whose symptom is a fresh process, and retention never deletes a pending row. A bucket (not a fixed id) so Clear All → cancelled can't wedge it shut through on_conflict_do_nothing; guessability is fine because the prefix is in _RESERVED_ID_PREFIXES. - Cooldown cursor uses time.monotonic() — datetime.utcnow() is deprecated and a wall-clock step could skip or extend the gate. - State the real boot cost in the comment (~20-90s of blocked lifespan, not one timeout) and name the multi-worker exposure. 9 tests; 5151 unit tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(system-agent): sync architecture.md with the review-pass fences (#1816) The last commit added two fences but only reached requirements + the feature flow. architecture.md is the current-design doc and carries the invariant, so both belong here too. - system_agent_service entry: recreate_missing_container refuses trinity-system (409, ADOPT-006). Worth stating where the service is described, because the reader's natural assumption is that the generic recovery rebuild is a valid way to bring the orchestrator back — it is not, and the downgrade it causes (system-scoped MCP key deactivated for an agent-scoped one) is irreversible. - Invariant #8 gains "Role ≠ human": assert_admin/require_admin answer what role, never is-this-a-human, because get_current_user resolves an agent-scoped MCP key to its owner carrying the owner's role. Generalized from the two known instances (#1644 retention ack, #1816 /restart + /reinitialize) into the rule that produced them — an endpoint whose blast radius is operator-scale needs reject_agent_principal in ADDITION to the role gate, and the trigger to revisit an existing gate is a change in what the endpoint DOES. Escalating a handler's destructiveness silently re-prices every principal that could already reach it. Refs #1816 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(system-agent): pin the real modules against sibling sys.modules leaks (#1816) The adoption suite passed alone and in the full alphabetical run, but 21 of its 67 tests failed when collected alongside tests/unit/test_start_agent_skip_inject.py — i.e. it was green by luck of import order, and CI randomizes. Five sibling unit files replace services.agent_service / .helpers with Mock objects in sys.modules at COLLECTION time (the grandfathered #762 class). This file resolves the real modules lazily, inside fixtures — after the contamination. The docstring already noted the file is not a contaminator; not being one is not the same as being immune to one. The failure mode is silent rather than loud, which is the reason to fix it rather than order the files: a leaked helpers Mock makes is_system_agent_name return False for everything, so the AC2 gate never fires and the recreate it exists to suppress happens — while every assertion still looks meaningful. check_base_image_state degrades the same way (a Mock can never return "unknown", so the never-alarm-on-unknown property becomes untestable). Fix: import the real modules at collection time (this file sorts before all five) and pin them per-test with monkeypatch.setitem, which self-restores — so the siblings' own harnesses, which hold direct module references rather than sys.modules lookups, are untouched. No new lint_sys_modules violations (monkeypatch.setitem is the sanctioned form). Reproduction now green: adoption + skip_inject 21 failed -> 0; the original 5-file set 21 failed -> 108 passed. Refs #1816 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
vybe
pushed a commit
that referenced
this pull request
Jul 29, 2026
…1804) (#1863) * docs(requirements): CAS-won terminal owns the paired activity close (#1804) Trinity Rule #1 — requirements before implementation. scheduling.md §10.15: the terminal-write contract now includes closing the paired agent_activities dispatch row. Records the one-owner helper, the activity CAS lattice (mirrors the execution predicate), the widened lookup, the bulk/per-row split by cardinality, and the terminal-write-anchored parity guard. infrastructure.md §12.9 (CLEANUP-001): the 120-minute activity sweep is demoted to a backstop for the unclaimed, and moves after the stale-slot reaper so it can no longer beat a legitimate closer within one cycle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(activities): make the activity close a lattice CAS with a tri-state outcome (#1804) T1+T2. The db-layer spine for the #1804 contract. - ActivityCloseOutcome (UPDATED / ALREADY_CLOSED / NOT_FOUND). One bool cannot answer both "did this row exist" (routers/internal.py 404s) and "did anything change" (activity_service broadcasts); once idempotent no-op closes are a designed outcome the two answers diverge routinely. - complete_activity becomes an atomic CAS on _close_predicate, which MIRRORS the execution-row predicate in db/schedules/executions.py: incoming COMPLETED -> activity_state != 'cancelled' (an authoritative close may upgrade a provisional FAILED — the #1083 late-SUCCESS-after-lease-expiry path); incoming CANCELLED/FAILED -> activity_state = 'started' (nothing overwrites an authoritative close, so a double close cannot clobber completed_at / duration_ms / error). The lattice diagram is inline over the predicate. - get_open_activity_id_for_execution takes include_failed so the LOOKUP agrees with the write — a lookup narrower than the CAS makes the widened predicate inert. Ordering prefers still-'started' rows. - close_open_activities_for_executions: set-wise bulk close for the watchdog sweeps, one transaction, no per-row WS, chunked at _SQLITE_MAX_IN_VARS. No schema change, no migration (Rule #9 not triggered). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(activities): add close_execution_activity, the single owner of the close contract (#1804) T3. A leaf helper on activity_service (imports only models/database/db.activities — no services.* edge, so no cycle) that every CAS-won terminal writer calls. - Reuses models.activity_state_for_terminal (#1332) — no second mapping. - Lattice-aware lookup: an authoritative terminal (SUCCESS/CANCELLED) searches started|failed so it can upgrade a provisional FAILED; a provisional terminal searches started only. Callers pass a terminal status and stay ignorant of it. - Delegates to complete_activity, so the agent_activity WebSocket broadcast and subscriber notify survive (what a db-layer close would have lost). - Broadcasts only on ActivityCloseOutcome.UPDATED; ALREADY_CLOSED is a designed no-op and must not emit an event claiming the activity just closed. - complete_activity's bool now means "exists and closed" — only NOT_FOUND is False, so routers/internal.py keeps its 404 semantics and an idempotent re-close does not start 404ing the scheduler. - spawn_close_execution_activity: sync fire-and-forget wrapper for the synchronous pull sink, mirroring spawn_task_terminal_event (strong task ref, fail-open with no running loop). Fail-open throughout: the close runs after a committed terminal write. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(execution): close the paired activity at every CAS-won terminal writer (#1804) T4-T8. Wires the eight terminal writers onto the single owner. task_execution_service: - _write_terminal_and_gate gains the missing CAS-LOSS branch (the SUCCESS applier has had one since #1332): re-read the row and close the activity in the terminal that actually stands. `activity_status` leaves the signature — it is derived from `status`, so the two can no longer drift. - the asyncio.CancelledError shutdown handler now closes too. #767 closed the execution record here so the sweep would not inflate ITS duration but left the activity for the 120-minute backstop to inflate instead; worse, the row is `failed`, so startup recovery (which scans `running`) skips it forever. routers/internal: the second shutdown writer, same shape (no activity_id in scope — the helper looks it up). cleanup_service: - watchdog and startup _recover_execution close on the CAS-won branch. The startup one was DISCARDING its CAS bool (returned True unconditionally absent an exception); it is now captured, gates the close, and is returned. - _close_bulk_swept_activities: a SIBLING of _emit_bulk_terminal_events, never folded into it — that method short-circuits when nobody subscribes, which would skip the close on every install with no event subscribers. Driven by the collect_failed rows #1714 already collects; one transaction, no per-row WS. - _sweep_stale_activities moves AFTER _sweep_stale_slots: it ran one line before the reaper that legitimately closes activities, so within a single cycle the 120-minute fabricator could beat a real closer. - _close_reaped_activity / _close_stale_slot_activity delegate to the helper. - CleanupReport.activities_closed_on_recovery (not summed into `total` — it is an observability counter over work already counted). pull_coordination_service: closes on the CAS-won branch via the sync spawn wrapper. lease_reaper_service: requeued_execution_ids, closed CANCELLED — a superseded attempt is not a failure, and the re-queue preserves execution_id so the next delivery opens a second activity against the same row. chat_execution_service: terminate folds onto the helper (it was the fifth hand-rolled copy of the idiom, and a copy is invisible to the parity guard). Behaviour unchanged — R4. db/schedules/cleanup: mark_no_session_executions_failed returns the CAS-WON count, not the candidate count (its sibling already did). report. no_session_executions was over-reporting; deliberate, visible value change. Tests updated where a mock encoded the old call surface; one behavioural assertion inverted on purpose (test_terminal_write_cas_gate: a lost CAS now closes the activity in the persisted state instead of leaving it open). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(1804): R1-R5 regressions, the two-table chain test, and the parity guard T9+T10. test_1804_recovery_closes_activity.py — db layer against the real schema (db_harness) + service + every wired writer: R1 a FAILED activity upgrades to COMPLETED, through the lookup (the pairing that keeps the fix from being inert) R3 an already-closed close never touches completed_at/duration_ms/error R2 _write_terminal_and_gate's CAS-loss closes with the PERSISTED state R5 both backend-shutdown CancelledError handlers close their activity plus: broadcast on UPDATED only, watchdog/startup gating on the CAS bool, the bulk close running with zero event subscribers (the #1714 gate scopes the event, never the close), the pull sink on applied-but-not-replayed/ conflict, and the lease reaper's requeued_execution_ids. test_1804_terminal_activity_chain.py — the two CAS predicates live in two different tables; mocks encode what the author believed, so these run the real statements and assert the tables agree. Includes the late-SUCCESS upgrade end to end and the cancel-is-never-overwritten mirror image. test_1804_terminal_activity_parity.py — anchored on terminal WRITES, not on completion-event emission. The emit set is a strict subset (both shutdown writers emit nothing), which is exactly how they hid from review. Explicit allowlist, each entry justified by lifecycle position, plus a self-test that the guard actually fires and a staleness check on the allowlist. R4 (terminate byte-identical) lives in test_1332, which the previous commit updated to the new call surface. tests/lint_sys_modules.py: clean, no new violations. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(architecture): record the terminal-activity close contract (#1804) New canonical block "Terminal-Activity Close Contract (#1804)" — one home for the feature, pointers from the places that used to imply the old model: - Fire-and-Forget Dispatch (#1083): the close is a property of winning the CAS, not of holding the activity_id local; apply_result is one writer among eight. - Task Completion Events (#1578): the emit set is NOT the close set — both shutdown handlers write a terminal and emit nothing, which is why the parity guard is anchored on terminal writes. - Background Services / Cleanup Service: the recovery closes, activities_closed_on_recovery, and the backstop moving last in the cycle. - services catalog: activity_service owns the closer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(feature-flows): the terminal-activity close contract (#1804) /sync-feature-flows. activity-stream.md — new "The Close Contract (#1804)" section: the owner (close_execution_activity + the sync spawn wrapper), the state lattice with the side-by-side execution/activity diagram and why the lookup must agree with the CAS, the split by cardinality, the full writer table, and the demotion of the 120-minute backstop. task-execution-service.md — the _write_terminal_and_gate lost-CAS branch (and the dropped activity_status param), the backend-shutdown close, and a row in the Activity Tracking table pointing at the contract. dashboard-timeline-view.md — why the amber bar used to lie, and that no frontend change was needed: ReplayTimeline was reading a lying database. feature-flows.md — Recent Updates row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(1804): make the close outcome identity-stable and never downgrade a recovery Two real defects the full-suite run surfaced (both green in isolation, red at 5,100 tests — the pollution class the dossier warned about). 1. ActivityCloseOutcome moves db/activities.py -> models.py. Callers compare it by `is`, and db_harness evicts `db.activities` from sys.modules per test. Any test that imported services.activity_service BEFORE an eviction held one enum class while a later `from db.activities import ...` got a different one, so every identity check silently went False. models is the leaf everything imports and nothing re-imports, so the object stays the same one — and it now sits beside ActivityState/TaskExecutionStatus, where a shared db↔service contract type belongs. Re-exported through db.activities. 2. Both _recover_execution closes get their own try/except. The terminal write and the capacity release have already succeeded at that point; letting a close failure propagate flipped a recovered row into the `errors` bucket and would make the watchdog retry a row it had already recovered. Fail-open is the rule for everything after a committed terminal — this was the one place I left it implicit. Full unit suite: 5136 passed, 14 skipped, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(1804): the #1083 result callback must look up failed activities too Self-review catch. The callback endpoint IS the late-SUCCESS path — the one the lattice was widened for. A lease reaper that beat the callback has already FAILED the row and closed the activity FAILED; the execution CAS deliberately lets a genuine late SUCCESS correct the row ("a FAILED row falls through so a late SUCCESS can still overwrite a reaper LEASE_EXPIRED"). With a started-only lookup the callback passed activity_id=None, apply_result closed nothing, and the pair settled at execution=success, activity=failed — permanently. That is #1804 inverted, on the exact path the upgrade exists for. Harmless for a FAILED callback: the close CAS refuses to overwrite an already-closed activity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(activity-stream): note the callback's include_failed lookup (#1804) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: confine the git-reset loader's sys.modules stubs to their own test `_load_git_service` installs Mocks under the real names `database`, `services.docker_service` and `services.activity_service`. It deliberately leaves them in `sys.modules` after the loader returns (the reset path imports `activity_service` lazily at call time) — but nothing put them back afterwards either, so the Mocks stayed installed for the rest of the session. That is a silent, ordering-dependent landmine for any later test that lazily imports one of those names. #1804 makes `cleanup_service._close_stale_slot_activity` delegate to `activity_service.close_execution_activity`; with this file running first, `test_1083_lease_reaper.py::test_swallows_errors` awaited the leaked Mock and died with `TypeError: object Mock can't be used in 'await' expression` — a real red under CI's pytest-randomly seeds, in a test unrelated to git reset. Adds the lint-recognised `_STUBBED_MODULE_NAMES` + `_restore_sys_modules` autouse pair, also restoring `services.git_service*` (the loader force-reimports it against the Mocks, so leaving that behind poisons later importers too). `tests/lint_sys_modules.py` drops this file from 9 violations to 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(1804): close the reflexive COMPLETED edge and make CAS-loss labels honest Three review findings on the terminal-activity close contract. 1. The close lattice inherited the upstream's permissive edge (the bug it exists to prevent). `_close_predicate` was specified as an authority ORDERING (`started < failed < {completed, cancelled}`) but implemented as an EXCLUSION copied literally from the execution CAS: `!= 'cancelled'`. Those two disagree exactly on the reflexive case — `!= 'cancelled'` matches `'completed'`, so a second COMPLETED close was permitted and re-dated `completed_at`/`duration_ms`/`error`. Executed against the real path, a 15-minute activity (`duration_ms=900000`) re-closed COMPLETED became ~9h: the exact fabricated-duration symptom #1804 exists to eliminate, arriving through #1804's own fix. It is reachable from `_write_terminal_and_gate`'s lost-CAS branch, which passes an explicit `activity_id` — so the narrower `started|failed` lookup cannot shield it. The literal copy was faithful: `update_execution_status`' SUCCESS predicate really is `!= CANCELLED` and really does admit `success -> success`. The execution row survives that edge only because an upstream guard (the #1083 callback's authoritative-terminal replay short-circuit) stops the duplicate before it reaches the CAS — a protection the activity's new call sites do not have. The predicate now states the ordering literally, `IN ('started','failed')`, and the branch's five lattice tests gain the `X -> X` case they all skipped because it looks like a no-op. 2. A CAS loss to a *successful* row stamped "superseded by ..." onto a clean success, and rendered the enum repr while doing it. `error` is now None on an authoritative SUCCESS close (a non-NULL `error` on a `completed` activity reads as a problem in every activity-derived view — the rule the shipped terminate path already follows), and the label uses `.value`, since `f"{TaskExecutionStatus.FAILED}"` renders as `TaskExecutionStatus.FAILED` on 3.11+ — the `str, Enum` footgun #1578 already paid for. 3. `_recover_execution` returning the terminal CAS bool (it must, to gate the close) changed what a False MEANS to `recover_orphaned_executions`, which counted every False as `errors`. A lost CAS is RELIABILITY-005's guarded writer working as designed — a real completion landed during restart — so folding it into `errors` makes a healthy restart race read as a failing one, and would have made this fix look like it introduced failures. False is now partitioned: genuine exceptions self-count into `stats["errors"]`, the remainder is reported as `cas_lost`. Docs updated in lockstep (architecture.md, requirements/scheduling.md, feature-flows/activity-stream.md) to state the COMPLETED arm as deliberately tighter than the execution CAS rather than a literal mirror, plus a learnings.md entry on inheriting a permissive edge when mirroring a predicate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
AndriiPasternak31
added a commit
that referenced
this pull request
Jul 29, 2026
…talog
One template whose `credentials:` — or `credentials.mcp_servers` — was a list,
a string, or **null** raised an uncaught AttributeError out of
`_build_local_template` -> `get_local_templates()` -> `GET /api/templates`:
HTTP 500 with ZERO templates listed. One bad template hid every good one.
The `github:` builder was worse: no `isinstance` guard at all on untrusted repo
metadata (`_fetch_template_yaml` returns `safe_load(...) or {}`, and a top-level
list is truthy). Deeply nested YAML escaped the parse handler entirely as
`RecursionError` — a `RuntimeError`, not a `yaml.YAMLError`.
Worse than the crash because it was silent: `env_file: "OPENAI_API_KEY"` — an
ordinary typo, the list dash forgotten — was iterated character by character
into the generated `.env`, emitting fifteen single-letter variables, never
writing the real credential, with no error, no warning and no crash. The agent
booted and its MCP server failed at first use with nothing pointing at the
cause.
And `credentials.config_files[].path` was joined onto the staging directory and
opened for write with no normalization, so an absolute path or a `..` escape
was an arbitrary-file-write primitive — reachable by any authenticated user,
since `deploy_local_agent_logic` accepts an uploaded template archive.
Four tolerant readers in `template_service.py` are now the only way into the
block, with two deliberately opposite contracts:
* Read paths never raise. The catalog degrades the derived field to empty,
attaches `credential_errors` to the entry, logs one WARNING naming the
template id — and the template STILL LISTS. `get_local_templates` also fences
each per-template build, so a future unguarded field cannot regress the
property the readers buy.
* The write path fails loud. `generate_credential_files` validates first and
raises the HTTP-free `CredentialDeclarationError`, which its only caller maps
1:1 to 400 `INVALID_CREDENTIAL_DECLARATION` (Invariant #1: services hold no
HTTP concerns). It is called before the docker try/except, so the 400 is not
flattened to a 500.
`config_files[].path` is rejected at the parse boundary AND re-checked at the
write sink (`crud._safe_cred_file_path` -> 400 `INVALID_CREDENTIAL_FILE_PATH`),
using the same resolve + `is_relative_to` CodeQL barrier as
`_safe_local_template_path`.
Absent / null / `{}` all stay a valid zero-credential contract — the ent#124
starter trio ships exactly that, and a commented-out block must not acquire a
spurious warning.
`credentials.env_file` stays a names-only list. The enriched per-variable
declaration lands under its own top-level key (PR-B) precisely so an older
Trinity reading a newer template is structurally untouched.
Tests: `test_ent128a_catalog_resilience.py` — 33 pass here, 31 fail on
`origin/dev`, the two headline ones with the exact bug signatures
(`AttributeError: 'list' object has no attribute 'get'` and the literal
`O=\nP=\nE=\nN=` output). Includes a parity test so a malformed BUNDLED template
fails CI instead of a user's fresh install.
No DB change, so the dual-track migration rule (#9) does not apply.
PR-A of trinity-enterprise#128 (AC #5). PR-B — the enriched declaration schema —
re-gates after this merges.
Refs Abilityai/trinity-enterprise#128
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Jul 30, 2026
…talog (ent#128 PR-A) (#1835) * fix(templates): stop a malformed `credentials:` block emptying the catalog One template whose `credentials:` — or `credentials.mcp_servers` — was a list, a string, or **null** raised an uncaught AttributeError out of `_build_local_template` -> `get_local_templates()` -> `GET /api/templates`: HTTP 500 with ZERO templates listed. One bad template hid every good one. The `github:` builder was worse: no `isinstance` guard at all on untrusted repo metadata (`_fetch_template_yaml` returns `safe_load(...) or {}`, and a top-level list is truthy). Deeply nested YAML escaped the parse handler entirely as `RecursionError` — a `RuntimeError`, not a `yaml.YAMLError`. Worse than the crash because it was silent: `env_file: "OPENAI_API_KEY"` — an ordinary typo, the list dash forgotten — was iterated character by character into the generated `.env`, emitting fifteen single-letter variables, never writing the real credential, with no error, no warning and no crash. The agent booted and its MCP server failed at first use with nothing pointing at the cause. And `credentials.config_files[].path` was joined onto the staging directory and opened for write with no normalization, so an absolute path or a `..` escape was an arbitrary-file-write primitive — reachable by any authenticated user, since `deploy_local_agent_logic` accepts an uploaded template archive. Four tolerant readers in `template_service.py` are now the only way into the block, with two deliberately opposite contracts: * Read paths never raise. The catalog degrades the derived field to empty, attaches `credential_errors` to the entry, logs one WARNING naming the template id — and the template STILL LISTS. `get_local_templates` also fences each per-template build, so a future unguarded field cannot regress the property the readers buy. * The write path fails loud. `generate_credential_files` validates first and raises the HTTP-free `CredentialDeclarationError`, which its only caller maps 1:1 to 400 `INVALID_CREDENTIAL_DECLARATION` (Invariant #1: services hold no HTTP concerns). It is called before the docker try/except, so the 400 is not flattened to a 500. `config_files[].path` is rejected at the parse boundary AND re-checked at the write sink (`crud._safe_cred_file_path` -> 400 `INVALID_CREDENTIAL_FILE_PATH`), using the same resolve + `is_relative_to` CodeQL barrier as `_safe_local_template_path`. Absent / null / `{}` all stay a valid zero-credential contract — the ent#124 starter trio ships exactly that, and a commented-out block must not acquire a spurious warning. `credentials.env_file` stays a names-only list. The enriched per-variable declaration lands under its own top-level key (PR-B) precisely so an older Trinity reading a newer template is structurally untouched. Tests: `test_ent128a_catalog_resilience.py` — 33 pass here, 31 fail on `origin/dev`, the two headline ones with the exact bug signatures (`AttributeError: 'list' object has no attribute 'get'` and the literal `O=\nP=\nE=\nN=` output). Includes a parity test so a malformed BUNDLED template fails CI instead of a user's fresh install. No DB change, so the dual-track migration rule (#9) does not apply. PR-A of trinity-enterprise#128 (AC #5). PR-B — the enriched declaration schema — re-gates after this merges. Refs Abilityai/trinity-enterprise#128 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(templates): make the credential-file path guard a two-step barrier CodeQL flagged `_safe_cred_file_path` with two HIGH `py/path-injection` alerts, on the join itself and on the containment check. It was right to: the helper had only step 2 of the pattern (resolve + `is_relative_to`), and CodeQL does not treat that alone as a barrier — the sibling `_safe_local_template_path` clears the same query because it allowlists the RAW string first. Add that step 1 here too: reject empty, absolute, `..`-bearing, and anything outside `[A-Za-z0-9._/-]` before the value ever reaches the join, then keep the resolve + containment as step 2. `_CRED_FILE_PATH_RE` permits `/` (unlike `_LOCAL_TEMPLATE_NAME_RE`) because this is a relative *path*, not a single slug. Not defensive padding for a scanner — the two-step shape is what makes the guard legible to a reader as well, and it is the established pattern in this file. Test extends to the allowlist rejections (empty, leading dash, space, semicolon) alongside the traversal cases. Refs Abilityai/trinity-enterprise#128 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
dolho
added a commit
that referenced
this pull request
Jul 30, 2026
…ent#180)
An exposed agent's card advertises every `template.yaml capabilities[]` tag, and
the well-known discovery route is unauthenticated by design — so today that full
capability list is world-readable for any exposed agent. This lets an operator
choose what the outside is told.
**A disclosure control, and the code says so.** The card's `skills[]` is
advertisement: inbound `message/send` dispatches free-form text via
`execute_task(triggered_by="a2a")`, with no per-skill routing. Filtering changes
what an orchestrator SEES, never what it may ASK for. That's stated in the
requirement, the seam docstring and the filter itself, because a filter
operators mistake for an invocation gate is a control that looks like security
and isn't. A real boundary (constraining a2a-triggered runs via
allowed_tools/guardrails) is separate work with its own threat model.
Extends the existing ent#157 seam rather than adding a module — same shape as
the inbound allow-list, second provider:
provider.exposed_skills(agent_name) -> Optional[List[str]]
- No provider (OSS) → identity function; the card is byte-identical to before,
by construction. The enterprise module owns the config, storage and UI.
- `None` = no opinion = advertise all: the unconfigured default, so exposure
(already opt-in, default OFF) keeps every existing card unchanged on upgrade.
- `[]` ≠ `None`: an explicit "advertise nothing".
- Stale ids are inert — the selection only subtracts; `template.yaml` stays the
source of truth for what exists.
- Fail-open on provider error (advertise all + WARNING): consistent with the
seam's availability bias and the advertise-all default. Honest only *because*
this isn't a security boundary — failing closed would silently empty a card
and break discovery invisibly.
Both card surfaces (public well-known + authenticated per-agent) go through one
router helper, so they can't disagree and a future third surface gets the filter
by default rather than by remembering. `generate_a2a_card` stays pure — the
provider lookup lives in the helper.
Writing the tests found a real gap: a provider returning a str (a defect) would
iterate into single characters, match no id, and silently empty the card —
fail-CLOSED, the opposite of the contract, and invisible. Malformed returns now
take the same fail-open path as a raised error.
Requirements §32.4 written before the code (CLAUDE.md rule #1); public docs
describe the generic seam only, per the standing enterprise-docs rule.
Related to trinity-enterprise#180
vybe
pushed a commit
that referenced
this pull request
Jul 31, 2026
….8 amend + new §9.9 (ent#260) Requirements-first (Rule #1): records the third dashboard mode (Timeline / Grid / List), the 28-item parity disposition, chassis filter/create migration, the /agents → /?view=list one-shot non-persisting redirect, NavBar consolidation, and the zero-backend-change / N+1-deletion performance shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Jul 31, 2026
…cator (ent#264) New §15.1h: reaction ack + elapsed-time placeholder + channel-agnostic start/progress/resolve seam, group gating (mention/reply OR all-mode; observe stays silent), degradation ladder, default-ON per-binding toggle, GET /telegram access hardening. §15.1c/§15.1e touch-ups. Requirements-first per Rules of Engagement #1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Jul 31, 2026
…ote updated (ent#261) Trinity Rule #1: requirements before implementation. §9.10 specifies the / hotkey filter across Timeline/Grid/List — store-seam predicate, pre-query node invariant, pill honesty, Esc layering, chassis query-empty overlay, kbd hint, list-mode composition, and the deliberate timeline owner-filter behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndriiPasternak31
added a commit
that referenced
this pull request
Jul 31, 2026
…metadata Closes ent#128 AC #1-2. A template can now describe each credential an operator must supply — title, description, required, secret, format, setup_url, default — and `template_service` surfaces the normalized result as `credential_requirements` on every catalog entry. **Enrichment lives in a NEW sibling top-level key; `credentials:` is FROZEN as names-only, forever.** An already-deployed older Trinity reads `env_file` through `credential_env_file_names` and then does `agent_credentials.get(var_name, "")` — hand it a list of mappings and that is `TypeError: unhashable type: 'dict'` at the moment it writes the agent's `.env`. A sibling key is structurally invisible to that binary, so there is no floor version and enrichment distributes immediately. **Base-set-plus-overlay, so the two keys cannot drift.** One record per variable `credentials:` declares, decorated by `credential_setup:` entries joined BY NAME. An entry naming nothing is a named three-line error (problem, cause, FIX) and is dropped; valid siblings survive. `credential_setup:` can only ever decorate — the sibling-key shape's usual failure mode is closed by construction, not by discipline. Stated honestly: for an EXTERNAL template that error is neither impossible nor visible in the UI — `credential_errors` has zero frontend and zero MCP consumers, so the only human channel is the backend log. It is LOGGED. `required` is a tri-state. Enriched-and-omitted means `True` (an author who described a variable meant it); a legacy bare `- FOO` is `"unknown"`, never `True` — it carries no authorial intent, and reading it as required makes a guided checklist cry wolf. `"unknown"` doubles as the enriched/un-enriched discriminator, which is why no `enriched: false` flag is needed. `secret` defaults `True` (fail-safe). Path-free by construction, so trinity#570's `template.yaml` → `trinity.yaml` rename cannot reach it. **The normalizer never raises, and that is load-bearing.** `_build_template` runs in bare list comprehensions in `get_all_templates()`, OUTSIDE PR-A's per-template fence (which covers `_build_local_template` only) — a raise there is HTTP 500 with an EMPTY CATALOG, i.e. PR-A's exact bug reopened by the change that surfaces the new metadata. And no bomb is needed: `title: 123` or a bare `title:` was enough. So the builders ALSO wrap the call and degrade to `[]` plus a named error, rather than fencing the comprehension — that keeps the named error the resilience contract promises. The property does not rest on one function's discipline. (Which earned its keep immediately: the wrapper caught a real NameError during development instead of emptying the catalog.) Trust boundary — `title`/`description`/`setup_url`/`name`/`source` are author-controlled strings from arbitrary GitHub repos flowing into an operator-facing "paste your API key" checklist: * **Type-guard before touching.** Never `str()` a container from untrusted YAML: `str()` EXPANDS a shared alias during the walk (443 B → 52 MB in 1.5 s, x10 per level), and both the sanitizer and the record cap act after that cost is paid. * **Cap the INPUT**, entries AND errors AND the base set. Capping records while leaving `errors` uncapped built a 35 MB response out of the cap meant to prevent it; and `default` had no type row, so the 100-record cap acted as a x100 multiplier on it. * **`source` is sanitized** — it carries the raw MCP server name, the exact string `_sanitize_for_warning`'s own docstring names as the threat, and it was not on the list. * **Per-field length caps.** Reusing the 80-char terminal-warning default truncated a realistic 159-char description and made a real 90-char vendor console URL unusable. * **`setup_url` above scheme-only**: https (case-insensitive — `HTTPS://` is a legitimate author), a parseable host, NO userinfo (`https://google.com@evil.tld` renders as one host and resolves to another — the display/resolve split IS the attack), ≤2048, printable. Validate THEN sanitize, and never through a truncator. Residual documented, not claimed closed: `isprintable()` rejects RTL/ANSI but an IDN homograph survives, so a consumer must render the parsed hostname beside the link. * **Never mutates its input** — `_metadata_cache` holds the parsed dict for 600 s and YAML aliases genuinely share nodes, so one in-place normalize would rewrite both aliased fields and persist for ten minutes. Asserted against a deep-copy snapshot, including a real `&anchor`/`*alias` document. `credential_shape_errors` also gains the per-server and per-ELEMENT rows for `mcp_servers`, mirroring what `env_file` already had. The element row is the one that matters — an `env_vars` entry smuggled in as a mapping was the single most dangerous shape in the block and was unnamed. Note this makes the write path (`generate_credential_files` → 400) reject a template that previously created an agent with a garbage declaration: correct per PR-A's fail-loud write contract, and release-noted. `generate_credential_files` is deliberately UNTOUCHED — it still reads `env_file` names-only, which is what makes the forward-compatibility argument true rather than asserted. Refs Abilityai/trinity-enterprise#128 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
The system agent was missing required Docker labels (trinity.ssh-port, trinity.cpu, trinity.memory, trinity.created), causing port allocation conflicts when creating new agents.
Without trinity.ssh-port label:
This fix adds all required labels to match the standard agent creation pattern in routers/agents.py, ensuring proper port tracking and conflict prevention.