feat: SMARTS trading pipeline with Telegram notifications and Miro visualization - #13
Closed
AndriiPasternak31 wants to merge 14 commits into
Closed
feat: SMARTS trading pipeline with Telegram notifications and Miro visualization#13AndriiPasternak31 wants to merge 14 commits into
AndriiPasternak31 wants to merge 14 commits into
Conversation
- Add scripts/smarts_diagram/ package with: - parser.py: Extracts architecture from agent templates - miro_generator.py: Generates diagram layout - miro_client.py: Miro REST API v2 client - Add scripts/update_smarts_diagram.py entry point - Update .claude/commands/update-docs.md with diagram step - Add MIRO_ACCESS_TOKEN and MIRO_BOARD_ID to .env.example Usage: python3 scripts/update_smarts_diagram.py --dry-run Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add 8 SMARTS pipeline agents + supporting templates: - market-regime: Market condition detection - news-sentiment: News and sentiment analysis - discovery: Trading opportunity scanner - analysis: Deep technical analysis - decision: Position sizing and decisions - execution: Order execution via Alpaca - portfolio-manager: Risk oversight - feedback: Performance tracking Also includes: - analyst-agent variants (bull, bear, risk, quant) - scanner-agent, executor-agent, synthesis-agent - smarts-trading, smarts-trader-minimal bundles - gcp-log-monitor utility agent - system.yaml configuration Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add SMARTS summary service for daily trading reports: - Pulls data from Supabase integration_context - Formats comprehensive summaries per agent - Sends to Telegram with deduplication - Scheduler for automated daily reports Add Telegram channel to notification handler: - Support bot_token and chat_id configuration - Environment variable fallback support - Markdown formatting for messages Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add new endpoints in ops router: - POST /api/ops/smarts/summary - Generate and send summary - GET /api/ops/smarts/test-telegram - Test Telegram connection Integrate summary scheduler in main.py: - Start scheduler on app startup - Stop scheduler on app shutdown Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add find_template_file() to check template.yaml then config.yaml - Support both template formats for backwards compatibility - Add default resources configuration - Improve credential extraction for SMARTS agents Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update all template loading code to use find_template_file() for consistent config.yaml support: - routers/credentials.py - routers/templates.py - services/agent_service/crud.py - services/system_agent_service.py Also use DEFAULT_RESOURCES constant for consistency. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add Telegram channel support to notification step configuration: - bot_token: Telegram bot token (supports env var) - chat_id: Telegram chat ID (supports env var) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add changelog entries for SMARTS features - Update architecture documentation - Update roadmap progress Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Script to deploy all SMARTS agents in cascade order. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add flow_visualizer.py to retrieve complete analysis flows from Supabase - Create update_smarts_flow.py CLI entry point - Extract and visualize full decision chain: market_regime → discovery → analysis → decision → execution - Support symbol-specific and auto-select modes - Add SUPABASE_URL/SUPABASE_ANON_KEY to .env.example Usage: python scripts/update_smarts_flow.py --symbol AAPL python scripts/update_smarts_flow.py --dry-run Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add MiroClient.create_board() for creating new boards via API - Flow visualizer now prefers MIRO_FLOW_BOARD_ID over MIRO_BOARD_ID - Update .env.example with separate board IDs for architecture vs flows Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove all text truncation from flow cards (show full data) - Reorganize flow layout: - Market Regime at top-left - News Sentiment cards stacked vertically (up to 3) - Main pipeline in horizontal row: Discovery → Analysis → Decision → Execution - PM Directive below pipeline with connectors - Add visual separator line between architecture and flow sections - Increase card sizes for better readability Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace \n with <br> tags for Miro sticky note compatibility - Add <br> spacers between sections for visual grouping - All format_*_content() functions now use HTML line breaks - Cards display with proper section separation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add changelog entry for flow visualization feature - Create data/smarts-flows/ directory for JSON exports - Add README with usage instructions - Gitignore JSON files (contain live trading data) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
vybe
added a commit
that referenced
this pull request
Apr 18, 2026
…idate-architecture Backend: unregister 7 Process Engine routers (processes, executions, approvals, triggers, alerts, process_templates, audit) plus the process-docs router; drop startup hooks for execution recovery and the process-engine WebSocket publisher. Services under services/process_engine/ remain in place as dormant code. Frontend: remove 11 process-related routes (/processes, /processes/new, /processes/docs, /processes/wizard, /processes/:id, /executions, /approvals, /executions/:id, /process-dashboard) and the dead isProcessSection computed in NavBar. Keep /alerts and /events legacy redirects to Operating Room. Docs: correct stale count claims in architecture.md (main.py line count, router count 45 -> 53, service count 23 -> 37, MCP tool modules 15 -> 16) and expand database.py scope description to reflect 27 domain op classes. Skill: expand validate-architecture to detect drift between arch.md and code: count alignment (D1), scope coherence (D2), enforced MCP parity under #13 (tool module OR '# mcp: none' opt-out), and inline authorization sprawl detection under #8. Output now includes suggested arch.md edits with line numbers, not just pass/fail. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced Apr 21, 2026
Closed
This was referenced Apr 29, 2026
This was referenced May 15, 2026
23 tasks
6 tasks
7 tasks
This was referenced Aug 6, 2026
vybe
pushed a commit
that referenced
this pull request
Aug 8, 2026
…PC/SSE) + control MCP tools (ent#157/#160) (#1628) * feat(a2a): A2A inbound server — public well-known card + JSON-RPC task endpoint (ent#157) Make an exposed Trinity agent reachable + taskable over the open A2A protocol by an external orchestrator (Google ADK, LangChain, Bedrock, another Trinity). OSS `a2a_exposed` primitive (edition-agnostic, default OFF): - agent_ownership.a2a_exposed column (schema.py + tables.py + two-track migration: db/migrations.py + Alembic 0024) + A2AExposureMixin accessors + surfacing on GET /api/agents (mirrors mcp_exposed). OSS owns the column + the read/enforcement; the WRITE is entitlement-gated by the enterprise A2A setter (core-primitive + enterprise-knob, like users.suspended_at). Public A2A server (routers/a2a.py, new a2a_server_router mounted in main.py): - GET /a2a/{name}/.well-known/agent-card.json — unauthenticated discovery; uniform 404 for non-exposed/non-existent (no enumeration oracle; safe by default since the flag can't be set without the entitled setter). - POST /a2a/{name} — JSON-RPC 2.0: message/send (sync bridge to execute_task(triggered_by="a2a")), message/stream (SSE), tasks/get, tasks/cancel; proper JSON-RPC error codes (-32700/-32600/-32601/-32602) + A2A task-not-found (-32001). Bearer = a Trinity MCP key, validated per call by get_current_user (fail-closed 401). Owner/shared access + per-agent inbound allow-list via the new services/a2a_gate seam (OSS no-op; enterprise provider, fail-open). Trigger-boundary idempotency on messageId (Invariant #18) so at-least-once re-delivery can't double-execute. Every task audit-logged (source="a2a"). taskId == Trinity execution_id. - Card honesty: protocolVersion pinned "0.3.0", url → the JSON-RPC endpoint (not the old chat placeholder), preferredTransport=JSONRPC, Bearer scheme. Phased (seam ready): tasks/resubscribe → -32004; message/stream is non-incremental (the agent turn is atomic) but spec-shaped so a streaming client attaches and receives the terminal task. Tests: tests/unit/test_157_a2a_inbound_server.py (16) — card honesty, well-known gating, JSON-RPC dispatch + error codes, message/send bridge (triggered_by=a2a), tasks/get + tasks/cancel, idempotent replay, allow-list allow/deny + fail-open. Schema + Alembic parity guards green. Related to trinity-enterprise#157 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(mcp): A2A control tools — exposure, card, allow-list, endpoints (#160) The MCP surface for the A2A interoperability management plane — the third surface (Invariant #13) over the entitlement-gated enterprise backend (trinity-enterprise#160, /api/enterprise/a2a/*). Distinct from the runtime call_a2a_agent (#736). New src/mcp-server/src/tools/a2a.ts (7 tools): - get_agent_a2a_config, set_agent_a2a_exposure, get_agent_a2a_card (proxies the OSS #737 served-card endpoint), set_a2a_inbound_allowlist, register_a2a_endpoint, list_a2a_endpoints, remove_a2a_endpoint. - Honest gating: an unentitled 403 ("not licensed") / OSS-only 404 return a structured { not_entitled | not_found } — never a silent success. Mutations are owner/admin + human-only, enforced at the backend (agent-scoped key → 403 human_only). Outbound credentials are write-only — the backend returns only has_credentials, so no tool echoes a secret. client.ts: 8 A2A methods (getA2AExposedMap swallows OSS-404/unentitled-403 → {}). agents.ts: list_agents/get_agent best-effort merge a2a_exposed (mirrors mcp_exposed, #846) — omitted in editions without A2A, no OSS↔enterprise coupling. server.ts: register the tool group (connector-denied visibility, like the rest). Tests: src/mcp-server/src/tools/a2a.test.ts (10) — proxy contract, credentials never echoed, entitlement/human-only/404 gating. Full suite 100 pass; tsc clean. Related to #160 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): drop redundant a2a_exposed merge — it's native on GET /api/agents (ent#157) With ent#157 surfacing agent_ownership.a2a_exposed natively on GET /api/agents, list_agents/get_agent already carry the field. Remove the belt-and-suspenders enrichA2AExposed helper + getA2AExposedMap client method (both read the same OSS column). Fewer round-trips, no behaviour change. MCP suite 100 pass, tsc clean. * test(a2a): update #737 card-service tests for the ent#157 honest card The card generator now pins protocolVersion "0.3.0", points url at the real JSON-RPC endpoint (/a2a/{name}) with a documentationUrl, and declares preferredTransport=JSONRPC. Update the two #737 shape assertions (test_card_full_template_shape, test_card_label_fallback_shape) to match — fixes the regression-diff CI failure. * test(a2a): expand ent#157 coverage — SSE, idempotency, task states, auth, soft-delete guard Broaden the A2A inbound-server suite (16→28) and add the OSS exposure-mixin DB suite (11), plus registry entries: - message/stream SSE (working → final completed task) - in-flight messageId idempotency → retryable -32603 - tasks/get state mapping across all Trinity statuses; multipart concat; empty-text invalid - exposed-but-stopped agent still serves the card (label fallback) - unauthenticated JSON-RPC POST → 401 (fail-closed, before dispatch) - A2AExposureMixin: default/set/get, per-agent isolation, missing-agent, and the deleted_at guard (soft-deleted agent can never be flipped/read exposed) Backend a2a+adjacent 142 pass, enterprise 21, MCP 100. * fix(a2a): tasks/get + tasks/cancel read the execution as an object, not a dict (live-caught) Live end-to-end testing surfaced a 500 on tasks/get: db.get_execution returns a ScheduleExecution MODEL, but the handler used row.get(...) (dict API) → AttributeError. Add _exec_field(row, name) that works for both a model (attribute) and a dict, and use it in tasks/get + tasks/cancel. Regression test returns an object-shaped row so CI catches this shape mismatch. Verified live against a running instance: expose → well-known 200 (honest card), message/send → real agent execution (state=completed), tasks/get → 200, messageId idempotency replay (no re-exec), triggered_by="a2a" on the row, allow-list enforcement → 403, credentials never echoed, unexpose → 404. * feat(a2a): A2A config UI — exposure toggle, card URL, skills, allow-list, endpoints (ent#158) Adds an owner-only "A2A" tab on Agent Detail (components/A2aPanel.vue), gated on the enterprise `a2a` entitlement (sessionsStore.a2aAvailable ← enterprise_features) so it never renders in OSS/unentitled builds — no blank tab. Enterprise Vue ships in the OSS bundle, gated by the feature flag (standard open-core seam). The panel: - Expose-over-A2A toggle (default OFF) → the enterprise setter that write-throughs to the OSS a2a_exposed column; not-exposed shows an explainer + CTA (no dead state). - One-click-copy public Agent Card URL (the ent#157 well-known discovery route). - Advertised skills (read-only, from the #737 served card). - Inbound allow-list add/remove. - Outbound endpoint registry add/remove; credentials are write-only (password input, never echoed back — the API returns has_credentials only). Store methods in stores/agents.js (getA2aConfig/setA2aExposure/updateA2aAllowlist/ registerA2aEndpoint/removeA2aEndpoint/getA2aCard); feature flag in stores/sessions.js; tab wired through OverflowTabs so overflow + ?tab= deep-linking keep working. No new backend endpoints — proxies the ent#157/#160 surface (all live-verified). Related to trinity-enterprise#158 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(a2a): route /a2a/ through the front door (nginx + vite dev proxy) (ent#157) The A2A public routes (well-known Agent Card + JSON-RPC/SSE task endpoint) live on the backend under /a2a/, but neither the prod nginx nor the vite dev proxy forwarded /a2a/ — so hitting the card URL on the frontend origin fell through try_files → index.html (prod) / the SPA (dev) and redirected. External A2A orchestrators reach Trinity through this front door, not :8000 directly, so the card was unreachable. - nginx.conf: add `location /a2a/` proxying to trinity-backend:8000 (SSE-friendly: proxy_buffering off, mirrors /api/). - vite.config.js: add `/a2a/` to the dev proxy. changeOrigin:false preserves the browser Host so the backend renders the card's `url`/documentationUrl against the real front-door origin (dev has no PUBLIC_CHAT_URL; prod nginx uses `Host $host`). Verified live on :8001: card served (200 JSON, no redirect) with a self-consistent url (http://localhost:8001/a2a/new_cool_agent). * docs(a2a): user guide for the A2A feature — expose, consume, GUI screenshots (ent#157/#158/#160) Rewrite the stale #737 A2A doc into a full how-to for the shipped feature: - Enable exposure via the new A2A tab (with screenshots) or the MCP tools. - Consume an exposed agent as an external orchestrator: discover the well-known card → authenticate with a Trinity MCP key → task over JSON-RPC (message/send, message/stream SSE, tasks/get, tasks/cancel), using the real live-verified curl flow. - Inbound allow-list, outbound endpoint registry, security/behavior notes, and a reference (public routes, JSON-RPC methods, error codes). Screenshots (headless-captured against the live UI): the full A2A config panel and the Agent Card URL section. Describes only the generic A2A capability + entitlement seam — no enterprise schema/module internals (enterprise-docs guard clean). Related to trinity-enterprise#158 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(a2a): address review — scope messageId dedup, rate-limit the public card, degeneric the seam Resolves the three criticals and the warnings from the #1628 review. Criticals: - messageId dedup was scoped per agent only, but messageId is a peer-controlled field that SDKs auto-generate and the spec only requires to be unique per client. Two callers colliding on "req-1" meant caller B received caller A's stored snapshot — the agent's full response text — and B's own task silently never ran. The scope now carries the caller principal, preferring mcp_key_id so agent-scoped keys resolving to one owner stay distinct. - The unauthenticated well-known card route had no throttle, while each hit costs a DB read, a live Docker API call, and an HTTP call into the agent container. Since the URL is published by design, one address could stall the event loop and hammer the fleet. Adds a per-IP limit ahead of all that work, matching public.py/files.py/webhooks.py. - services/a2a_gate.py named a private enterprise table in a comment. The comment now describes the mechanism, and the file joins SEAM_FILES so the guard covers this new seam (the #1461 class it was blind to). Warnings: - tasks/cancel discarded the bool from terminate_execution_on_agent and always reported "canceled". A queued task would drain later, run, and bill against a caller who believed it cancelled. Queued rows now cancel through the backlog CAS, terminal rows return TaskNotCancelable, and a failed terminate is reported honestly. - An SSE client disconnect raises CancelledError, a BaseException that the generator's `except Exception` never caught, so the row stayed in_flight and wedged the messageId for the full 24h TTL. - The allow-list UI and guide advertised DIDs and caller URLs while the code compares email-or-username, so a DID-only list denies everyone. Both now say email, and the guide states the gate fails open rather than presenting it as a hard boundary. - The access gate had no coverage; adds cases for exposed-but- inaccessible, 404 indistinguishability, and the admit path. Also: cancelled now maps to `canceled` rather than `failed`, the JSON-RPC body is capped before parsing, stream replay returns SSE instead of JSON, and architecture.md records the new routes, the a2a_exposed column, the 0.3.0 protocol version, and the seam service. Every new test was verified to fail against the unfixed code. Related to #1628 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(a2a): requirements + feature flow for the inbound server, correct stale claims Closes the documentation gaps from the /validate-pr re-review. Requirements (requirements/mcp.md): the inbound server is a new capability — two public routes, a new column, an exposure flag, the allow-list seam, MCP control tools — but §32 covered only the #737 authenticated card, explicitly scoped "Phase 1". Adds §32.2 (inbound server, ent#157) and §32.3 (control over MCP, ent#160), each recording the decisions a future reader would otherwise have to reverse-engineer: why the dedup scope carries the caller, why the public route is limited before its own work, why cancel reports what happened, and that the allow-list fails open. Feature flow: adds feature-flows/a2a-inbound-server.md and indexes it. A vertical slice across router, seam, MCP tools, Vue panel, and a private module is the shape feature-flows exists for (cf. mcp-connector.md). Stale claims — two user-facing docs asserted the opposite of what this PR ships, and docs/user-docs/** auto-publishes to a public search index: - faq/collaboration.md said "There is no public unauthenticated /.well-known route yet" - faq/advanced-features.md said "the public /.well-known/agent-card.json route is not served yet" Both now describe exposure, the two public routes, and that tasking still requires a key. "A2A v1.0" is corrected to 0.3.0 in eight places, including four in a2a_card_service.py — the file that emits "0.3.0". The router docstrings still described a single-endpoint Phase 1 surface and called public serving a follow-up needing "a separate access-policy decision"; that decision is the per-agent exposure flag, and it shipped here. Related to #1628 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(a2a): use the canonical ✅ Implemented status label; route the index row Two nits caught re-validating my own commit: - §32.1 used "✅ Shipped", a label the legend doesn't define. The four canonical labels are ⏳ Not Started / 🚧 In Progress / ✅ Implemented / ❌ Removed, and "Implemented" is used 179 times elsewhere; "Shipped" appeared exactly once, introduced by me. (#737 is closed, so ✅ itself is right.) - The requirements index row for mcp.md still said "A2A discoverability", which no longer routes a reader to the inbound server now living in the same area file. Related to #1628 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(a2a): configurable exposed skills — the OSS seam + card filter (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 * feat(a2a): curate advertised skills from the A2A panel (ent#180) The UI half. The panel showed the advertised skills read-only; it now lets an owner choose them (the panel is already entitlement-gated as a whole, so no extra gating here). - Checkbox list over the agent's FULL capability set with the current selection applied; Save / Cancel / "Reset to all". - The selectable list comes from `/api/agents/{name}/info`, NOT the card: the card now returns only the curated subset (that's the point), so it can no longer tell the UI what is available to pick. - Ticking everything stores `null`, not the full list — "no opinion" keeps the agent advertising whatever its template declares as the template evolves, instead of freezing today's tags into a selection that silently goes stale on the next repull. Un-ticking everything stores `[]` (advertise nothing), which the read view names explicitly so it can't be confused with "no capabilities". - The copy says what the control does: hiding a skill stops it being *advertised*, not asked for. Verified by compiling the SFC (script + template) and asserting every new binding is exposed. `npm run build` fails on this machine for an unrelated, pre-existing reason — `mermaid` is declared in package.json but not installed locally, and AgentWorkspace.vue fails to resolve it identically with my changes stashed. Public feature-flow documents the seam mechanism only, per the standing enterprise-docs rule. Related to trinity-enterprise#180 * fix(a2a): re-chain the a2a_exposed revision onto 0024 — one Alembic head (ent#157) The rebase surfaced a collision the SQLite track absorbed silently and the PostgreSQL track could not: #1666 landed `0024_agent_ownership_volume_base_name` on dev while this branch carried `0024_agent_ownership_a2a_exposed`, both chained to `0023`. Two revisions, one parent — a branched history, so `alembic upgrade head` has multiple heads and the pg-migrations job fails at the real PG boot path. The two tracks fail differently, which is worth remembering: `db/migrations.py` is a LIST, so its conflict was a union and both entries just run. Alembic is a LINKED LIST, so the same "both sides added a migration" merges cleanly at the text level and breaks at runtime. A green SQLite parity check says nothing about the chain. Renamed to `0025_agent_ownership_a2a_exposed` with `down_revision = 0024_agent_ownership_volume_base_name`. Verified by walking the graph: 26 revisions, exactly one head, no parent with more than one child, no dangling down_revision, and an unbroken chain head→`0001_baseline`. Related to trinity-enterprise#157 * ci(a2a): fix gitleaks FP + the #1310 inline-auth guard for the A2A inbound gate Two red checks on #1628, both the documented escape-hatch cases: - regression diff — HEAD introduced test_1310_auth_wiring:: test_no_inline_auth_gates_in_routers. routers/a2a.py::_authorize_inbound does an inline `db.can_user_access_agent(...) → raise 404`, which the INV-8 static guard flags. It's an intentional uniform-404 design (non-exposed OR inaccessible → the SAME 404, so /a2a/{name} is not an enumeration oracle) with an allow-list 403 that only fires AFTER access is proven — a shared AuthorizedAgentByName dependency can't express that shape. Added ("a2a.py", "_authorize_inbound") to the guard's _ALLOWLIST alongside the other intentional-404 helpers, with the rationale. Verified it's the sole violation and the entry clears it. - gitleaks — the curl-auth-header rule flagged docs/user-docs/integrations/ a2a-protocol.md:92, a `Authorization: Bearer trinity_mcp_YOUR_KEY` example. Every other user-doc uses `Bearer $TOKEN` (a shell var, no entropy, never flagged); the a2a doc was the lone literal placeholder. Switched it to $TOKEN (prose now says `export TOKEN=trinity_mcp_…`) — matches the sibling convention, no .gitleaks.toml carve-out, and no risk of exempting a real secret in docs/user-docs (unlike a blanket path skip). Related to #157 * ci(gitleaks): allowlist docs/user-docs as a documented curl-example zone The head edit (a2a-protocol.md → `$TOKEN`) can't clear the gitleaks check: it scans the whole PR commit RANGE, and the `trinity_mcp_…` placeholder was added in an in-range commit, so the curl-auth-header rule still fires on that commit's diff. Added `docs/user-docs/` to the [[allowlists]] paths (a pre-scan file skip), alongside the existing docs/memory, docs/archive, docs/releases and docs/security-reports zones — same accepted "docs-are-not-code, they carry curl `Authorization: Bearer` examples" tradeoff (#1164). Surgical to the docs tree; real code paths stay fully scanned. Related to #157 * fix(a2a): re-chain the Alembic revision onto dev head 0028 after rebase (#157) The rebase onto dev replayed the A2A migration at 0025 chained onto 0024 (the pre-merge state); dev's own chain runs to 0028_agent_reminders, so that left two Alembic heads. Renamed 0025->0029_agent_ownership_a2a_exposed and re-chained down_revision to 0028. Single head verified. SQLite mirror docstring updated to match. Related to #157 * fix(a2a): re-chain the Alembic revision onto dev head 0030 after rebase (#157) The rebase onto dev landed 124 new commits, two of which added Alembic revisions (0029_product_events, 0030_slack_channel_allow_proactive). Both this branch's 0029_agent_ownership_a2a_exposed and dev's 0029_product_events hung off 0028_agent_reminders, so the tree had two heads and `alembic upgrade head` would have failed on PostgreSQL — the exact failure the pg-migrations CI job exists to catch (Invariant #3). Renamed to 0031 and re-pointed down_revision at 0030_slack_channel_allow_proactive, plus the stale cross-reference in the mirrored SQLite migration's docstring. Verified rather than assumed: ScriptDirectory.get_heads() resolves to exactly one head with a 32-revision chain, run inside the backend image because the revision tree is what CI walks, not a hand-read of down_revision lines. Related to Abilityai/trinity-enterprise#157 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(a2a): register the a2a trigger in the Executions filter allow-list (ent#157) `triggered_by="a2a"` was added to `_TRIGGER_BUCKETS` (analytics) but not to `_VALID_TRIGGERS` (the fleet Executions list filter). That constant is a filter allow-list over a plain TEXT column, and its own comment states the failure mode: an unlisted value degrades to "no filter", so `?triggered_by=a2a` returned EVERY execution on the install instead of the A2A ones — a filter that lies rather than one that errors. Found by the enum-completeness pass of /review: a new trigger has to be wired into every constant that enumerates triggers, and this diff reached one of two. Related to Abilityai/trinity-enterprise#157 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(learnings): the three-constant trigger wiring class (ent#157 review) The permissive-default asymmetry is the durable part: the trigger constant that is easy to remember (analytics buckets) degrades safely to `Other`, while the one in a different file degrades to "no filter" and silently returns every row. Ranking an enum's call sites by how they FAIL rather than how findable they are is the reusable review move. * chore(a2a): re-chain the Alembic revision + restore the enterprise pointer after rebase (ent#157) Rebasing onto dev (57 commits) needed two fixes the conflict resolution couldn't cover: 1. Alembic head. The a2a revision was chained onto 0030, but dev has since added 0031_channel_report_back and 0032_telegram_progress_indicator — so the rebased branch had a duplicate 0031 number and TWO heads, which breaks PostgreSQL boot (`alembic upgrade head` refuses a branched graph). Renamed 0031_agent_ownership_a2a_exposed -> 0033_ and re-pointed its down_revision at 0032_telegram_progress_indicator. `ScriptDirectory.get_heads()` now returns exactly one head over a 34-revision chain. 2. Enterprise submodule pointer. The bump to 740063e3 (the A2A module: ent#160 registration wrapper, #180 curated skills) rode on the dev-merge commit that a flat rebase drops, so the rebased branch silently reverted to dev's e898cef8 — an ancestor — and an enterprise build would have lost the module this PR's OSS seam exists for. Restored. The SQLite track needed no fix: its migration entry appends after dev's two new ones, which is the correct application order. Related to #1628 * fix(a2a): alert on an unresolved command from an inbound A2A task (ent#157) `triggered_by="a2a"` was registered in `_TRIGGER_BUCKETS` and `_VALID_TRIGGERS` but not `_AUTONOMOUS_TRIGGERS`, with no rationale written down — the exact "decision, not a reflex" this branch's own learnings entry calls for. That set means "no human is watching the reply", so an unresolved slash-command run earns an Operating Room alert (#1410). An inbound A2A task is dispatched by a remote MACHINE caller — structurally what `agent` already is — so nobody on this install sees the "Unknown command" text come back. Added, and the reasoning (including why `manual`/`mcp`/`public`/`chat`/`session` stay out) is now a table in the feature flow rather than tribal knowledge. Also corrects two claims the rebase left stale in that flow doc: - the Alembic row still named `0024_…` with `down_revision 0023_…`; it is `0033_agent_ownership_a2a_exposed` onto `0032_telegram_progress_indicator` - the bridge section said a2a "buckets into the analytics catch-all until it earns its own bucket" — it has had the `Agent-to-agent` bucket all along Related to #1628 * chore(a2a): re-chain the Alembic revision onto the current dev head (ent#157) dev has moved twice since this branch last renumbered. `0033_agent_ownership_a2a_exposed` and dev's `0033_agent_evaluations` shared the parent `0032_telegram_progress_indicator`, so the graph had two heads — `alembic upgrade head` refuses that, and on PostgreSQL that is a boot failure, not a migration warning. Now `0035_agent_ownership_a2a_exposed` chained to `0033_agent_evaluations`. 0034 is skipped rather than taken because PR #1901 holds `0034_skill_sources` unmerged off the same head: whichever of the two lands second still has to re-chain (inherent to two open PRs adding revisions), but skipping the number keeps the filenames from colliding, so that rebase is a one-line edit rather than a rename plus an edit. The reasoning is recorded in the revision docstring so the next rebase doesn't have to re-derive it. Verified: the version graph has exactly one head and no dangling down_revision; 61 alembic-guard tests and 440 tests across a2a/migration/schema /registry/trigger pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(a2a): classify the a2a trigger as pull-stranded, and drop the premature submodule bump Two drift fixes from dev moving under this branch. No change to the A2A feature itself. **1. `triggered_by="a2a"` was an unclassified autonomous trigger.** #2057 landed on 2026-08-07, after this branch's last commit, adding `test_2048_pull_pilot_reach::test_the_five_stranded_triggers_are_named_and_complete`: assert _AUTONOMOUS_TRIGGERS - PULL_REACHABLE_TRIGGERS == set(_STRANDED) This PR adds `a2a` to `_AUTONOMOUS_TRIGGERS`, so the assertion fired deterministically across all three seeds — which is the guard working as designed: it exists so a new trigger surfaces as an unreviewed addition rather than silently joining the stranded set. Classified as **stranded**, deliberately rather than by default. `PULL_REACHABLE_TRIGGERS` is an explicit allow-list, so an unlisted trigger lands there automatically; the reason is now written at the site. A2A's `message/send` consumes `result.response` synchronously to build the JSON-RPC artifact it returns to the remote caller, and a pull-claimed row is dispatched by the agent later with no synchronous response — so pull dispatch structurally cannot serve this trigger. Same reason `fan_out` and `loop` are stranded. Test renamed off the hard-coded count. **2. Reverted the `src/backend/enterprise` bump to `740063e3d`.** That commit lives only on trinity-enterprise#161's unmerged feature branch: it is 6 commits behind that branch's own tip and diverged from enterprise `main` (main +5/-7). Merging it would point dev at unmerged code missing five commits of enterprise main, with green CI — the stale-pin failure mode where entitled features 403 everywhere and nothing looks wrong. The pin stays at dev's current `e898cef83`. Per the sequencing agreed on ent#161, the pointer moves in its own PR after ent#161 lands on enterprise main — and that PR must ride a branch already containing this one, or a dev→main cut carrying the bump without it reproduces the 403-forever state. Verified: 170 passed across the four a2a suites, the pull-pilot suite, the Invariant #8 auth-wiring guard, and the Alembic revision-id lint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: trinity-ability <trinity-ability@users.noreply.github.com> Co-authored-by: trinity-ability <309458136+trinity-ability@users.noreply.github.com>
AndriiPasternak31
added a commit
that referenced
this pull request
Aug 9, 2026
…ription The panel is a new file specifically so Settings.vue's raw-color counts cannot move — the ratchet only lets per-file counts shrink, and Settings.vue sits at 63 non-gray. It gains one import and one tag, neither carrying a class. Measured: the new panel is 0 non-gray / 0 hex / 32 semantic tokens, and Settings.vue is byte-identical to origin/dev at 63/832/1 (the gray drift vs the committed baseline predates this branch). The status block is the point of the panel, not decoration. Fail-open means a broken registry looks exactly like a working empty one from the catalog, so this is the only surface where an operator can see that their registry 404s. The backend publishes a fixed lowercase code and never prose — a hostile server's response text must not reach an admin's screen — so the explanations are ours, keyed by code, and each one names the remedy. Uses LoadFailed for a failed fetch and InlineError for a failed verb rather than sharing one error line: "couldn't load the settings" and "couldn't save the URL" point at different remedies, and principle 18 says a failed verb persists next to the control rather than becoming a toast. The MCP fix: create_agent's `template` description asserted that list_templates "on a default install returns local templates only until an admin curates GitHub repos in Settings". With a default-enabled registry that is false, and a wrong tool description actively misleads a model. Description only — no parameter, client method, type or behaviour change, so Invariant #13 stays satisfied without touching the tool. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Aug 9, 2026
…ity-enterprise#14) (#2033) * docs(requirements): TMPL-002 remote template registry (ent#14) Rule of Engagement #1 — requirements land before implementation. New §4.2.2 in core-agent.md, placed immediately after §4.2.1 because the registry is a new *source* for the seam TMPL-001 documents: a reader who lands on 4.2.1 must find it in the next paragraph, not in another file. §4.2.1's "None = use defaults" sentence gets the one-line cross-reference that keeps it true. Two things the entry states plainly rather than comfortably: - `repo` is a capability pointer. It is literally true that the four allowlisted fields only change which repos are listed and how they are labelled/ordered; as a security statement that is misleading. Choosing the repo chooses which template.yaml Trinity fetches and trusts, and that document declares mcp_servers, credentials, schedules, data_paths, persistent_state and fork_to_own. The allowlist bounds the direct blast radius, not the indirect one. - The honest GitHub cost. A non-empty registry re-introduces the per-repo metadata fetches #1931 had driven to zero: workers x windows/hr x entries. On a PAT-less install that exceeds the 60/hr anonymous budget above ~5 entries, which is exactly the precondition that makes the fork_to_own fail-open reachable. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe-yaml): distinguish the per-repo payload from the remote index The TWO ALIAS POLICIES docstring says a "template catalog entry" may legitimately anchor a repeated block, and that clause reads as an argument for giving the remote registry a BUDGET policy. It is not one — the only shipped BUDGET caller is load_template_yaml, a per-repo template.yaml, which really can anchor a repeated block. The remote index cannot: it is a flat list of unique entries carrying one pointer and three display scalars each. Amend the clause so the next author cannot make the same misread, and add load_template_registry_yaml() pinning REJECT + the 256 KiB cap at the utils layer — the same reason load_template_yaml exists, so the decision is never relitigated at a call site. Duplicate-key rejection is load-bearing for this document specifically: a registry with two `templates:` keys would silently last-wins, showing one catalog to the human editing the file and serving another to Trinity. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(config): registry URL + hard kill switch, and its SSRF gate TEMPLATE_REGISTRY_URL / TEMPLATE_REGISTRY_ENABLED on the OPERATOR_INTAKE_ENABLED shape, plus validate_template_registry_url(). Two choices worth their comments: The switch is deliberately NOT settings_service._resolve_bool_flag. That helper's env leg is opt-in only ("true"/"1"/"yes" -> True, anything else falls through to `default`), so with default=True it would silently swallow TEMPLATE_REGISTRY_ENABLED=false and ship a kill switch that does nothing — #1039's inert-by-obscurity class, caught before a line of it was written. The validator is a sibling of validate_skills_library_url, not a reuse: that one is github.laiyagushi.com-only, which is right for a clone target and wrong for a registry an operator may self-host. What carries over is the perimeter check; what is new is refusing userinfo outright rather than stripping it (a stripped credential gets persisted into a settings row and echoed through a status payload) and refusing an unresolvable host rather than deferring, since unlike a later git clone nothing downstream will fail loudly on our behalf. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(templates): remote template registry service (ent#14) The fetch/parse/cache half. Never raises; every failure returns []. Fail-open here is structural, not an except branch: get_all_templates() returns local + github, local needs no network, github is empty by default, and this module can only ADD to github. So unreachable / 5xx / timeout / malformed / bombed / oversize / redirected / empty all reduce github toward [], which is the already-shipping default state of the product. The except layers are a second layer on top, because fail-open must not depend on this module being bug-free. Decisions that are load-bearing and easy to undo by accident: - The byte cap is enforced on bytes RECEIVED, not on Content-Length, which is absent on chunked responses and trivially lied about. resp.text on a 10 GB body OOMs the worker before any parse-time cap can act, so the transport ceiling is the gate and the parser's max_bytes is the belt. - follow_redirects=False is a security control. A URL that passed the SSRF gate and then redirects is a bypass; a redirect is a fetch failure. - TTL is 3600s + jitter, deliberately NOT aligned with the 600s per-repo cache. Aligning them is a correlated thundering herd, not a shared rhythm: one expiry fires the registry fetch and N per-repo GitHub fetches in the same instant, and --workers 2 drift into phase. It is also the cheapest lever on the anonymous GitHub budget, so it is doing security work, not tidiness. - Serve-stale is capped at 7 days. Unbounded stale keeps a de-curated, renamed or compromised repo listed indefinitely while the operator sees a catalog that still renders — no signal at all. - Invalidation is a generation counter read at cache-hit time, not a per-process clear. A per-process clear half-applies under --workers 2, and a nondeterministic setting is worse than a slow one. Coupled to the TTL raise; the two must not be split. Found while smoke-testing the parser: `../evil` matches all four copies of the owner/repo regex, because `.` is inside the character class. That breaks the one invariant the catalog card rests on — the id would render "../evil" while both the API and clone URLs normalize to a different repo. This gate now refuses dot segments explicitly and is deliberately stricter than its three siblings, which share the hole on paths reachable only by hand-typed ids. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(templates): resolve the registry in BOTH resolvers, fail the fork gate closed Two changes that only make sense together. get_all_templates() was never the only resolver. get_github_template() resolves the same repo list for GET /api/templates/{id} and for agent creation (crud._resolve_github_repo_and_pat), and its ladder ended in a dynamic fallthrough calling _build_template with no override dict at all. Landing the registry in one of them would have listed a template as "Cornelius - your second brain" and resolved it by id as "cornelius" - and under a rate limit the detail view degrades to the repo basename, so the hardening the registry buys would not have reached the detail path. Both now share one ladder; a test pins list, detail and create to it. learnings.md 2026-07-10, on schedule. The second change is why the first one matters. An unreadable template.yaml and an absent one were the same value: _fetch_template_yaml_result already distinguishes them and the catalog wrapper threw the reason away one frame below the code that computed it. So a 403 produced fork_to_own: None, `== "required"` was False, and creation bound the agent to the shared upstream template repo instead of a user-owned copy. Silent, and the thing in the wrong place is the user's knowledge base. That bug is pre-existing. This feature is what makes it expected rather than theoretical: it re-introduces per-repo fetches on a default install, ships default-on, and its own arithmetic puts a PAT-less install over GitHub's 60/hr anonymous limit above ~5 listed repos - while the fleet the registry exists to serve very likely includes the fork_to_own template. The reason is now cached alongside the metadata and surfaced as metadata_unavailable, and creation refuses (503, retryable) rather than guessing. Scoped to the branch that is unsafe: a caller who IS forking gets a user-owned repo whatever the template declares, so an outage does not block them. A clean 404 stays "absent", so a repo that ships no template.yaml creates as it always has. Also here, because it is the same builder: priority becomes overridable, which is the only dimension of "curate freely - feature, order, add, deprecate" that had no mechanism. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(settings): GET/PUT/DELETE /api/settings/template-registry Same shape as /api/settings/github-templates deliberately — the registry is a new source for that seam, not a second settings idiom. Registered before the /{key} catch-all (Invariant #4), verified by inspecting the router's own route order rather than by reading it. reject_agent_principal on both writes is not optional. assert_admin answers what role, never whether this is a human: get_current_user resolves an agent-scoped MCP key to its owner carrying the owner's role, so on a default admin-owned install any agent's injected TRINITY_MCP_API_KEY passes a bare admin gate (trinity-ops-agent#232). The consequence here is total — an agent could repoint the platform's template registry at a URL it controls and every operator browsing templates would see its catalog. The catch-all blocks all four registry keys, on PUT *and* DELETE. PUT because the generic route takes an unvalidated Dict[str, str], which would leave the SSRF gate one request away from bypass. DELETE because — unlike the #1644 retention acks, where deleting an ack re-arms a guard and fails safe — deleting template_registry_enabled reverts it to its default of ON, re-enabling egress an operator deliberately switched off, and that route carries no human gate. generation and lkg are blocked because they are the cache itself. The status block on GET is part of the contract, not decoration: fail-open makes every registry failure invisible in the catalog by design, so this is the only place an operator can see that their registry 404s. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(settings-ui): template registry panel; fix a stale MCP tool description The panel is a new file specifically so Settings.vue's raw-color counts cannot move — the ratchet only lets per-file counts shrink, and Settings.vue sits at 63 non-gray. It gains one import and one tag, neither carrying a class. Measured: the new panel is 0 non-gray / 0 hex / 32 semantic tokens, and Settings.vue is byte-identical to origin/dev at 63/832/1 (the gray drift vs the committed baseline predates this branch). The status block is the point of the panel, not decoration. Fail-open means a broken registry looks exactly like a working empty one from the catalog, so this is the only surface where an operator can see that their registry 404s. The backend publishes a fixed lowercase code and never prose — a hostile server's response text must not reach an admin's screen — so the explanations are ours, keyed by code, and each one names the remedy. Uses LoadFailed for a failed fetch and InlineError for a failed verb rather than sharing one error line: "couldn't load the settings" and "couldn't save the URL" point at different remedies, and principle 18 says a failed verb persists next to the control rather than becoming a toast. The MCP fix: create_agent's `template` description asserted that list_templates "on a default install returns local templates only until an admin curates GitHub repos in Settings". With a default-enabled registry that is false, and a wrong tool description actively misleads a model. Description only — no parameter, client method, type or behaviour change, so Invariant #13 stays satisfied without touching the tool. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(ent14): the fail-open matrix, proven rather than asserted 309 tests across five files. Every failure mode is driven through the real parser and a real httpx client over MockTransport, and every assertion is on the CATALOG OUTPUT — not on whether an except branch ran. A test that asserts the except branch ran would still pass if that branch returned garbage. The matrix: unreachable, read timeout, 404/403/500/503, redirect, oversize body, lying Content-Length, malformed YAML, level-6 alias bomb, duplicate keys, HTML captive portal, binary body, top-level list, future schema version, non-list templates, empty body, empty registry — each asserted twice, once for "the catalog equals the bundled floor" and once through the router's own sort, which is where a non-int priority used to 500 the endpoint. Plus a registry service that simply raises, because fail-open must not depend on it being bug-free. Two tests exist to stop the matrix being vacuous: one pins that the real bundled floor is non-empty (otherwise every "degrades to the floor" assertion is trivially true), and one re-probes the lying Content-Length through a second client to prove the header really said 12 while the body was 100 KB — otherwise a future refactor could start trusting Content-Length and these would still be green. The F2 regression test was verified to be red without its fix: neutralising the guard fails 10 of its 24 assertions. The parity test encodes a real divergence rather than papering over it. `.` is inside the shared owner/repo character class, so `../evil` matches all three older copies of the pattern; the registry gate refuses dot segments and the test pins BOTH halves — the shared corpus where all four agree, and the containment that everything the registry accepts, the others accept. If somebody later tightens those copies, the test fails and points at deleting the assertion rather than leaving a stale comment. tests/unit/pytest.ini overrides pyproject.toml, so asyncio_mode = auto does not apply here and a bare `async def test_*` is collected and silently never awaited. Everything is sync; the one async call under test is driven through an explicit asyncio.run. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ent14): architecture + feature flows for the remote template registry architecture.md x3, in place — the registry is one source feeding one existing service, not a cross-cutting subsystem: the template_service catalog entry gains the ladder, template_registry_service gets its own <=2-line catalog entry, and the Platform Settings table gets the three endpoint rows. The flow doc is standalone rather than a section inside platform-settings.md. The plan named that as an escape hatch if the section ran past ~60 lines; it ran to 495, because the fail-open matrix, the cache semantics and the fork_to_own fix each need their own argument. So platform-settings.md gets a short section with the three things a reader of THAT file needs, and the index gets a catalog row — a new doc always gets one. library-page.md's `GET /api/templates` row said the GitHub half was the admin-configured list or config.py defaults. That becomes wrong on merge, so it now names the three-tier ladder. The page itself is unchanged, which is the point: registry entries are source: "github" and land in the existing grid. platform-settings.md already carried the sentence "trinity-enterprise#14 will repoint this seam". This change is what makes it past tense. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(templates): the fork_to_own fail-open limitation is fixed, not restated /sync-feature-flows found this and my own docs pass had missed it. template-processing.md carried, as a Known Limitation: "fork_to_own: required enforcement is fail-open when the template.yaml metadata fetch fails (advisory gate; empty metadata -> flag unseen for that 10-min cache window)" That was an accurate description of a real defect, written down and left. It is now false, and a stale limitation is worse than none — the next reader would have designed around a gate that no longer behaves that way. Replaced with what actually happens, keeping the old sentence quoted so the history is legible: the fetch reason rides the metadata cache as metadata_unavailable, the non-forking creation path refuses with a retryable 503, a clean 404 still means "declares nothing", and a caller who is forking is never blocked. The trade-off is stated rather than buried — creation now depends on GitHub API reachability where it previously depended only on git clone. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ent14): restore the vendored safe_yaml mirror and two stubbed seams Three regressions the full unit run caught that the targeted runs could not. 1. safe_yaml.py is a BYTE-IDENTICAL vendored mirror into the agent server (Invariant #5, ent#314/#1965) and I edited only the canonical copy. The parity guard is exactly the CI mechanism that exists for this and it worked; re-copied. Worth noting the architecture doc's own warning applies here: a guard that walks only one tree is not a guard, and this one walks both. 2. get_all_templates() had moved onto a renamed bulk fetch (_fetch_all_metadata_results), which silently bypassed two existing test doubles — test_ent89's fence stub and test_1931's network stub — so both fell through to real code. _fetch_all_metadata keeps its name and its `repo -> metadata` return shape; the fetch reason rides _metadata_cache instead and is read back by _metadata_reason_from_cache. No caller pays a signature change, and routers/settings.py's direct call is untouched. The bulk path still submits the reason-preserving variant, deliberately: a cache entry written with reason=None for a repo whose fetch actually 403'd would tell the creation path "readable, declares nothing" for a full TTL — reopening the fork-gate bypass through the catalog's own cache. 3. Two test doubles updated where the seam genuinely moved rather than the code being wrong: test_ent89's _build_template stub was one parameter short (a short stub raises TypeError *inside* the fence that class exists to test, so the assertion failed for an unrelated reason), and test_1931's stub set now includes _fetch_template_yaml_logged — without it the counter-test silently stops counting, which is the failure mode that test was written to prevent. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(ent14): pin the settings stub by sys.modules key, not by module object 43 of my own tests passed in isolation and failed under the full run — the exact trap test_ent89_template_schedules.py documents, walked into anyway. Both consumers resolve the module lazily, at call time: template_service's `from services.settings_service import get_github_templates` and the registry service's `from services.settings_service import settings_service` each read sys.modules by key. Patching an attribute on a separately-imported reference works right up until an earlier file has swapped the module object, after which the stub is silently ignored. The resulting failure was worse than confusing, it was inverted: with the stub bypassed the real accessor read the real tmp SQLite database, found the durable last-known-good a previous ent14 test had legitimately persisted, and served it — so a test asserting "a connect error degrades to the floor" reported a fail-open bug that does not exist. A green isolated run and a red full run disagreeing about a security property is the worst shape this could have taken. Fixed with monkeypatch.setitem on the exact key, and the stub built from a copy of the real module's namespace so SettingsService and the key constants still resolve through it. Full unit suite: 7781 passed, 16 skipped, 0 failed. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ent14): the hard kill switch was inert — wire both registry knobs into both composes config.py read TEMPLATE_REGISTRY_ENABLED and TEMPLATE_REGISTRY_URL from the environment, and neither compose file injected them. Prod compose launches standalone — no base merge, no env_file: on any service — and it is what every deploy path uses, so the explicit environment: list is the only route into the container. Both levers were inert on every real instance while working fine on a laptop, where the shell environment reaches the process directly. TEMPLATE_REGISTRY_ENABLED is the feature's hard kill switch: the documented air-gap answer that no system_settings row may override. An inert one is not a missing convenience — it is a default-ON feature making outbound requests with no way to stop it. The irony is the reason this is pinned rather than just fixed. The comment block directly above the os.getenv call argues at length, and correctly, against routing the flag through settings_service._resolve_bool_flag, because that helper's opt-in-only env leg would "silently swallow TEMPLATE_REGISTRY_ENABLED=false and ship an inert kill switch (#1039 class)". The flag then shipped inert anyway, by the other route, ~15 lines away. Avoiding a known failure mode in the layer you are looking at does not avoid it in the layer you are not. Sixth recurrence of the #1039/#1056 packaging-gap class (#1056 VOIP_*, ent#31 LOG_*, #1039, #1871 AGENT_LOG_*, #411 CANARY_*), so it gets a guard shaped like test_canary_env_prod_parity.py — the guard for the fifth. What the guard asserts, and why presence is the wrong property: - injection in BOTH composes; drift between them IS the bug - the ${VAR:-true} form, not a hardcoded `- TEMPLATE_REGISTRY_ENABLED=true`, which satisfies a presence check while re-breaking the switch - the URL's full non-empty default; a bare `:-` arrives set-but-empty, os.getenv returns "" instead of falling back, and the registry points at nothing (#1076) — strictly worse than absent, because it looks wired - compose and config.py defaults are equal, since two spellings of one default drift invisibly (the container always wins, so the code default becomes decorative while still reading as authoritative) - a meta-test that goes red on pre-fix content AND on the hardcoded revert Verified by rendering rather than grepping: `docker compose config` shows both vars on backend for each file, and TEMPLATE_REGISTRY_ENABLED=false propagates through to "false". The guard was additionally proven by deleting the real prod line and watching it go red — the first attempt at that used `grep -v 'VAR=${VAR'`, where $ is an end-of-line anchor, so nothing was removed and the suite reported a false green against an unmodified tree. Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ent14): CSO diff findings — wire-byte ceiling, creation-path fork gate, CGNAT Three findings from the security review of this branch, each a case where the control was present and correct one level above where the attack lands. S1 — a byte ceiling on decoded bytes is not a byte ceiling. _fetch_registry_text streamed under a 256 KiB running total, with a docstring correctly explaining why Content-Length cannot be the gate and why resp.text on a huge body OOMs the worker. Both true, both defeated one level down: a custom headers= dict does not override httpx's default Accept-Encoding: gzip, deflate, and iter_bytes() yields DECOMPRESSED chunks. A 199 KiB wire body — under the cap, so the early abort passes it — inflated ~1030:1 before the running total was ever consulted: 458 MB of transient allocation on the event-loop thread. The cap bounded the decoded total, which is not the resource under attack; the peak is. Now the ceiling counts wire bytes via iter_raw(), any Content-Encoding is refused outright before the body is read (encoding_refused, named rather than folded into bad_shape so gzip bytes hitting a UTF-8 decode don't report as a shape problem), and Accept-Encoding: identity is sent as the polite half only — a request header is a preference, never a control over a hostile server. Same body post-fix peaks at 0.01 MB. The harness hid it and then inflated it: httpx.Response(content=…) decodes and buffers in the CONSTRUCTOR, so every fixture built a response the production iter_raw() path cannot even read and that could only ever exercise iter_bytes(). The byte-ceiling tests were green against a shape incapable of expressing the failure. Both transport fixtures now convert every mock to a real stream — a transport test whose mock does not stream is not a transport test. S2 — the fork_to_own gate read the catalog cache, not its own read. The gate is correct in direction and was wrong in source: it consulted gh_template["metadata_unavailable"], derived from the global platform PAT, off the default branch, through a 600 s cache — while crud.py had already read the same template.yaml one line earlier with the caller's resolved PAT and parsed ref, cache-bypassed, for ent#89's schedules. ent#89's own docstring had written the warning: "the exact silent-ignore class this feature exists to close, reintroduced one layer up." Both error directions followed. FALSE PASS: GitHub answers 404, not 403, for a repo a token cannot see, so a private fork_to_own: required template readable only by the creator's own PAT was classified absent, the gate passed, and the agent bound to the shared upstream — the precise outcome the gate exists to prevent, with no attacker involved. FALSE REFUSE: a poisoned shared cache entry 503'd a creator whose own PAT read the file fine, for a full TTL. Fixing only the availability half would NOT have closed the false pass — for a private template the catalog's fork_to_own VALUE is None for the same reason its verdict is wrong, so the gate would sail past a `required` it still could not see. Both inputs now come from _read_source_template, and `required` from EITHER read enforces: a union, so this can only remove a false pass, never add one. Costs zero extra GitHub calls, and the read that DECIDES is now made with the same credentials as the clone that follows. S3 — RFC 6598 shared address space (100.64.0.0/10) was admitted. Python's ipaddress reports CGNAT as neither is_private nor is_reserved, so the standard predicate stack passed it. Not reachable in Trinity's own topology (both Docker networks are 172.28/16 and 172.29/16, which are is_private), but several cloud providers address internal endpoints out of that range, so a Trinity deployed there had a hole the same shape as 10.0.0.0/8. Both /10 boundaries are held public so a legitimate registry is not blackholed, and one CGNAT record among public ones is enough to refuse. Docs: requirements TMPL-002, architecture, template-registry + scheduling flows, and three learnings entries for the transferable classes: a decoded-byte ceiling; a gate reading the catalog's cached metadata instead of the creation-path read standing one line above it; and the packaging class from the preceding commit — its ledger entry rides here because learnings.md is a single append-only file and the compose fix was committed on its own to stay self-contained. Full unit suite: 7851 passed, 17 skipped, 0 failed (pytest-randomly). Refs trinity-enterprise#14 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(ent14): adapt two suites to dev's newer gates after the rebase Rebasing onto dev picked up two changes that landed under this branch and moved what its own tests assert. Both are the other change being right. - ent#15 (#2040) inserted an import-intent gate into `_resolve_template` ahead of the fork gate. Two `SimpleNamespace` config doubles predate the field, so `config.import_intent` raised AttributeError before reaching the assertion — a stub gap, not a production one. The fork/clone path and the S2 fail-closed gate are unchanged, and dev's `copy` intent carries its own `fork_to_own: required` refusal before its early return. - #1890 made `require_admin` itself reject agent-scoped principals, closing the class at the gate rather than per-endpoint. This suite asserted an agent key could still READ the registry setting, on the reasoning that the human check belongs on the writes. That is now overruled, and correctly: the GET returns the registry URL plus a live status block, which for a private catalog is exactly the pointer an agent should not enumerate. The explicit `reject_agent_principal` on PUT/DELETE is kept as belt-and-braces — it states the intent locally and survives any relaxation of the gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: trinity-ability <309458136+trinity-ability@users.noreply.github.com>
vybe
pushed a commit
that referenced
this pull request
Aug 11, 2026
…ps (ent#326) (#1983) * feat(executions): GET /api/executions/timeline — bucketed fleet rollups (ent#326) The backend half of ent#94's grid-widget foundation. Three tile sub-issues (#96 executions-by-trigger, #98 fleet cost, #101 fleet context) were blocked on an endpoint the epic listed but never filed; they now share one query instead of each growing their own. Time-series sibling of `/api/executions/stats`: same table, same access model, buckets instead of scalars. Read-only — no schema change, no migration, no MCP surface (Invariant #13's three-surface cost buys nothing for a dashboard read). **An analytics axis cannot degrade the way a filter can.** `/stats` and the list route coerce an unknown `hours` to 24. That is right for a filter — the worst case is more rows than you asked for — and wrong for a chart, where it silently redraws a window the caller never requested and gives them no way to tell. Both `group_by` and `hours` therefore 422 by name. `hours=0` is refused for `hour`/`day` specifically: an all-time axis emits one bucket per interval since the fleet's first execution, which is an unbounded response nobody asked for. It stays allowed for `trigger`/`agent`, which have no continuum and are bounded by the number of distinct values. Trigger folding happens in Python through `_TRIGGER_BUCKETS`, not a SQL CASE, so a newly-added trigger type lands in the explicit `Other` catch-all instead of vanishing from a chart the first time someone forgets the SQL. Buckets slice the stored ISO-Z `started_at` with `substr` rather than a date function — dialect-agnostic across SQLite and PostgreSQL, and the same UTC the row was written with (Invariant #16). **The token question ent#326 requires settling: option 1.** `schedule_executions` has no usage-token column — `output_tokens` lives only on `chat_messages`, which covers chat turns rather than fleet executions. So the endpoint reports context-window OCCUPANCY under the name `context_used`, and ent#94's #101 tile must be labelled to match. Presenting this as "tokens consumed" is exactly the liveness-vs-quality mislabel the issue warns against. A guard fails if such a column is ever added, so the schema-change option gets revisited deliberately rather than silently. That guard matches by explicit NAME, not substring: `claim_token` (the #1081 pull-lease CAS value) contains "token" and answers a completely different question — my first version of the check tripped on it. tests/unit/test_ent326_executions_timeline.py — 30 checks, including the dangerous access direction (an empty allow-list returns an empty series, never everything) and the DB layer driven against a real SQLite table. Related to Abilityai/trinity-enterprise#326 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(executions): add the missing db facade pass-through for the timeline (ent#326) CI's regression diff caught this after the rebase: `test_database_facade_delegation` landed on dev after this PR was written, and it fails here because `routers/executions.py` calls `db.get_fleet_execution_timeline(...)` while `DatabaseManager` has no such method. There is no `__getattr__` on the facade, so that call raises AttributeError — the endpoint would have 500'd on every request. The PR's own 524-line suite passed throughout, because every mock of that seam was written as: monkeypatch.setattr(router_mod.db, "get_fleet_execution_timeline", lambda *a, **k: [], raising=False) `raising=False` opts out of monkeypatch's existence check, so the tests stubbed a method that did not exist and never touched the real facade. Green tests over a guaranteed 500. Two changes: the pass-through on `DatabaseManager` (delegating to `_schedule_ops`, the shape its `get_fleet_execution_stats` sibling already uses), and all nine `raising=False` opt-outs removed so the mocks now assert the seam exists. Mutation-checked: with the facade method removed again, 9 of these tests fail — before this commit they all passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(executions): fold and gap-fill in the db layer, with the shared bucket order (ent#326) Three review items. 1. Bucket ordering was alphabetical (`sorted(..., key=bucket)`), which puts `Other` sixth — between `MCP` and `Public`. `_BUCKET_ORDER` exists so `Other` sorts LAST and the legend/stack order is stable, and without it the #1107 Overview chart and this endpoint's tile render the same buckets in different orders on the same page. It is now exported from `db/schedules/__init__.py` (with `_OTHER_BUCKET` and `_bucket_for_trigger`) and used here; a label present in the map but missing from the order sorts after the known ones rather than being dropped. 2. Invariant #1: both Python-side transforms were re-implemented in the router, including `_TRIGGER_BUCKETS.get(..., "Other")` with a LITERAL fallback instead of `_OTHER_BUCKET`. Moved to `ScheduleStatsMixin` beside the query (the #1107 precedent), reached through the `DatabaseManager` facade as `db.shape_execution_timeline(...)` so the router imports no private db symbol. The router no longer mentions `_TRIGGER_BUCKETS` at all, and a test pins that. 3. The legacy-timestamp note is fixed rather than documented. Pre-#1474 scheduler rows are `YYYY-MM-DD HH:MM:SS`: they pass the cutoff and ARE counted by /stats and by group_by=trigger|agent, but `substr(started_at,1,13)` yields `2026-08-06 10`, which never matches the axis key `2026-08-06T10` — so the hour/day chart showed a real zero for executions the stat card above it counted. That is the exact 'chart contradicts the card' failure this endpoint counts `error` as failed to avoid, arriving by another route. Bucketing now normalises the separator with `replace(started_at, ' ', 'T')` (ANSI, present on both backends) instead of waiting ~30 days for retention to age the rows out. 4 tests added (Other-last, order-is-the-shared-constant, router-has-no-copy, legacy-timestamp bucketing). 34 pass. Still needs the entitlement ruling — asked on the PR. Related to trinity-enterprise#326 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(ent#326): record the OSS-core ruling for /api/executions/timeline The gating question was deferred to the maintainer rather than guessed, per CLAUDE.md: an enterprise-tracker feature is entitlement-gated by default unless explicitly ruled OSS-core, because monetization is not the implementer's call. Ruling: OSS-core, ungated. Recorded in architecture.md beside the endpoint so it reads as a decision rather than something inferred later from the fact that the PR merged ungated — which is exactly how an unstated default becomes an accidental precedent. Rationale: generic fleet telemetry over OSS tables, and its consumer (the Dashboard Grid, ent#47) already shipped OSS-core. 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: trinity-ability <trinity-ability@users.noreply.github.com> Co-authored-by: trinity-ability <309458136+trinity-ability@users.noreply.github.com>
dolho
added a commit
that referenced
this pull request
Aug 11, 2026
`schedule_executions` has carried five origin columns for audit since
AUDIT-001, and the backend populates them on every path it owns. The
scheduler is a separate service with its own DB module, and its
`create_execution()` listed none of them in the INSERT — nor accepted
them in its signature, so there was nowhere to put a caller even if one
had been forwarded. Every scheduler-created row was written with all
five NULL.
`triggered_by='manual'` therefore recorded *that* a human ran something
and never *who*. The attribution lived only in backend/MCP-server logs,
bounded by log retention, so past a few weeks the durable record could
not answer "did anyone trigger this run, and who?".
The identity was dropped at three points, not one:
1. the backend's delegating POST sent no body at all, so the
authenticated caller — in scope right there — never crossed the hop;
2. `_trigger_handler` had no parameter to receive one;
3. `create_execution()` had nowhere to put it.
An `ExecutionOrigin` value object is threaded through all three. One
object rather than five parallel parameters at four call depths: five
positional siblings is how one of them silently stops being forwarded.
Two paths the DB fix alone would have left blank are covered too. A
retry inherits the original run's origin — it has no caller of its own,
but a chain of retries that drops the initiator makes the first attempt
the only attributable one; the read is fail-open, since an audit lookup
must not be able to stop a retry from running. A reminder inherits the
provenance #1296 already persisted.
Cron ticks stay NULL. Attributing an autonomous fire to, say, the
schedule's owner would make the column actively misleading — a blank
reads as "unknown", a wrong name does not.
Also hardened while here:
- the untrusted trigger body is validated at the scheduler boundary.
`source_user_id` is dropped rather than coerced when it is not an int:
`bool` IS an `int` in Python, so `True` would have persisted as user
1, a real account attributed to a run it had nothing to do with.
Strings are length-capped and blank-to-None, so "" and NULL are not
two spellings of "unknown".
- the backend prefers the validated `current_user.agent_name` over the
raw `X-Source-Agent` header — the reverse of chat.py's precedence,
which is fine for a collaboration hint but would let a caller pin its
run on a sibling agent in an audit column.
- the MCP trigger tool forwards the origin headers `chat()` already
sends (Invariant #13). Without it an MCP-triggered run attributes to
the key OWNER but not to which key or agent fired it — the part that
identifies the actor when one human owns many of both.
Not a vulnerability: nothing authorizes on these columns.
Backward compatible in both rolling-deploy directions — an old scheduler
ignores the new body fields, and a new scheduler treats a bodyless POST
as an unattributed manual trigger.
tests/unit/test_1970_execution_origin.py — 27 checks, 25 of which fail
against the pre-fix tree.
Related to #1970
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Aug 11, 2026
…ted (#1968) (#1976) * fix(scheduler): record who initiated an execution (#1970) `schedule_executions` has carried five origin columns for audit since AUDIT-001, and the backend populates them on every path it owns. The scheduler is a separate service with its own DB module, and its `create_execution()` listed none of them in the INSERT — nor accepted them in its signature, so there was nowhere to put a caller even if one had been forwarded. Every scheduler-created row was written with all five NULL. `triggered_by='manual'` therefore recorded *that* a human ran something and never *who*. The attribution lived only in backend/MCP-server logs, bounded by log retention, so past a few weeks the durable record could not answer "did anyone trigger this run, and who?". The identity was dropped at three points, not one: 1. the backend's delegating POST sent no body at all, so the authenticated caller — in scope right there — never crossed the hop; 2. `_trigger_handler` had no parameter to receive one; 3. `create_execution()` had nowhere to put it. An `ExecutionOrigin` value object is threaded through all three. One object rather than five parallel parameters at four call depths: five positional siblings is how one of them silently stops being forwarded. Two paths the DB fix alone would have left blank are covered too. A retry inherits the original run's origin — it has no caller of its own, but a chain of retries that drops the initiator makes the first attempt the only attributable one; the read is fail-open, since an audit lookup must not be able to stop a retry from running. A reminder inherits the provenance #1296 already persisted. Cron ticks stay NULL. Attributing an autonomous fire to, say, the schedule's owner would make the column actively misleading — a blank reads as "unknown", a wrong name does not. Also hardened while here: - the untrusted trigger body is validated at the scheduler boundary. `source_user_id` is dropped rather than coerced when it is not an int: `bool` IS an `int` in Python, so `True` would have persisted as user 1, a real account attributed to a run it had nothing to do with. Strings are length-capped and blank-to-None, so "" and NULL are not two spellings of "unknown". - the backend prefers the validated `current_user.agent_name` over the raw `X-Source-Agent` header — the reverse of chat.py's precedence, which is fine for a collaboration hint but would let a caller pin its run on a sibling agent in an audit column. - the MCP trigger tool forwards the origin headers `chat()` already sends (Invariant #13). Without it an MCP-triggered run attributes to the key OWNER but not to which key or agent fired it — the part that identifies the actor when one human owns many of both. Not a vulnerability: nothing authorizes on these columns. Backward compatible in both rolling-deploy directions — an old scheduler ignores the new body fields, and a new scheduler treats a bodyless POST as an unattributed manual trigger. tests/unit/test_1970_execution_origin.py — 27 checks, 25 of which fail against the pre-fix tree. Related to #1970 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(scheduler): return a real execution_id, and 409 when nothing started (#1968) `_trigger_handler` was fire-and-forget: it spawned the run with `asyncio.create_task` and responded immediately, *before* the execution record existed. So it had no id to return. The backend relayed the same id-less fields, and the MCP tool interpolated the missing key — telling every agent `Execution started with ID 'undefined'`, on every trigger, while the execution ran fine. Callers could not correlate a trigger with its run, poll it, or fetch its result; the workaround was to guess from `list_recent_executions` by timestamp. The same ordering hid a second problem. The response was emitted before `_execute_manual_trigger` had even attempted the distributed lock, so a trigger suppressed because the schedule was already running still answered `"status": "triggered"`. A suppressed trigger and a real one were byte-identical to the caller. The handler now acquires the lock and creates the row synchronously, then hands both to the background task. That makes two facts sayable that simply did not exist yet at response time: which execution this is, and whether one was started at all. * 200 carries a real `execution_id`, valid the moment the caller receives it — a fast poller must not 404. * 409 `already_running` replaces the false "triggered", with no id and no row, because nothing ran. Exactly one row per trigger: `_execute_schedule_with_lock` takes the pre-created execution and skips its own create. Two rows would hand the caller an id naming a row that never runs while a second did the work. Because a row can now exist before a gate decides not to run, an abandoned run FAILs its pre-created row rather than leaving it `running` forever — canary E-01's exact signature, and a task the UI would show indefinitely. The handler also now holds the lock across a DB write, which is new, so every exit from that window releases it: creation raising, creation returning None, the run raising, and normal completion — exactly once each. A second release is the dangerous one, since a lock re-acquired by the next run in between would be freed out from under it. Relayed through the remaining surfaces: * the backend forwards `execution_id` (and records it on the audit row, so a trigger and its run are joinable after the fact) and maps 409 rather than flattening it into "Failed to trigger schedule" — a worse lie than the original, since it claims failure where the schedule is healthily busy; * the MCP tool returns a structured `already_running` instead of throwing, so an agent gets a decision it can act on, and GUARDS the id instead of interpolating it — an older backend still omits the field, and swapping one confident lie for another is not a fix; * `ScheduleTriggerResult.execution_id` becomes optional. Typing it as a required `string` while the wire never sent it is precisely why the compiler stayed happy through every `undefined`; * the UI reads 409 as "already running" rather than "nothing was changed — try again", and the CLI prints the id it was already fetching and discarding. tests/unit/test_1968_trigger_execution_id.py — 22 checks, 17 of which fail against the pre-fix tree. Related to #1968 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(scheduler): keep a strong reference to the spawned trigger task (#1968) Self-review finding on this PR. The event loop holds only a WEAK reference to a task, so a bare `asyncio.create_task(...)` whose result nobody keeps can be garbage-collected mid-flight — the asyncio docs say so outright. The bare call predates this PR, but this PR changes what it costs. Before, a collected task meant the run silently did not happen. Now the lock is acquired and the execution row created BEFORE the task is spawned, so a collected task strands a `running` execution whose id the caller already holds and pins the schedule's lock until its Redis TTL. Uses the `_inflight` set + `add_done_callback(discard)` shape the #1083 result-callback path already established (`agent_server/services/result_callback.py`), so the set cannot grow without bound. Guarded by `test_the_spawned_task_is_strongly_referenced`, which checks the reference is held in the window BEFORE the task runs — the only window where collection is possible — and released after. Related to #1968 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(learnings): record the create_task-owns-state class surfaced by #1968 review Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(scheduler): remove the duplicate ExecutionOrigin my rebase introduced My rebase of this branch onto dev left `src/scheduler/models.py` with TWO `class ExecutionOrigin` definitions: this PR's at line 110 and dev's copy (#1974, as merged) at 187. Python keeps the last one, so the second shadowed the first and both of this PR's fixes became dead code — the None-comparing `is_empty` and the SQLite-range guard on `source_user_id`. Three `test_execution_origin_properties` cases went red on head, correctly. Git did not flag it. Both sides added the class at different offsets, so the textual auto-merge took both hunks and reported no conflict — a semantic duplicate that only a reader or an importer would notice. Two things on my side let it through: - I re-read only the files git marked as conflicted, not the whole merged result of a 4-commit rebase. - My post-rebase check was `-k "1968 or ent326 or timeline or scheduler or executions"`, which does not match `test_execution_origin_properties`. The filter was narrower than the blast radius, so 129 tests passed and said nothing about the file I had just broken. Kept the first copy: it is a strict superset (identical fields, `is_empty` comparing against None so `user_id=0` is not reported empty, and the `_SQLITE_INT_MIN/MAX` range check that stops an out-of-range id reaching the INSERT and taking the dispatch down). Deleted dev's older copy. Swept the other six branches I rebased for the same class of damage — duplicate top-level defs in any changed .py — all clean. 350 passed across origin/scheduler/1968/1969/1970/execution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(webhooks): a busy schedule is a healthy 202, not a 503 (#1968) The blast-radius table listed three consumers of the scheduler's `/trigger` endpoint and asserted none regress. There is a fourth — `routers/webhooks.py`, the unauthenticated public trigger — and it is the one that does. With #1968's 409, `if response.status_code not in (200, 202)` caught a HEALTHY, busy schedule and turned it into an ERROR log line plus a 503 'Trigger failed — try again later' to a public caller, advice that only hits the same lock. Worse, the `except HTTPException` arm then called `idempotency_service.fail(idem)`, releasing the #525 dedup claim for a delivery that never failed — so a retry could fire a second execution the moment the lock cleared. Before #1968 that delivery returned 202. Now a 409 is recognised as its own outcome: INFO log, the claim COMPLETED with a snapshot recording what happened, and 202 with `status: "already_running"` and a message saying the delivery was coalesced into the run in flight rather than claiming a fresh execution started. Genuine scheduler errors (500/502/400) still surface as 503 — the carve-out is on 409 alone, and a test pins that it does not widen. tests/test_webhook_triggers.py asserts `status_code in (202, 503)` throughout, which is why CI accepted the regression silently, so the new tests name the status instead of tolerating a set. They run in-process (TestClient + faked scheduler) rather than against a live instance: carve-out removed: 3 failed, 3 passed as shipped: 6 passed Two of the non-blocking notes, both one-liners: * `_abandon_precreated_execution` now passes `expected_status=RUNNING` to a new optional CAS precondition on the scheduler's `update_execution_status`. It was safe by argument (every abandon gate runs before dispatch); #1082 exists to retire exactly that kind of argument, and the clause makes it safe by construction. Default None keeps every other caller byte-identical. * `schedules.ts` logged `execution_id: undefined` above the `!result.execution_id` guard — the exact string this issue is named after. Moved below it. Related to #1968 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Aug 16, 2026
…h sessions, close the portal agent-key hole (#2198) (#2222) * perf(agents): join concurrent duplicate fetches instead of issuing both (#2198) Agent Detail issues the same GET several times on one mount because several independent triggers ask for it at the same moment. Neither existing precedent collapses that: `stores/executions.js` is a RESULT cache, so two simultaneous first-calls still both hit the network, and `stores/fleetGrid.js` is an in-flight SKIP that returns no value, which `loadAgent()` cannot use because it needs the agent object back. Adds the missing third shape — an in-flight JOIN — as one shared primitive (`utils/inflight.js::dedupe`) rather than a fourth bespoke mechanism, and wires `fetchAgent`, `getAgentInfo`, `getAgentDashboard` and `checkDashboardExists` through it. Three semantics, each load-bearing and each covered by a test: - JOIN, not skip: every caller resolves with the winner's value. - Cleared in `finally`: this is a dedupe, NOT a cache, so two SEQUENTIAL calls still issue two requests. `AgentDetail.waitForAgentStatus()` polls `fetchAgent` in a loop and a stale entry would freeze it forever. - Rejection propagates to every joiner and clears the entry, so one failure never poisons the next attempt (`fetchAgent` re-throws and AgentDetail's 404 branch depends on that). Deliberately not a global axios interceptor: that is an app-wide behaviour change, it would MASK genuine repeat-fetch bugs, it must never apply to POSTs, and it would change what e2e `page.route()` interceptors observe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(agents): stop Agent Detail loading everything twice on first mount (#2198) Vue fires BOTH onMounted and onActivated on the first mount of a KeepAlive'd component (`App.vue` includes 'AgentDetail'), so every data call in both hooks ran twice — visible as two `/api/agents/{name}` requests sharing one timestamp. Separately, `checkDashboardExists()` has FOUR triggers (route watcher, status watcher, onMounted, onActivated) and each ran its own 3-step boot retry ladder, for 9 `/api/agent-dashboard/{name}` calls spread over ~9 seconds. Three changes, all in this file: 1. A CONSUMABLE first-activation sentinel. onMounted arms it before its first await; onActivated reads AND clears it as its very first statement, above the `redirectRetiredSessionLink()` early return. Consumable matters: a one-way flag would skip the data half on EVERY later activation, so a KeepAlive revisit would never refresh the agent — the entire reason onActivated exists (#1672), a worse bug than this one, and invisible to a request-count test. Consuming it above the early return matters for the same reason: the retired-link path would otherwise leave it armed. onActivated still runs redirectRetiredSessionLink, applyDeepLinkRouting and startAllPolling unconditionally in both hooks (#1672/#2130/#2153/ent#358). reconcileDeepLinkVisibility and startEmotionCycling are deliberately NOT in that set — both are *consuming*, and running reconcileDeepLinkVisibility before the agent has loaded would judge `?tab=sharing`/`?tab=brain` invisible against a null agent, fall back to Overview and clear the flag, making onMounted's own call a no-op. That regresses #2130 and #2153 silently. 2. One dashboard probe instead of four. The store-level join cannot collapse this — the four triggers fire hundreds of ms apart (measured +0/+326/+396ms) and a promise-join only merges concurrent calls, so the join has to happen at the level of the probe. The probe key carries running-ness, not just the name, because the ladder early-returns on `status !== 'running'`: a probe started while the agent was booting settles without asking, and a "just became running" watcher joining it would strand a slow-starting agent without its Dashboard tab forever. The route watcher drops the probe so agent B never inherits A's answer. 3. The `loading` skeleton gate now compares IDENTITY, not presence (design-system-contract:41-43 — a background refresh of the same entity is invisible, a switch to a different entity animates). A plain `!agent.value` would be a regression: the route watcher resets hasDashboard/agentTags/ authStatus/tokenStats but deliberately never clears `agent.value`, so an A -> B switch would show agent A's data while B loaded. Measured, `acme-sage` (running, no dashboard.yaml — the worst case), full page load then a same-document revisit: page load 65 -> 52 requests revisit 27 -> 22 /agent-dashboard/{a} 9 -> 3 (one ladder, not three) /agent-dashboard/{a}/exists 3 -> 1 /api/agents/{a} 2 -> 1 /avatar/emotions 2 -> 1 /api/agents/{a}/info 3 -> 2 (rest in a follow-up commit) `/api/agents/{a}/activity` x4 is unchanged on purpose — that is correct 5s polling, not a duplicate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(agents): one /info and one /playbooks fetch per Agent Detail mount (#2198) Two more duplicate pairs on the same page, both from a second component independently fetching what a sibling already has. `/api/agents/{name}/info` x3 -> x1. `OverviewPanel.loadSidecars()` issued a RAW `axios.get` for it inside its 10-call `Promise.allSettled`, while AgentDetail's `checkBrainOrbCapability()` asked the store for the same thing from both lifecycle hooks. Overview is the default landing tab, so all three ran on every mount. Routing the sidecar through `agentsStore.getAgentInfo` puts it behind the in-flight join; `loadAnalytics()` two functions above already uses the cached store methods, so the precedent was literally adjacent. Note the deliberate `.data` drop at the assignment: the store returns `response.data` already unwrapped, unlike the nine raw-Axios siblings. Keeping `.data` would have set `info` to `undefined` with no throw and no console error — a permanently blank "About" lead on the default tab. Silent is worse than loud, so it is called out in a comment at the line. `/api/agents/{name}/playbooks` x2 -> x1. ChatPanel fetches the list for <ChatEmptyState> and ChatInput's autocomplete composable fetched the identical list (same endpoint, same `user_invocable` filter) for the slash-command dropdown. ChatPanel is `v-show` in AgentDetail, so it mounts on EVERY tab, not just Chat. ChatInput gains an OPTIONAL `playbooks` prop and skips its own load when it is supplied. `null` (not supplied — fetch your own) is deliberately distinct from `[]` (supplied and empty): `views/PublicChat.vue` is the other <ChatInput> consumer and depends on the composable's public-token path, which ChatPanel never exercises. Defaulting the prop to `[]` would silently kill slash-commands in public chat. A module-scoped cache inside the composable was rejected for the same reason — it would leak across agents and across the public/authenticated boundary, which carry different auth headers. Measured, `acme-sage` (running, no dashboard.yaml), full page load: /api/agents/{a}/info 3 -> 1 /api/agents/{a}/playbooks 2 -> 1 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(settings): fetch feature-flags once per page load, not once per store (#2198) `GET /api/settings/feature-flags` was requested twice on every authenticated page. Not a KeepAlive artifact: two domain-scoped Pinia stores fetch it independently and parse disjoint slices of the SAME payload — `stores/sessions.js` keeps eight booleans and derives `a2aAvailable` from `enterprise_features`, `stores/enterprise.js` keeps only `enterprise_features`. So `sessions.a2aAvailable` and `enterprise.enterpriseFeatures` are two HTTP calls reading one array. Adds `once()` beside `dedupe()` in the shared primitive: an in-flight join PLUS a resolved-value cache. The stronger form is required here and measured, not assumed — the two calls land 319 ms apart, so the second starts after the first has already resolved and a pure in-flight join does not touch them. `once()` is deliberately not the default, and the agent endpoints deliberately stay on `dedupe`: it is only safe for a document that is immutable for the lifetime of a page load. Agent state changes underneath you, and `AgentDetail.waitForAgentStatus()` polls `fetchAgent` in a loop expecting a fresh answer each time. A failure is never cached, so a later caller can retry. Chosen over having one store call the other (the obvious minimal fix): that would require sessions.js to start retaining `enterprise_features` it does not want, couple two domain-scoped stores against design-system-contract:87, and risk an import cycle. Everything that had to survive, survives — enterprise.js's `isAuthenticated` short-circuit stays OUTSIDE the shared fetch (sessions.js has no such guard, so folding them would issue a request where today none is issued), `force` is threaded through so `Settings.vue`'s explicit refresh still refetches, and both stores keep their own `featureFlagsLoaded` flag and public API across all 13 call sites. Blast radius: every page with a NavBar. Measured, `acme-sage`: /api/settings/feature-flags 2 -> 1 per page load. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(nav): drop NavBar's duplicate /api/users/me, keep the admin gate closed (#2198) NavBar fetched the whole user profile on mount to populate `userRole`, which is used for exactly one thing: `isAdmin`. `auth.js::fetchUserProfile` already merges the identical response into `authStore.user`, on both session restore and admin login. In-repo precedent: `MonitoringPanel.vue:278` reads the store "rather than a duplicate /api/users/me round-trip" (#1109). The security direction here is the opposite of the obvious one, so it is worth stating. `initializeAuth()` restores `user` — role included — SYNCHRONOUSLY from `localStorage['auth0_user']`, which is user-editable. So a naive `authStore.user?.role === 'admin'` would not fail closed while the profile loads; it would fail OPEN on a forged value, which is strictly worse than the independent fetch it replaces. So this adds `profileVerified` to the auth store — set true only by a SUCCESSFUL GET /api/users/me, never in the catch, cleared on logout, and never persisted — and gates `isAdmin` on it. That reproduces exactly today's posture, where the nav gate only ever reflected a real server response. It stays a computed, never a read-once, because the store reports `user` before /api/users/me lands (Library.vue:474) and the nav must become admin reactively when it arrives. The gate is cosmetic either way — every admin endpoint is enforced server-side by `require_admin`, which since #1890 also rejects agent principals — so a forged role reveals menu items, not data. It is still worth keeping honest. Measured, `acme-sage`: /api/users/me 2 -> 1 per page load. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(dashboard): stop re-probing an agent that already answered (#2198) The Agent Detail page spent 3 requests over ~9 SECONDS on every load of a running agent with no dashboard.yaml, forever. #2130 recorded that same ladder as what delayed deep-link landing by ~10s. It is the only part of #2198 a user can actually feel. Root cause is missing information, not policy. An agent with no dashboard.yaml replies HTTP 200 with `{"has_dashboard": false, "error": "No dashboard.yaml found at /home/developer/dashboard.yaml"}` — verified live against `acme-sage`. An unreachable agent produces `{"has_dashboard": false, "error": <string>}` too. The two were byte-indistinguishable, so the only safe frontend behaviour was to assume the transient case and retry at 0s / 3s / 9s. `GET /api/agent-dashboard/{name}` now carries `settled: true` when the agent ran its handler and answered, and the retry ladder stops on it. Three properties worth stating: - Derived from the TRANSPORT, not the error text. Only an HTTP 200 with a parseable body reaches that line, so it is correct on every already-deployed agent image and needs no base-image rebuild — unlike adding a reason code agent-side, which would be absent on every existing container. - Every inconclusive path stays unsettled. Timeout, connection error, non-200 and a stopped container all route around it, so a still-booting agent keeps its retries. This is the fail-safe direction and it is the load-bearing assertion in the tests: a false positive here would permanently hide the Dashboard tab of a slow-starting agent, with no retry able to recover it. - The frontend treats an ABSENT `settled` as today's behaviour, so an old backend with a new bundle simply keeps the ladder. This supersedes the plan's proposed `/exists` tri-state, which aimed at the same 9 seconds from a worse position: `/exists` reads `agent_dashboard_cache`, which only ever records POSITIVES (`config_json` is NOT NULL), so persisting "known: no dashboard" would have required a new column, a dual-track migration (Invariant #9: SQLite `migrations.py` AND an Alembic revision), and it would still have paid the full 9s on the FIRST load of every agent because nothing is cached yet. Fixing it where the information already exists costs no schema change and removes the 9 seconds on the first load too. Measured, `acme-sage` (running, no dashboard.yaml), full page load: /api/agent-dashboard/{a} 9 -> 1 (was 3 after the probe-dedupe commit) /api/agent-dashboard/{a}/exists 3 -> 1 Backend verified by `tests/unit/test_2198_dashboard_settled.py` (7 tests, all paths). Frontend reader verified end-to-end in the browser by fulfilling the real backend response with the field merged in, since the local stack's backend does not yet carry it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(workspace): one viewer-scoped call for the sidebar's thread list (#2198) `GET /api/enterprise/client-portal/sessions` returns every thread the caller has, across every agent on their roster. The Workspace sidebar renders a merged, cross-agent, recency-sorted list, so it asked the per-agent route once per rostered agent — literally N+1 — from all six `refreshThreads()` call sites, including every thread open and every completed turn. Each of those cost 2-3 DB queries, because `list_sessions` re-resolves the roster through `agent_on_roster` before it touches the session table. This resolves the roster once and issues one query. Access boundary made STRUCTURAL, not conventional. `agent_on_roster` did not call `_roster_rows`; it independently re-queried the same two rosters. Collapsing N gated reads into one would have left the batch's scope depending on two functions staying set-equivalent by convention. Both now go through `roster_agent_names()`, and a test asserts the equality directly rather than sampling it. `agent_name IN (:agents)` is the tenant scope, not an optimisation: filtering on `client_email` alone would re-surface threads for an un-shared agent, which the per-agent gate hides. `include_owned = principal.is_platform`, tested BOTH ways. Deliberately not modelled on `search_chats`, which reads only the shared roster and so silently omits a platform user's own agents — a real separate defect, to be filed, and inheriting its shape would have put the same hole in the sidebar. Details that are each a verified trap: - `def`, not `async def`, matching `portal_sessions`: pure sync DB work, so FastAPI runs it in the threadpool and it cannot block the event loop. - `client_email = :email` with the lowercasing python-side at the bind, like `list_portal_sessions` — NOT `lower(col) = :email` like `search_portal_sessions`, which puts a function on the column. - `agent_name` is SELECTed; the per-agent query omits it because the caller already knew it. - Chunked at 500: an expanding bindparam emits one placeholder per agent and SQLITE_MAX_VARIABLE_NUMBER is 999 on SQLite < 3.32, so a large fleet would have turned today's always-working N queries into a hard 500 on the bootstrap path. Chunking then breaks the global ORDER BY — each chunk is sorted independently — so the rows are re-sorted on the same key past the threshold. That was caught by its own test, not by review. - Empty roster short-circuits before any SQL (the bindparam raises on []). - Rate-limited per viewer (`portal_sessions_all:{email}`, 120/60s). There is no global limiter middleware, and this becomes the hottest authenticated read in the Workspace — no longer even incidentally throttled by the browser's per-host connection cap, and in production behind cloudflared (HTTP/2) there is no such cap at all. No cap and no `total`, deliberately: today's per-agent query is unbounded and runs N times, so this ships the same row volume in one request. Adding a cap would be a NEW behaviour that collides with the starred-chat pinning guarantee in requirements §5.10 — a pure recency LIMIT can drop a starred-but-old thread out of the pinned section. That deserves its own issue, not a side effect here. No schema change, no migration, no new index. The only index on the table is (agent_name, client_email, last_message_at), so `client_email = ? AND agent_name IN (...)` resolves as one index seek per agent with both leading columns on equality — the same plan each of today's N queries already gets, once instead of N times over HTTP. Invariants: #1 (router thin: auth + rate limit + error map), #4 (declared beside the other viewer-scoped literals; this router has no top-level catch-all), #8 (no agent parameter, so no existence oracle — strictly less enumerable than the route it replaces), #9 (n/a), #13 (no MCP tool: no client-portal route has one, and an MCP key cannot hold a PortalPrincipal), #14 (models in `client_portal/models.py` — the guard globs `routers/*.py` and never sees this package). Coordinated with #2196: this touches no function it touches — `get_roster`, `_agent_briefing`, `_row_to_card` and `_roster_rows` are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(workspace): sidebar loads its threads in one call, and survives failing (#2198) `fetchAllSessions` now issues one `GET /client-portal/sessions` instead of one per rostered agent. Constant in roster size, on all six `refreshThreads()` call sites — bootstrap, every thread open, every completed turn. The count is the easy half. Collapsing N calls into one INVERTED a failure mode, and this commit is mostly about that: Before, `fetchAllSessions` could not reject. Every per-agent call carried its own `catch { return [] }` — the in-code comment says why, "one down agent never blanks the whole list" — and `refreshThreads` `Promise.all`s it while only `fetchChatState` was caught. One request cannot degrade per agent, so a single 500 would (a) blank a populated sidebar, which design-system-contract :43/:55 forbid, and (b) reject out of `bootstrap()` BEFORE `resolveAgentQuery()` and the deep-link `sessionId` branch — breaking Workspace deep-link landing entirely, on the most client-visible surface in the product. So: the store catches internally and returns its LAST GOOD list rather than an empty one, raising `sessionsFailed` for an honest banner; `refreshThreads` catches both halves as a belt on the bootstrap property. Four tests cover it, including that a 5xx does NOT fan out (that would turn one failure into N). Two constraints pinned by existing tests, both preserved: zero GETs on an empty roster (`workspaceRoomsGate.spec.js` F17b breaks if the batch fires unconditionally), and every thread carrying `agent_name`, most-recent-first (`workspaceAgentLanding.spec.js`). `agent_name` now arrives from the DB row instead of being stamped on client-side from `this.agents`, so the list is filtered to the DISPLAYED roster. The backend scopes by the caller's roster — that is the access boundary and it is not duplicated here. This narrower filter is a rendering rule: a thread whose agent the sidebar does not show would route nowhere. Today the two sets are identical so it is a no-op that preserves rendering exactly, and it keeps the sidebar correct whatever #2196 decides about hiding container-less agents. A drop is logged in dev, because it means the two rosters have diverged. TRANSITIONAL: a 404 falls back to the per-agent fan-out, for the deploy-skew window where a cached bundle reaches a backend without the route. 404 only — a 5xx already degrades correctly above. Delete one release after this ships. Behaviour change to name rather than let someone discover: today one unreadable agent degrades alone; with one query the read is all-or-nothing. Acceptable — it is a single indexed read, not N agent round-trips — but it is a real change in failure granularity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(agents): pin the dedupe semantics and the sentinel's consumability (#2198) Two specs, plus a correction to a claim I made in the primitive's own docstring. `agentDetailFetchDedupe.spec.js` — behavioural, over Pinia + a mocked axios. The three semantics that a naive "just cache it" gets wrong: a JOIN returns the value to every caller (an in-flight SKIP would leave one `undefined`, which is why `fleetGrid`'s shape was not reusable), two SEQUENTIAL calls still issue two requests (`waitForAgentStatus` polls `fetchAgent` in a loop and a sticky entry freezes it forever), and a rejection reaches every joiner while clearing the entry so the next attempt is not poisoned. Plus `once()`'s extra contract: answers a later caller from memory, honours `force`, never caches a failure. `agentDetailMountDedupe.spec.js` — source-structure, in the shape of `agentDetailDeepLink.spec.js` and the Python AST guards, because there is no component-mount harness in this project and every property is about statement ORDER inside a lifecycle hook. The assertion that earns its keep is that `onActivated` CLEARS the sentinel: a one-way flag passes every request-count test on a fresh load and silently disables the KeepAlive revisit refresh, which is a worse bug than the one being fixed. `learnings.md:189` records the same lesson from #1804 — the transition that gets missed is the one where the before and after states are equal. Both suites were mutation-checked rather than assumed. Reverting each fix in turn fails exactly the guard that owns it: one-way sentinel -> "CONSUMES it"; `!agent.value` gate -> "identity-aware"; hoisting reconcileDeepLinkVisibility above the guard -> "the skip guards the DATA half only"; dropping running-ness from the probe key -> "keyed on running-ness"; removing the store join -> "two concurrent callers issue ONE request". CORRECTION to `utils/inflight.js`: its docstring claimed no existing precedent joins an in-flight promise. That was wrong. `src/api.js:71` (`deduplicatedGet`, PERF-269) is exactly that — same shape, keyed on URL+params, cleared in `.finally()`. It went unnoticed because the calls in question use raw `axios` with an explicit `authStore.authHeader` rather than the `api` instance (the widespread Invariant #7 deviation), so it never applied to them. Moving the store onto `api` would have been the smaller diff and is rejected for a stated reason rather than an oversight: `api`'s response interceptor hard-redirects to `/login` on ANY 401, while AgentDetail deliberately renders its own error banner and its own 404 panel (#1914). That migration is an Invariant #7 cleanup, not a request-count fix. The docstring now says all of this. (Found while writing the tests: the same `api.js` reassignment is why this spec's axios mock must return a DISTINCT object from `create()`, unlike the portal specs — otherwise `api.get = …` lands on the shared mock and clobbers `axios.get`.) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(e2e): assert the page fetches each endpoint once, and still polls (#2198) Two @smoke specs. `@smoke` is not optional: `frontend-e2e` runs `test:e2e:smoke` only, so an untagged spec never executes in CI. Even tagged it cannot block a merge (the workflow is advisory) — which is why the load-bearing assertions for this issue live in vitest and pytest. These are the end-to-end confirmation, not the gate. `agent-not-found.spec.js` gains AC #3: the missing agent is fetched ONCE. It is the natural home — it already owns a `page.route('**/api/agents/*')` handler with the URL-shape test that isolates the single-agent GET from `/api/agents` and `/sync-health` — so the extension is a counter inside a handler that exists. Its sibling 500 test gains a note: that interceptor now fires once instead of twice, which is deliberate (the rejection still propagates to every joiner), not a symptom. `agent-detail-request-dedupe.spec.js` asserts no agent-scoped endpoint is fetched twice per mount, with two explicit exemption lists — the pollers, and the dashboard boot ladder, which may legitimately spend up to 3 requests when the agent's answer is inconclusive but never one ladder per trigger. Its second test is the AC #5 guard and is the more interesting one: `/activity` appears ~4x in a 20s window and looks exactly like the bug being fixed. It is not. So a future "cleanup" that silences it has to fail here. Both were checked against unfixed code rather than assumed. Against the unmodified tree the dedupe test fails naming all eight classes verbatim (`/api/users/me x2, /api/agents/{a} x2, /api/settings/feature-flags x2, /exists x3, /playbooks x2, /info x3, /avatar/emotions x2, /agent-dashboard/{a} x9`) and the AC #5 test PASSES — which is the correct split for a guard whose job is that correct behaviour stays correct. Two test defects were found and fixed by that exercise rather than shipped: - the activity window ran from `goto`, so it measured load latency as much as interval and flaked under parallel workers on a loaded instance. It now opens after the agent has rendered. - it asserted EVERY gap > 2s, which failed on unfixed code for a 66ms start-up burst — real, but incidental timing this change does not control, so it would have flaked. It now asserts that at least one gap falls in a plausible ~5s band, i.e. that an interval is running. Pre-existing failures recorded, both verified identical on the unmodified tree and untouched by this branch: `dashboard-type-filter.spec.js` (1 failed — owned by #2200, in flight) and the #2199-owned `workspace-absorbs-session.spec.js` / `continue-as-chat.spec.js` (2 failed, 3 passed — the missing `testfix` fixture). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: record the request-dedupe changes in the flows they belong to (#2198) Tiered per Rule of Engagement #4, and decided at plan time rather than derived after the fact. `feature-flows/dynamic-dashboards.md` — the flow that owns the dashboard probe and its retry ladder. Its Data Flow steps 1 and 2 both became wrong: the visibility check now joins ONE probe across its four triggers, and the ladder stops on a `settled` answer. Both entries say WHY the obvious simpler shape does not work — a store-level in-flight join cannot collapse triggers that fire hundreds of ms apart, and the probe key has to carry running-ness or a slow-starting agent loses its Dashboard tab permanently. Also disambiguates the two same-named `checkDashboardExists` functions (component vs store). `feature-flows/agent-overview-dashboard.md` — its ASCII call diagram named `GET /api/agents/{name}/info` inside `loadSidecars()`. That call now resolves through the store, which is the whole reason the duplicate collapsed. `feature-flows/workspace-sidebar-ia.md` — a new step 0 for how the list is loaded at all, which the doc never covered. Records the three load-bearing properties (the roster set is the tenant scope; the batch must not become a single point of failure; the client-side filter is a rendering rule, not a second access check), the deliberate absence of a cap, and the two honest downsides: the change in failure granularity, and the transitional 404 fallback. `architecture.md` Workspace block — the API change, in the prose form that block uses (client-portal routes are not in the endpoint tables). `requirements/core-agent.md` §5.10 — the Endpoints bullet gains the new route. Additive; §5.10's behaviour is unchanged. No requirements change beyond that bullet: behaviour is the same, cost is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(app): correct the KeepAlive rationale, drop a dead include entry (#2198) The C3 investigation's artifact. #2198's plan escalated a question to the human: should AgentDetail be KeepAlive'd at all? Six of the eight duplicate classes were attributed to it, and its stated justification — "preserves terminal WebSocket connections" — refers to a tab that is deprecated and hidden. The answer was to investigate first, then decide. DECISION: it stays. The justification was stale; the behaviour is not. Verified stale: the Terminal tab is commented out of `visibleTabs`, `TerminalPanelContent` is imported by `AgentDetail.vue` and never rendered anywhere in its template, and `terminalRef` is bound to nothing. There is no terminal WebSocket to preserve. Verified live, and each of these would have broken: - `e2e/schedules-toggle-scroll.spec.js:203-212` names this caching as a "PREMISE (load-bearing)" in its own words: had AgentDetail remounted, its T4/T5 "would pass even with the watcher-clear and loadSeq guard deleted". Removing the include does not fail that spec — it silently converts two shipped regression tests into tautologies, which is worse than a red test and invisible to every metric. - `ChatPanel.onUnmounted` calls `closeSSE()` and stops an active voice session. ChatPanel is `v-show`, so it is mounted on every tab and today survives navigation. Un-caching makes navigating away kill an in-flight chat stream and end a live voice call. - `activeTab` is a local ref that is never URL-synced, so every revisit would reset the user to Overview. And the premise itself did not hold. Measured on `acme-sage` (running, no dashboard.yaml), same-document revisit via SPA push + history.back(): page load revisit KeepAlive ON 65 27 KeepAlive OFF 57 47 It removes 2 of the 8 duplicate classes, not 6 — classes 6-8 (feature-flags, users/me, playbooks) never involved KeepAlive at all, and 3-5 are driven by the route and status watchers, which fire either way. So it buys 6 requests once and costs 19 on every revisit, and `learnings.md:118-120` records the cached-revisit path as "the common path". The duplicates are fixed at their sources instead. Two changes here, both no-ops at runtime: - the comment now says what is actually true, and why, so the next reader does not have to re-derive it; - `'SystemAgent'` leaves the include list. It matches no component anywhere in `src/` (verified by grep — the only occurrence was this line), because `/system-agent` redirects to `/agents/trinity-system`, i.e. to AgentDetail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(portal): agent-scoped keys can no longer traverse the Workspace as their owner (#2198) Plan §12.5 E7, answered rather than shipped silent. get_portal_principal's platform branch resolves the caller through get_current_user, which resolves an agent-scoped MCP key to its OWNER carrying the owner's role (the ent#293/#297 trap). Any agent's injected TRINITY_MCP_API_KEY therefore reached all 30 portal routes as is_platform=True — a REST path around the MCP layer's agent-to-agent permission matrix (it could read the owner's threads with agents the calling agent holds no agent_permissions edge to) — and the new batch route amplified exfiltration from N calls against N discovered names to ONE call returning the owner's whole cross-agent thread index. Fixed at the dependency, not the route, so every current and future portal route inherits it. reject_agent_principal, deliberately not the stricter reject_non_interactive_principal: the portal is a *use* surface, so a user's own user-scoped key scripting their own Workspace stays legitimate, and scope='system' keeps platform breadth by design. Connector and portal_delegate keys were already fenced to their own routes inside get_current_user. There is no legitimate agent caller to break: no MCP tool targets this surface and neither agent images nor the base image call it (verified by grep). Tests: agent-scoped principal → 403 at the boundary; a user-scoped key and a JWT still pass as platform; a portal session token never touches the fence. All 306 portal-adjacent unit tests stay green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: trinity-ability <309458136+trinity-ability@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 16, 2026
Open
4 tasks
vybe
pushed a commit
that referenced
this pull request
Sep 2, 2026
…tecture.md counts (#2238) A full /validate-architecture run on 9b0ed63 found Invariant #13 unenforceable — 44 of the non-excluded routers had neither a same-named MCP tool module nor a `# mcp: none` marker — and seven architecture.md counts more than 25% stale. - Every router outside the by-design exclusions (internal/setup/auth/public/paid) now opens with a `# mcp:` header: 30 x `none — <reason>` (admin, grant-vs-use human-only, UI-only) and 14 x a pointer to the covering tool module (`agents.ts (rename_agent)`, ...), so the validator can tell "unexposed on purpose" from "forgotten". Comments only; each module docstring stays the first statement (AST-checked). - architecture.md: router / service / tool-module / tool counts, agents.py size, AGENT_REFS cascade width, compatibility check count, four endpoint-group counts; the MCP tools table gains its three missing modules (a2a, connector, rooms) and the chat/skills rows catch up; the dead "DEPRECATED Redis credential keys" bullet (zero readers or writers) is removed; Invariant #13 records the header convention. - CLAUDE.md: MCP tool count and endpoint/router count. Related to #2238 — the remaining #6/#7/#15/#18 gaps stay tracked there. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AQHbGS2GV78AmBFnZX78j6
vybe
pushed a commit
that referenced
this pull request
Sep 2, 2026
…tecture.md counts (#2238) (#2482) A full /validate-architecture run on 9b0ed63 found Invariant #13 unenforceable — 44 of the non-excluded routers had neither a same-named MCP tool module nor a `# mcp: none` marker — and seven architecture.md counts more than 25% stale. - Every router outside the by-design exclusions (internal/setup/auth/public/paid) now opens with a `# mcp:` header: 30 x `none — <reason>` (admin, grant-vs-use human-only, UI-only) and 14 x a pointer to the covering tool module (`agents.ts (rename_agent)`, ...), so the validator can tell "unexposed on purpose" from "forgotten". Comments only; each module docstring stays the first statement (AST-checked). - architecture.md: router / service / tool-module / tool counts, agents.py size, AGENT_REFS cascade width, compatibility check count, four endpoint-group counts; the MCP tools table gains its three missing modules (a2a, connector, rooms) and the chat/skills rows catch up; the dead "DEPRECATED Redis credential keys" bullet (zero readers or writers) is removed; Invariant #13 records the header convention. - CLAUDE.md: MCP tool count and endpoint/router count. Related to #2238 — the remaining #6/#7/#15/#18 gaps stay tracked there. Claude-Session: https://claude.ai/code/session_01AQHbGS2GV78AmBFnZX78j6 Co-authored-by: trinity-ability <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Sep 3, 2026
…fied twice, symlinks preserved, named errors, idempotent + locked (#2060) (#2485) * docs(requirements): deploy-local integrity contract — new §4.1.2 (#2060) Rule #1 (requirements before implementation): the embedded-manifest integrity contract, layered requiredness, the accident-proof-not- adversary-proof honesty note (token-bound tool argument; CLI escape), the symlink decision matrix, the security-vs-drift layering rule, caps, idempotency scope + per-base-name deploy lock, evidence response fields, and the residue/compensation guarantees. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(deploy): embedded-manifest integrity verification, symlink contract, caps + compensation (#2060) deploy_local_agent silently deployed incomplete agents: the archive rides the calling model's own turn and nothing compared what landed against what was on disk — a pruned-but-well-formed archive deployed status:"success". - Embedded .trinity-manifest.json (computed from the disk tree, shipped inside the archive) verified POST-EXTRACT (before any side effect; extras are drift) AND POST-COPY (before the .env credentials merge). Fail-closed 400 MANIFEST_DRIFT naming missing/altered/extra/ link-mismatched paths (capped lists + counts); recovery text directs at rebuild-without-excludes / the CLI, never at pruning the manifest. MANIFEST_REQUIRED (flag set, no manifest — carries the generation snippet) and MANIFEST_INVALID (5 MB read cap, entry bounds, dup/ traversal/self-listing) complete the taxonomy. - Symlink contract: copytree(symlinks=True) preserves in-root links end to end (previously silently dereferenced); dangling in-root links preserved with a named warning (previously an opaque shutil.Error 500 + partial-dir residue); escape refusals unchanged and regression-pinned; extractall(filter='tar') pinned ahead of the Py3.14 default flip. - Caps: MAX_FILES 1000→10000; new MAX_EXTRACTED_SIZE 500 MB from member headers pre-extraction (gzip-bomb hole); observed+limit on every cap rejection; macOS AppleDouble ._* members skipped with a warning. - Evidence-bearing response: verified / files_expected / files_deployed / symlinks_deployed + fail-open #668 STATIC compatibility_hard_count (unavailable → None + warning, never 0, never a failed deploy). - Idempotency-Key on POST /api/agents/deploy-local (scope agent_deploy:{user_id}, mirrors agent_create incl. the #2040-F3 staleness branch; in-flight → 409 DEPLOY_IN_FLIGHT) — an un-keyed transport retry forked twice (my-agent-2 AND my-agent-3). Per-base-name agent:deploy_op: SETNX lock (fail-open, 409 DEPLOY_IN_PROGRESS), registered in agent_runtime_state.EXEMPT_KEYSPACES. - Residue + compensation (#2006 class): dest_created assigned BEFORE the rmtree/copytree pair so a mid-copy failure is cleaned (named 500 TEMPLATE_COPY_FAILED); failed deploys reclaim the prepopulated workspace volume (label + unattached double-guard, #1581 shape) and restart the previous version they stopped (incl. create_agent_fn raise); stale prepop volume: unattached → remove-and-recreate (put_archive overlays, never prunes), attached/unknowable → 409 WORKSPACE_VOLUME_IN_USE; catch-all 500 gains code DEPLOY_FAILED. Security layering preserved: containment/link/member-type validation stays strictly PRE-extraction in _validate_tar_member; only manifest drift verification runs post-extract. TDD: tests/unit/test_2060_deploy_integrity.py written RED-first against the pre-fix base (44 of 49 failed; the 5 passers are deliberate regression / static pins). Live-stack e2e classes appended to tests/test_deploy_local.py for the FULL verify stage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(deploy): manifest-aware MCP tool + CLI — third/fourth surfaces of the #2060 contract MCP tool (Invariant #13): description rewritten with the embedded-manifest generation snippet (same excludes as the tar command, writes .trinity-manifest.json INTO the agent dir), the COPYFILE_DISABLE=1 macOS note, the caps, and the HONEST token-bound payload ceiling directing large agents at the trinity CLI / curl from bash. execute() sets require_manifest: true in tool CODE (not a model-controlled parameter) and derives a deterministic Idempotency-Key over [userId, tool, name, archive] via deriveMcpIdempotencyKey + extraHeaders — same-args-only by design (a rebuilt tar is deliberately a new deploy). Response interface carries the new evidence fields. New agents.deploy.test.ts pins body/require_manifest, key determinism, and the description contract (5 tests; suite 201 pass, tsc clean). CLI (fourth consumer): computes the manifest during its existing archive walk (git ls-files + walk modes; symlinks stored as link entries with link_target, files with sha256), injects it into the tar IN-MEMORY (never mutates the user's source dir), sends require_manifest: true, and prints the verified/counts evidence + compatibility hard-finding count + warnings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(flows): local-agent-deploy #2060 update + architecture rows - local-agent-deploy.md: manifest workflow (Step 0 snippet consistent with the tar excludes beside it), post-extract/post-copy verification steps, deploy lock, dest_created-before-copy, stale-volume hygiene, compat gate, compensation, refreshed error-code/size-limit tables, security layering + honesty notes, refreshed stale line refs (agents.ts block moved 556-643 → 744+). - feature-flows.md: dated #2060 row + index one-liner. - architecture.md: POST /api/agents/deploy-local added to the Agents endpoint table (pre-existing gap) with the integrity-contract summary; agents.ts MCP tools row touched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(deploy): review fixes — close the compensation window at create-return, name the NUL-path refusal, strict require_manifest read, shared lock-release predicate (#2060) Four review findings fixed in place: 1. Compensation over-restart: once create_agent_fn RETURNS, the new version is live — a failure past that point (realistically only response construction; the compat gate is fully fail-open) must not restart the previous version alongside it, or one base name runs two live versions (the F5 double-run hazard: both fire schedules). previous_stopped_name is now cleared at create-return; the restart-on-create-raise path is unchanged. Pinned by test_late_failure_after_create_does_not_restart_previous. 2. NUL byte in a manifest path escaped the named 400: os.path.lexists raises ValueError("embedded null byte") during verification, landing as the catch-all 500 DEPLOY_FAILED instead of MANIFEST_INVALID. Refused at parse with every other bad shape; new nul-byte parametrize case. 3. require_manifest read strictly (body.require_manifest) instead of getattr-with-default: DeployLocalRequest owns the field (default False) and the router is the only production caller — a duck-typed body missing it should fail loudly, not silently un-require verification. The two test harness body stubs now model the field. 4. Deploy-lock release uses the shared redis_breaker_util.lock_token_matches predicate (#1919: one ownership comparison for hand-rolled single-flight locks; carries the bytes belt if client decoding config ever drifts). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(deploy): compensation window closes at create-return — align §4.1.2 + flow with the review fix (#2060) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(deploy): adopt the shared SingleFlightLock for the per-base-name deploy lock (#2060) The branch forked before #1920 consolidated the SETNX single-flight idiom onto `redis_breaker_util.SingleFlightLock`, so its hand-rolled `set(nx=True, ex=...)` + GET-then-DELETE release now trips `tests/unit/test_1920_no_hand_rolled_single_flight.py`. Adopt the primitive instead of allowlisting: same key (`agent:deploy_op:{base_name}`), same TTL, same fail-open on Redis down, same 409 `DEPLOY_IN_PROGRESS` on contention — but the token/compare-and-delete logic now lives in one place. The `_deploy_lock_client` seam is kept as the site binding the primitive's injected-client contract asks for (and the unit tests monkeypatch). Docs: requirements §4.1.2, the feature flow (6c + changelog dated to the landing), and architecture.md's SingleFlightLock consumer/site lists name the adoption. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GJdf1sDRofuPxKf8cmwRh1 * fix(deploy): make the dest_path containment guard the barrier shape CodeQL proves (#2060) CodeQL raised 10 py/path-injection alerts on the PR, every flow request body → base_name → version_name → dest_path → {copytree, the post-copy manifest walk: lexists/is_symlink/readlink/is_file/sha256}. The #950 guard already normalizes dest_path and refuses an escape, but its condition (`!= base and not startswith(...)`) leaves the fall-through branch unproven, so the sanitizer never applied and the new post-copy sinks surfaced it. Rewrite it in the `not startswith(...) or == base` form that `_remove_partial_deploy` uses and documents as the recognized barrier — identical accept/reject set, no behavior change; the pre-existing guard simply becomes visible to the analysis that the new manifest walk exposed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GJdf1sDRofuPxKf8cmwRh1 --------- Co-authored-by: trinity-ability <309458136+trinity-ability@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Sep 8, 2026
…wn negations, and a Push reports what it untracked (#2529) (#2595) * test(git-sync): failing regression gate for the .gitignore precedence bug (#2529) The canonical `_GITIGNORE_PATTERNS` block is appended to the END of an agent's `.gitignore` on every Push, and git is last-match-wins, so it silently reverses every `!negation` the agent wrote above it — and `_build_rm_cached_ignored_command` then untracks the files those negations were protecting. Two tests, both red against the real builders and real git repos: - `.env.example` (compat F-004 requires it) negated by the agent, with an unrelated rule after the negation so the win cannot come from being last - `.claude/settings.json`, the escape hatch #2036's own rationale offered TWO consecutive Pushes is the load-bearing shape: on a pre-canonical repo the file is already gone by the second Push, so a single-Push assertion can pass with the bug live. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(requirements): 11.15 — canonical .gitignore precedence + sweep reporting (#2529) Rule #1: the requirements entry lands BEFORE the implementation. Records the two-region shape and why one block provably cannot carry both defaults the user may override and guarantees the user may not; AC-1 rule (b) and why not rule (a); the dir-form residual with its rejected contents-form alternative and that alternative's cost; the three report fields across five surfaces and which one outlives the session; and the decision that `.trinity/operator-queue.json` stays runtime state rather than a commit every 15 minutes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(git-sync): rebuild .gitignore into two managed regions so agent negations win (#2529) The merge is now a normalize-and-rebuild instead of an append: every managed line is stripped wherever it appears, then the file is written as [defaults][user region, original order][protected floor], and only written at all when the computed content differs. An agent's `!.env.example` / `!.claude/settings.json` now beats a canonical default without having to be the file's last line. It does NOT beat the floor: the six credential patterns with their two canonical negations, plus `.trinity/*` and the eight `!` re-includes derived from `_TRINITY_AUTHORED_PATHS`, sit BELOW the user region — because a single hoisted block would flip every currently inert `!.env` in the fleet live in one unattended Push (measured), and would let a user `*.sh` beat `!.trinity/setup.sh` (trinity-enterprise#76 / #1704, measured). `!.env.example` and `!.mcp.json.template` join `_GITIGNORE_PATTERNS` at the tail, where the constant's order becomes exactly the agent guide's fence order. Shell hardening, each reproduced before it was fixed: the strip grep is a real command with an explicit rc<=1 check (a failure inside a process substitution is invisible and the mv then destroys the user's rules); `LC_ALL=C grep -a` (a NUL byte otherwise drops the whole user region while exiting 0); the strip list carries every line bare AND CR-suffixed; `[ -e ] || : >` instead of `touch`; and an import-time reject of empty or newline-bearing managed lines. Three report probes ride on the two execs the Push already ran — removed, untracked-before/after, and a `git check-ignore -v` shadow probe whose verdict comes from the deciding pattern text, never the exit code (check-ignore exits 0 even when a negation decides). Verified against real git repos: idempotent (byte-identical second run), floor beats the user's `!.env` and `*.sh`, user negation wins for `.claude/settings.json`, user rules keep their order, and the dir-form residual (`content/keep.md` under `content/`) is swept AND reported. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(git-sync): report what a Push untracked, on every surface (#2529) `_migrate_workspace_gitignore` returned `None`, so nothing downstream could tell "5 files pushed" from "5 files pushed, 4 silent deletions" — which is why both field incidents surfaced two months late. It now returns a `GitignoreSweep` (removed / unignored / shadowed), and every failure path returns the empty one so a caller can read the fields unconditionally. `sync_to_github` attaches it to ALL FOUR returns — 200, 409, non-200 and the exception path — because the index mutation happens before the HTTP call, so a conflict is exactly as obliged to report it as a success. The router's HTTPException keeps only `detail`, so the one-line summary is folded into `result.message` inside the service rather than special-cased per status code. Surfaces: the API response (the hand-built router dict drops anything not named, so all three keys are listed), the `_audit_git` details on both the success and failure paths (#905), the commit message (best-effort — `git rm --cached` only stages, and if the auto-sync loop commits first the deletions ride in someone else's commit, which is exactly what `47efd80` was), and an operator-queue `gitignore_untracked` entry, which is the only one that outlives the session. Guards moved with the fix: - `test_doc_and_constant_in_sync` is now set EQUALITY. Asserting `constant ⊆ doc` is how the guide carried `!.env.example` for months while the constant did not. - `ALLOWED_NON_CANONICAL` shrinks to empty (AC-3): the reviewed delta between the guide's fence and the constant no longer exists. - The 14 bundled templates are regenerated as the merge's own fixed point, which is what preserves #1908's byte-identity property that #953 depends on; a flat canonical list is no longer its own fixed point, so the #2069 no-drift seed is reseeded to block shape and its contract restated. - `test_ent123`'s hand-written `_GitSyncResult` stand-in gains the three fields and a `model_copy` — a closed kwarg list raises TypeError at the very return that test asserts on. - New `test_guide_fence_order_matches_the_constant`: the fence a template author copies must produce the file the platform writes, order included. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(mcp,ui): surface the .gitignore sweep on the MCP result and the sync toast (#2529) Invariant #13: the backend router, the agent server and the MCP tool are three surfaces of one contract, so `git_sync`'s description now names removed_paths / unignored_paths / shadowed_negations. No `client.ts` / `server.ts` change is needed — `run()` JSON-stringifies the client result wholesale and `gitSync` returns `Promise<unknown>` — but that passthrough is now a pinned regression test rather than an assumption, because an agent reading only `files_changed` cannot tell a 5-file push from a 5-file push that also untracked two committed files. The toast names the removal count in the same breath as the files pushed, on the success, no-changes and failure paths alike (the sweep runs before the agent call, so "nothing to commit" and "the index was mutated" are not mutually exclusive). Type stays 'success' on a successful sync: the toast host is a binary success/danger ternary, so anything else paints a completed verb in the danger token. The durable surface for a removal an operator must act on is the operator-queue entry, not a 3-second toast. Verified: `node --import tsx --test src/tools/git.test.ts` 10/10 and `npx tsc --noEmit` clean in src/mcp-server; `npm run test:unit` 2175/2175 in src/frontend plus the 5 new specs; raw-color scan unchanged (the composable carries no colors and is not in the baseline). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(git-sync): pin precedence, the protected floor, and the sweep report (#2529) 31 tests over REAL git repos, because the defect lives in git's own last-match-wins and dir-descent semantics, which a mocked container cannot express. Modelled on `test_2070_trinity_authored_paths.py`'s harness, and the report is produced by the exact two commands the backend runs — not a second, test-only code path. Precedence: `.env.example` and `.claude/settings.json` survive two consecutive Pushes with an unrelated rule after the negation; corbin's `!**/.env.example` at nested depth; the defaults block sits above the user rules and the user rules keep their original order. The floor: `!.env` / `!.env.production` cannot un-ignore a credential; a user `*.sh`/`*.yaml`/`*.json` cannot beat `!.trinity/setup.sh` (the R2 regression); the floor is a total partition of the canonical list; every entry of `_TRINITY_AUTHORED_PATHS` is protected by derivation, so a tenth authored path inherits it; `!.env.example` follows `.env.*` in index order. Mechanics: byte-identical second run with a clean porcelain (the mtime is deliberately NOT asserted — content-identity is the invariant); a CRLF canonical copy is stripped while the user's own CRLF line keeps its endings, asserted at byte level because `str.splitlines()` splits on a bare CR and would hide the distinction; an unreadable `.gitignore` aborts without writing; a NUL-bearing file keeps its user region; no empty or newline-bearing managed line; no line-gluing on a file with no trailing newline. Reporting: removed/unignored/shadowed each pinned, including the `git check-ignore -v` exit-0-on-negation trap and the interleaved-stderr blob (`container_exec_run` does not demux). AC-1 is proved as the universal it is — `before - after == set(removed)` over a 10-file repo spanning both regions, a dir-form parent, effective negations and genuine #462/#1596 targets — and parametrised over both pattern forms, so it cannot pass on a file-form fixture and certify a guarantee that is false for the 23 dir-form patterns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: architecture delta and the agent-guide escape hatch + recovery (#2529) architecture/agent-lifecycle.md — the Git Sync Health paragraph (the Architecture Map's owner of git_service.py) gains the precedence sentence in the map's own "read it first because" voice: a canonical rule appended below an agent's negation silently reverses it and the sweep then untracks the file, and a canonical rule hoisted above a platform negation silently reverses that one instead — which is why there are two managed regions and not one. Plus the dir-form residual, the rejected contents-form alternative, and the five report surfaces. The core architecture.md is untouched: no new area, invariant or topology change, and no schema change. TRINITY_COMPATIBLE_AGENT_GUIDE.md — AC-7. Section 5 documented no escape hatch and no recovery at all. It now shows the three-part file, states that a negation no longer has to be the last line, names what the protected floor refuses and why, states the honest git limitation that a negation cannot re-include a file under an excluded directory (inert at any position — and that the sync now says so via shadowed_negations), and gives the one-line recovery for an already-stripped file: a negation below the managed block plus `git add -f`. The canonical fence itself is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(feature-flows): the two-region merge and the sweep report (#2529) github-sync.md — "Per-Push Gitignore Migration" rewritten for the rebuild shape, why two regions rather than one hoisted block, that AC-1 is enforced by git itself once the block sits above the agent's rules, the three report fields with their honest caveats (unignored is `after - before` against a live container; the commit message is best-effort because `git rm --cached` only stages), and the dir-form residual with its rejected contents-form alternative. The sequence diagram now shows the rebuild, the probes, the operator-queue entry and the three fields on the response. git-sync-health.md — §0 gains the region shape and the content-idempotence contract (the 14 bundled templates are the merge's fixed point, which is what keeps #1908's byte-identity that #953 depends on); the Testing section gains the five real-git suites it omitted, including the two #2069 files. feature-flows.md — Recent Updates row (standing rule: always add one). .claude/agents/test-runner.md — catalog entry for the 31 new unit tests, the four amended files, and the frontend/MCP specs; statistics bumped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(git-sync): route the untracked alert through the #1677 budget seam (#2529) `test_1677_operator_alert_emitters.py` caught this and it was right. The sibling `git_bloat` / `sync_failing` emitters are direct `db.create_operator_queue_item` calls because their cadence is the 60-second platform poller's. This one fires from `sync_to_github`, and `git_sync` is an MCP tool an agent-scoped key may call on itself — a repeated `git add -f <ignored>` + sync loop yields a fresh `removed` set every time against a timestamped, non-idempotent id, so nothing upstream bounds the volume. That is the `_alert_skill_not_found` (#1410) shape the budget seam exists for. So: `create_bounded_alert` with `gitignore_untracked` registered in `_BUDGETED_ALERT_TYPES`, and `gitignore-untracked-` added to `_RESERVED_ID_PREFIXES` so an agent cannot pre-create the id and silently suppress its own alert through the sink's `on_conflict_do_nothing` (the C2 class). The emitter is now async and still swallows everything — `create_bounded_alert` never raises and returns False when refused, but an alerting failure must not be able to fail a Push either. Two tests pin the classification and the payload (path cap at 20, empty sweep files nothing, a raising sink never reaches the caller). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(feature-flows): operating-room carries the second budgeted emitter (#2529) The #1677 emitter inventory in operating-room.md said "today `task_execution_service._alert_skill_not_found`" — it is no longer the only one. `gitignore_untracked` joins it as agent-influenceable, and the entry states WHY its `git_bloat`/`sync_failing` siblings stay direct creates: their cadence is the 60-second poller's, which is exactly the influence-not-location distinction #1677 was built on. The reserved-prefix list in the ingestion-caps section was also three prefixes stale (`db-backup-`, `log-archive-`, `sub-headroom-`); brought to match the source alongside the new one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(git-sync): pin the interaction with the other .gitignore writers (#2529) Decision 8 of the plan: the coverage frame is the FILE, not the three merge call sites. Two writers were reasoned about and are now pinned rather than asserted. `materialize_data_paths` (#1169) appends through the generic `_build_gitignore_append_command`, i.e. to the END — which after this change is BELOW the protected floor. Harmless (positive ignores over agent-declared data directories shadow nothing the floor protects) and self-correcting: they are not managed lines, so the next merge carries them into the user region with everything else. What actually matters is that neither writer duplicates them in either order, because a growing `.gitignore` is exactly what the 15-minute auto-sync loop would re-commit forever. And the honest cost of the rebuild: a user who typed a canonical pattern themselves keeps one copy — ours, in the managed block — so `*.log` / `!important.log` / `*.log` leaves only the negation below the block, WEAKENING an ignore they meant. It can never untrack something new, the floor covers the cases where weakening would be dangerous, and `shadowed_negations` reports the rest. Documented in the agent guide; now also pinned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(git-sync): the field incident's own shape, pinned (#2529) Revised criterion 3 as it actually happened: an earlier Push swept four `.env.example` files — still on disk, gone from the index, and IGNORED, so nothing re-added them. The issue's follow-up names the second-order property, "un-ignoring a file does not re-track it": three of four were restored by hand and one sat untracked for two days. The test starts from exactly that state and asserts the next Push reports the file in `unignored_paths` — the report that would have made the sweep a same-day finding instead of a two-month-old one — and that the same Push's `git add -A` re-tracks it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(git-sync): report removed paths only after the rm actually ran (#2529) `$ignored` is captured before `git rm --cached`, so echoing it before the rm named files that are still tracked whenever the rm failed — a false alarm on the operator-queue entry, the one surface whose whole job is to be trusted. Moved below the rm, so a failed sweep aborts the `&&` chain and reports nothing, which is what this code did before #2529 anyway. Pinned by a structural test that also holds the two post-sweep probes below it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(git-sync): bound the sweep probes, and keep the removed count exact (#2529) `container_exec_run` reads a whole exec output into memory as one blob, and both probed sets are unbounded in exactly the cases this code exists for. The untracked probe runs BEFORE the canonical block is written on a pre-canonical agent, so nothing is ignored yet and `$HOME` answers with every file under `.local/lib/python3.13/site-packages`, `.npm` and `.cache`. And `$ignored` is five figures on the #1596 population — an agent with a committed `node_modules/`, the 44 GB repos those patterns were written for. Both are the migration moment, fleet-wide, which is the worst time to ship an unbounded read. So every probe list is `head`-capped in-container at 2000 lines, with two properties that keep the report honest rather than merely small: - The cap is `head -n CAP+1` on the two set-difference operands, so truncation announces itself; when either side is truncated `unignored_paths` is SUPPRESSED rather than computed, because a difference over a truncated operand invents entries that are only "new" because the other side was cut off. The field is advisory, and a fiction is worse than a gap. - The removed count is emitted separately and exactly (`wc -l` over the full list) into `removed_total`, so a capped list never turns into an undercounted "untracked N file(s)" — the number is what an operator acts on. It falls back to the list length if the count line is missing or unparseable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(git-sync): pin probe-tag prefix-distinctness + delivery record (#2529) `_tagged_output_lines` matches by `startswith`, so a tag that is a prefix of another silently absorbs its lines — and `removed` vs `removed-count` is one character away from being exactly that. Also records the delivery in `.plan/issue-2529-delivery.md` (gitignored): AC-by-AC coverage for the original seven plus the four revised criteria, the two structural decisions with the measurements behind them, the three things found during implementation that the plan did not anticipate, and every verification command with its actual result. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(git-sync): `.ssh/` belongs in the protected floor (#2529) Review found the mirror of this issue's own bug. Hoisting the defaults block above the user region is only safe for a default the agent may genuinely override, and `.ssh/` is not one — `credential_paths.py` classifies `.ssh/id_*` as secret material. It sat in the OVERRIDABLE region because floor membership was drawn around the constant's "Credentials — NEVER COMMIT" comment heading, and `.ssh/` lives under "Instance-specific directories". Reproduced on the live fleet shape, not argued: an agent whose `.gitignore` carries `!.ssh` with no `.ssh/` line of its own (every pre-#2529 Push appended the canonical block BELOW that negation, so `.ssh/id_rsa` was ignored) comes out of the rebuild with `.ssh/id_rsa` NOT ignored, and the unattended 15-minute `git add -A` then commits a private key to the owner's GitHub repo. That is Decision 1's own `!.env` hazard, one directory over. Floor membership is now decided against `credential_paths.py` and pinned both ways: a behavioural regression test that reproduces the live shape end-to-end (RED without this commit) and a membership assertion, so adding a credential-bearing pattern to the overridable region has to be a decision someone makes on purpose. The 14 bundled templates are regenerated — they are the merge's own fixed point, so `.ssh/` moves with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(git-sync): report the sweep in BOTH directions, not just removals (#2529) Every reporting surface gated on `sweep.removed`, so the addition half of the rebuild was exactly as silent as the deletions this issue exists to end — and the addition is the worse half. A removal is recoverable from the working tree; a newly un-ignored path is already in the remote's history and may need a credential rotated rather than a file restored. The `.ssh/` case in the previous commit produced `unignored_paths` correctly and reported it through none of the five. `GitignoreSweep.changed_tracking` (`removed OR unignored`) is now the single gate for the operator-queue entry, the commit-message body, the `message` summary line and the toast. `shadowed` is deliberately NOT in it — standing advice about the file, not a change this Push made, so including it would file an alert on every Push of every agent that has a dir-form negation. An unignored-ONLY entry files at `medium`, not `high`: `unignored` is `after − before` across two execs against a live container, so a file the agent's own session creates in that window lands there too, whereas a removal is a confirmed destructive act. A band that cries wolf stops being read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(learnings): inverting a precedence order re-decides every conflict at once (#2529) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(agent-guide): appending the negation is enough — the merge normalizes it (#2529) The recovery snippet told the reader to hand-move the appended line above the floor markers. Unnecessary: the negation is not a managed line, so the next merge carries it into the user region automatically, where it already beats the defaults block. The instruction only invited manual editing inside a region the merge rewrites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(git-sync): pin the HTTP surface — the response dict is hand-built (#2529) `_with_sweep` populating `GitSyncResult` is only half of AC-4's API half: `routers/git.py::sync_to_github` hand-builds its response dict and drops anything not named in it, so a sweep field can be correct on the model and invisible to every HTTP and MCP caller (the MCP tool is a passthrough over this dict). The code comment names the hazard; nothing asserted it. Two tests, both proven RED by mutating the router: - the success dict must carry every field `_with_sweep` writes, derived by BEHAVIOUR (baseline vs populated result) rather than a hardcoded list — a hardcoded list is the same "someone must remember" mechanism that loses the field. A fourth sweep field forgotten in the dict now fails here. - a 409 raises `HTTPException`, which keeps only `detail`, so the folded summary is the whole structured surface — pinned alongside `removed_paths` on the #905 audit row, the durable half. The sweep mutates the index before the push, so a failed Push has untracked the files just as surely as a 200. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TvPBC72QFs1kXSA7YUyiS1 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: sim <sim@example.com>
vybe
pushed a commit
that referenced
this pull request
Sep 8, 2026
…preview and delete (#2582, Abilityai/trinity-enterprise#548) (#2608) * docs(workspace): requirements + architecture + flows for the Files tab (#2582, Abilityai/trinity-enterprise#548) Trinity Rule #1 — the docs land before the code. - requirements/core-agent.md: new §5.30 (the six ACs, the permission matrix, the storage decision, the stated limits); §5.20 AC-2/AC-4 amended — the Files signal now covers uploads and the refresh triggers widen. - requirements/content-files.md: §13.10's flat "Content-Disposition: attachment" bullet was stale against shipped ent#461; rewritten as the server-decided allowlist plus the one-way ?download=1. - architecture/integrations.md: the ent#461 delivery-policy paragraph records the one-way flag, why it is applied in the handlers, why it is parsed tolerantly, and that sig is a stored bearer token rather than an HMAC over the URL — so appending the flag cannot invalidate it. - architecture/api-endpoints.md: GET row updated, the missing HEAD row added, and the three new client-portal routes catalogued. - architecture/workspace.md + database.md: the upload announcement, the session-type-dependent matrix, and portal_file_dismissals with the three reasons its shape is what it is. - feature-flows: workspace-rail.md gains Slice 3; file-sharing-outbound.md corrected on three counts that were stale since ent#461 and #568; workspace-agents-at-the-centre.md and the index updated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq * feat(workspace): portal_file_dismissals on both migration tracks (#2582, Abilityai/trinity-enterprise#548) A Workspace viewer needs to remove an agent-shared file from THEIR list without revoking the share. agent_shared_files has no audience column, so portal_documents lists every active share of an agent to every rostered client; and the one generic per-user preference store is FK'd to users.id, which a portal principal has no row in. So: new storage. - tables.py / schema.py (DDL + the sweeper's file_id index) / migrations.py (SQLite) / migrations/versions/0058 (PostgreSQL) — Invariant #9, both tracks, single head confirmed by scripts/ci/check_alembic_heads.py. - agent_name is on the table FOR the AgentRef registration: agent_shared_files is a CASCADE ref, so deleting an agent hard-deletes its shares without going through the revoke sweeper — every dismissal keyed on those ids would be orphaned forever, and a table with no agent column would sidestep the parity guard that exists to catch exactly this. - Both purge paths in db/agent_shared_files.py delete the matching dismissals in the same transaction. - PK leads with client_email (the read is WHERE client_email = ?, once per participant per turn end); the file_id index serves the sweeper. test_agent_cleanup_parity + test_schema_parity: 8 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq * feat(files): the one-way ?download=1 flag, and the Workspace file verbs (#2582, Abilityai/trinity-enterprise#548) routers/files.py - GET and HEAD accept `download`, which may only ever force `attachment`. There is no ?disposition= and no way to force `inline` — that direction is ent#461's XSS allowlist. `_format_disposition`'s docstring now records the asymmetry, so the next reader does not resolve the apparent contradiction in the wrong direction. - Typed Optional[str] with a truthy check, NOT bool: a bool query param 422s on ?download= or ?download=x, and this is the public link opened from Telegram / WhatsApp / iOS — a malformed query it ignores today must keep being ignored. - Applied in the handlers, never in _validate_download_request, whose arg list is AST-pinned by test_file_download_no_session_gate. - A ranged PREFIX read no longer bumps download_count. The Workspace preview reads from byte 0, so without this every preview would inflate the owner's numbers and bury the audit log. The audit row is kept and made separable instead: details.ranged_prefix. - `# mcp: none` header (Invariant #13). client_portal - portal_owns_agent + `owned` on the roster row and the card: the SAME membership the card renders, so the UI's "Delete for everyone" and the service's gate cannot disagree. - Three routes: read one of your own uploads back (attachment, nosniff, no-store), delete one (idempotent), and remove/revoke an agent share. Each is _require_roster -> rate_limiter -> service -> audit. - TWO limiter tiers, env-tunable, per router.py's own stated rule — and the burst tier is 20, tighter than upload's, because this is the first time a rostered client can reach extract_from_agent (sync docker-py iteration on the global 4-worker pool, ~3x file size resident). Delete gets its own looser counter; an rm is not a get_archive. - portal_revoke_shared_file is access-first (roster 404 -> owner 403 -> row 404), never existence-then-access (Invariant #8), and 404s where its operator sibling 204s — enumeration-uniformity on an external surface, stated in the docstring so nobody aligns it. - portal_dismiss_shared_file does NOT validate the file_id (an existence oracle over every share in the install) and caps rows instead — the same fork set_chat_star already resolved. - _inbox_path_for: `_safe_filename(name) == name` or a uniform 404. Every shell use is shlex.quote plus `--`, because _safe_filename admits a leading hyphen. - _read_inbox guesses mime_type, and PortalUploadItem declares it — the row's FileIcon has been rendering the generic icon since it shipped because the response model stripped the undeclared field. - portal_documents drops dismissed ids and appends &download=1. Only this base URL gets the flag; the agent's chat link is untouched. test_ent79_portal_exposure: the two download_url assertions updated (the AC changes the URL) and the fixtures gained the table portal_documents now reads. 71 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq * feat(workspace): the Files tab — uploads at once, save, preview, delete (#2582, Abilityai/trinity-enterprise#548) stores/clientPortal.js - uploadDocument queues the agent in a pending-agent SET. It is the ONE funnel all three upload surfaces already call, so notifying here makes "Files you sent" update before any agent reply WITHOUT touching PortalConversation.vue or PortalRoom.vue — the files the delivery sequence is serialized to protect. - A set, not a scalar, because both real gestures defeat a scalar: a sequential multi-file batch (a consumer joining the in-flight read gets a listing taken before the later files landed) and a room's fan-out across three DIFFERENT agents in one Vue flush window (only the last value survives). - fetchUploadBlob / deleteUpload / deleteDocument. stores/portalRailFeeds.js - noteUpload(agent): shares _fetchToken with refresh() (a refresh issued before the upload but resolving after it would otherwise silently clobber the fresh listing), coalesces leading AND trailing, and re-checks participation after the await. upload() now delegates to it rather than carrying a second read. portalRail.js / usePortalRailFeeds.js - filesSignalItems(documents, uploads) projects uploads onto the created_at key the Files dot already reads, and BOTH the signal and markSeen read it — one mechanism, or opening the tab would mark documents seen and leave the uploads' dot lit forever. The collections stay separate; only the signal merges them. - The pending set is drained by the owner: clear-then-read, so a note arriving mid-drain is a new entry rather than one this drain already claimed. portalFiles.js (new, pure — vitest pins environment: node) - flattenFiles owns BOTH render order and preview index; two orderings drift and the lightbox opens the wrong file, silently. `groups` is a parameter so ent#484's shared folder becomes a third entry with no structural change. - previewKind is extension-first for text: Python's mimetypes maps .ts to video/mp2t and .toml to nothing, and a shared .md arrives as text/plain. - neighbour skips non-previewable rows and stops at the ends. - errorDetail reads a Blob body first: with responseType 'blob' the usual err.response.data.detail idiom yields undefined, so the promised "server's named reason" would degrade to a generic line for exactly the new verbs. PortalFilePreview.vue (new) - Images ONLY through <img :src>, never inline <svg>, never v-html. - Text capped at 256 KB, fetched whole and sliced: CORS allow_headers omits Range, so a ranged preview dies silently cross-origin — and slicing keeps preview off the transfer-start counter path. The cap is stated in the UI. - Escape/arrows registered with { capture: true } + preventDefault, because the conversation's turn-cancel listener is on document in the BUBBLE phase. - v-if not v-show (the column and the mobile sheet are siblings), z-40 below ConfirmDialog's z-50, focus on the safe action, a Tab trap. - Never a blank modal: an unpreviewable type, an over-cap image AND a failed byte fetch all land on the same name/size/type + Download card. PortalRailFiles.vue - One v-for over the flat list; Download on both lists; the delete matrix mirrored off the roster card's `owned`; one ConfirmDialog with copy that restates the consequence per case; per-row InlineError with the server's reason. The dead single-file upload() (zero callers) is gone. - AC-3's `download` attribute is deliberately dropped: the control is a button driving a blob save and cannot carry it, and it is inert on a cross-origin anchor anyway — which is why the server-side flag exists. npm run test:unit: 101 files, 2238 passed. check:tokens OK. Both new files scan 0 raw_nongray / 0 hardcoded; loading gates total 70, equal to baseline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq * test(files): the one-way flag, the upload verbs, and the delete matrix (#2582, Abilityai/trinity-enterprise#548) Three new files, registered in tests/registry.json, plus the ent#461 guard extended so the asymmetry is asserted where a future widening would be edited past. test_2582_download_flag.py (35) - Proves the ASYMMETRY, not the feature: the flag forces attachment and NO input forces inline (?download=0 on text/html stays attachment). - ?download= / ?download=x / a repeated pair never 422 — the regression a bool annotation would have shipped on the public link ent#461 exists to keep opening from a phone. - HEAD agrees with GET; the sig survives the extra query pair; every other ent#461 header is carried through; the flag rides the 206 branch. - A ranged prefix read is audited ranged_prefix:true and does NOT bump download_count; a full-file range and a plain GET do; a mid-file seek does neither. Mutation control: forcing is_ranged_prefix=False fails that test. test_2582_portal_uploads.py (28) - The traversal 404 asserted at the handler AND through the mounted route, with a positive control — %2F shapes never match the route while %2E%2E reaches the handler, and a status-only test cannot tell the two apart. - Gate order (off-roster before any docker work), ent#308's collision on the read side, the translated extract_from_agent exceptions (its 404 echoes the container path), the deliberate 409/502 on a stopped/missing agent, rm -f -- with quoting proven by a name that needs it, and both limiter tiers. test_ent548_portal_share_delete.py (23) - The matrix, including the two cases that read as bugs without the rule: a non-owner admin is a viewer, and so is an owner on a portal token. - Access-first revoke proven with a call-recording monkeypatch (the row must not be read before the caller is authorized). Mutation control: reordering to existence-then-access fails it. - The dismissal's non-validation and its row cap as a PAIR, both purge paths, the AgentRef registration, both migration tracks, the PK's leading column. Mutation control: dropping the delete_for_agent purge fails it. 218 passed across the plan's verification set; alembic heads still 1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq * test(workspace): the upload signal, the flat projection, and the preview rules (#2582, Abilityai/trinity-enterprise#548) portalRailFiles.spec.js (19) — and it found a real defect. The room fan-out test failed against my own first cut: `noteUpload` shared `_fetchToken` with `refresh()`, so three concurrent per-agent reads each invalidated the last and two of three listings were discarded. The two questions are different — "has the chat moved on?" is global, "is this agent's listing still the newest?" is per agent — so the store now carries a `_scopeToken` (participant changes and clear only) and a per-agent `_uploadEpoch` that `refresh()` snapshots before its awaits. That snapshot is also what stops a refresh issued before an upload and resolving after it from silently clobbering the fresh listing. Three mutation controls, each failing exactly its own test: - a shared counter instead of the per-agent epoch -> the fan-out test and the clobber test fail; - dropping the trailing re-fire -> the two-file batch test fails; - refresh ignoring the epoch snapshot -> the clobber test fails. The spec mounts the composable inside an effectScope stopped after each test: its watcher on the SHARED portal mock has no component to unmount it, so the oldest surviving watcher drained the queue against a previous test's Pinia store. Diagnosed from the symptom (uploads.scout undefined, a fan-out noting one agent), not guessed. Also pinned: the dot lights with Files CLOSED after one targeted read; opening the tab clears it (markSeen must read the same projection or the upload's dot stays lit forever); a non-participant bump does nothing; a hidden rail drops the note rather than queueing it; and source guards that PortalConversation.vue and PortalRoom.vue are untouched and know nothing of the rail. portalFiles.spec.js (47) — previewKind's extension-first rule with the three cases that motivate it (.md arriving as text/plain, .ts as video/mp2t, .toml as nothing), neighbour skipping and stopping, flattenFiles order equalling render order, the fileActions matrix failing closed, sameOriginPath on a RELATIVE url (the default install), the Blob error path, and the component's source guards: an <img> and no v-html, no Range, capture + preventDefault, revokeObjectURL, z-40 below ConfirmDialog, v-if not v-show, never a blank modal including on a failed fetch, zero raw palette classes and zero hex. portalRailFeeds.spec.js: the portal mock is now reactive() with the new fields, so the owner's watcher is not permanently inert there. 103 files, 2304 passed. check:tokens OK; loading gates 70 (= baseline); both new files 0 raw_nongray / 0 hardcoded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq * docs(workspace): correct the upload-ordering mechanism, and record the third Escape owner (#2582, Abilityai/trinity-enterprise#548) Two corrections and one addition from the tail steps. The docs written before implementation said the drain "shares the feed store's _fetchToken with refresh()". That is no longer true and was never sufficient: the room fan-out test proved a shared counter makes three concurrent per-agent reads invalidate each other. The mechanism is a _scopeToken (chat identity) plus a per-agent _uploadEpoch that refresh() snapshots before its awaits. Corrected in requirements/core-agent.md §5.30, architecture/workspace.md, feature-flows/workspace-rail.md (Slice 3, with the finding recorded as the reason) and the feature-flows.md changelog row. /sync-feature-flows also surfaced one flow the docs commit missed: chat-turn-cancellation.md owns the "what may take Escape" rule, and this PR adds a THIRD way of owning it — a rail-mounted overlay with no ref the conversation could name, taking Escape in the capture phase with preventDefault() rather than joining the per-surface overlay list. The known residual (the voice-call branch above shouldCancelOnEscape never consults defaultPrevented) is recorded there too, with a revision-history row, and the index row now points at it. The Slice 3 Testing block names the effectScope the spec needs and the three mutation controls. NOTE: /update-tests also updated .claude/agents/test-runner.md, which lives in the private .claude submodule (detached HEAD in this worktree) — that edit is left UNCOMMITTED there rather than creating a dangling commit and dirtying the gitlink. It needs landing in trinity-dev separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq * fix(workspace): Download a share through the server flag, not a 50 MB blob (#2582) Self-review finding. The first cut fetched every row as a blob and handed it back through a synthetic <a download> — which made the one-way ?download=1 flag decorative on the one surface it was added for, pulled up to 50 MB into the tab to save a file the browser could stream itself, and used exactly the programmatic-blob-save path the plan's own risk register flags as the classic iOS Safari failure, on a surface whose primary form is a phone sheet. An agent share now saves by an anchor click on its already-attachment URL. A client upload keeps the blob path, because it has no URL at all — no DB row, no token — which is the whole reason that path exists. AC-3's `download` attribute rides on that anchor as belt-and-braces; a browser ignores it cross-origin, which is precisely why the server-side flag is the mechanism rather than the attribute. Pinned by a source guard with a mutation control (collapsing the branch fails it). 103 files, 2305 passed. Docs corrected in §5.30 and workspace-rail.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq * fix(workspace): the delete confirm owns Escape too, or it cancels the turn (#2582, Abilityai/trinity-enterprise#548) Review finding. The preview modal took Escape in the capture phase with preventDefault() so it could not reach the conversation's bubble-phase turn-cancel listener — but the ConfirmDialog it raises has no key handling of its own, and nothing was added for it. So Escape on an open delete confirm dismissed nothing and arrived at shouldCancelOnEscape with defaultPrevented still false: the dialog stayed up and an in-flight turn was destroyed. Same shape, same reason. And because two capture listeners on `document` fire in registration order — the tab body mounts before the modal it opens — the preview now returns early on event.defaultPrevented, so one keystroke closes one overlay rather than both. Also documents the three PORTAL_FILE_* limiters in .env.example, beside the PORTAL_CHAT_*/PORTAL_UPLOAD_* pair they were modelled on. They are the only bound on the newly-exposed extract_from_agent path, and an operator cannot tune a knob they cannot discover. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq * perf(workspace): one inbox read per upload, not two (#2582, Abilityai/trinity-enterprise#548) Review finding. `portalRailFeeds.upload()` awaited `noteUpload()` itself while `clientPortal.uploadDocument()` — which it calls one line earlier — already queued the same agent for the rail owner's drain. The drain's note therefore arrived while the store's own read was in flight, became its trailing re-fire, and every drop-zone upload cost two container execs: eight for a four-file batch, on the shared 4-worker executor. The funnel is the mechanism, so let it be the only one. `upload()` now just sends. The two specs that pinned "delegates to noteUpload / re-reads its own agent" pinned the redundancy, so they now pin ZERO reads from `upload()` and move the "skips a chat that moved on" property onto `noteUpload`, where it actually lives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq * docs(workspace): name the four filed follow-ups (#2582, Abilityai/trinity-enterprise#548) The plan listed four things to file and the docs said "filed as a follow-up" while nothing existed on either tracker. Filed, and named where a reader lands: trinity#2598 Escape during a voice call ends the call even when an overlay already claimed the keystroke — PortalConversation.vue's voice branch sits above shouldCancelOnEscape and reads no defaultPrevented, so preventDefault() cannot reach it trinity#2599 the Workspace Files routes are uncatalogued in api-endpoints.md ent#549 shared files have no audience — every rostered client sees every share of an agent, ?sig= included; a dismissal is a preference, not authorization ent#550 a client upload has no DB row, which is why listing, download, delete, preview and MIME are five different mechanisms Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq * docs(memory): the Escape-ownership class, from the #2582 review An overlay that claims Escape must claim it for the dialogs it raises too — the 4.14 incomplete-fix class in UI clothes, and destructive-and-silent here because the wrong Escape kills billed work with no sign. Plus the two rules that fall out: capture listeners on `document` fire in registration order, not z-order; and a `preventDefault()` protocol is only as good as the consumer branch that reads it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq * test(files): prove the one-way flag where it is WIRED, not only where it is computed (#2582) `_apply_download_flag` is a pure function, and the existing test pins its asymmetry exactly. But a pure function cannot tell you the route handed it the right `inline=`. The plausible regression — a handler deriving the disposition from the query parameter instead of from `is_inline_safe(row["mime_type"])` — passes every helper-level assertion in this file and ships stored XSS on a public token-gated link. So the route fixture is now parametrized by type, and a `text/html` row is asserted `attachment` across every flag value, on GET and HEAD both. Verified by mutation: deriving the disposition from the parameter turns 6 tests red, 5 of them these. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(workspace): forward the portal file limits from every compose (#2582, Abilityai/trinity-enterprise#548) The three knobs were read by `client_portal/router.py` and documented in `.env.example` while being forwarded by NONE of the three compose files, and neither prod nor hosted uses `env_file:` — so the advertised `.env` lever was inert on every install. That is worse than an undocumented knob: the operator sets it, sees no effect, and has nothing to debug. The #1056 / trinity-enterprise#31 packaging-gap class, caught at the `/validate-pr` gate. Proven by render rather than grep — `docker compose config` showed NONE before and honours `PORTAL_FILE_HOURLY_LIMIT=7` over the 100 default after, on both dev and prod. Wiring prod also broke #2280's wholesale env parity against `docker-compose.hosted.yml`, which is the guard doing its job: the gap was three files wide, not two. Guarded so it cannot recur: the knobs must be forwarded by all three composes, with defaults agreeing across them AND with the module default. #2433's guard is scoped to its own two vars by a hardcoded tuple, so extending it would have been the wrong home. Mutation-verified — dropping one var from prod turns 2 tests red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(workspace): exclude file previews from download counts --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: sim <sim@example.com>
vybe
pushed a commit
that referenced
this pull request
Sep 9, 2026
…o fills which role (#2596) * feat(assignments): the OSS seam, the execution-context fields, and the rename sweep The edition-agnostic half of role assignments (trinity-enterprise#500). Inert on its own: no provider is registered in a core build, so `resolve_assignment` returns None, the three new `ExecutionContext` fields stay None, and the prompt block renders byte-identically to today. * `services/assignment_provider.py` — the seam, in the `mfa_gate` / `a2a_gate` shape (Protocol + register/get/clear + a resolve function). The SEAM owns the failure handling, not the provider: `compose_system_prompt` has no exception handler and all three of its callers lose the execution-context block if this raises (one loses the platform prompt entirely). A malformed non-raising answer is validated separately, because try/except cannot see a `str` where a list was promised — it would iterate into single characters and render the wrong prompt without raising. * `platform_prompt_service.py` — `primary_user_display` / `role_id` / `stakeholders` / `proactive_consent`, auto-filled as ONE provider call beside collaborators and platform_url. `needs_assignment` joins the `replace` guard, not just the value: a caller that pre-filled both existing fields would otherwise skip the block and the new fields would silently never render. The primary line requires a display NAME — never an email, because this block reaches anonymous public-link and paid turns. * `db/agent_cleanup.py` — `cascade_rename` now sweeps `EXTRA_AGENT_REFS`, which its own docstring already promised it did. A registered private table kept the OLD agent name across a rename, so the agent lost its rows and a later agent taking the freed name inherited them — the recycled-name cross-wire `delete_reports_to_refs` exists to prevent, one function below. * `operator_queue_service.py` — `ROLE_DRIFT_ALERT_PREFIX`, reserved so an agent cannot pre-create and ON CONFLICT-silence the alert about its own role file, which lives in its own writable workspace. * `enterprise-docs-guard.yml` — the new seam file joins `SEAM_FILES` and both path filters, so a leak in its docstring fails the build instead of shipping green (#1461's lesson). Refs trinity-enterprise#500 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(mcp): get_agent_assignments — an agent can read who it works for The third surface for role assignments (Invariant #13, trinity-enterprise#500): tool module + client method + server registration, plus the tests. Read-only, and that is structural rather than a v1 scope cut: creating an assignment is a GRANT, and the backend refuses every non-interactive principal on the write path, so an MCP write tool would be one that can only ever fail. Narrower than credential_vault.ts on purpose. That module branches on the detail SHAPE because its backend raises coded refusals; this route raises none — its failures are the entitlement 403 (plain-string detail) and a UNIFORM 404 that covers "route absent" and "no such agent / no access" alike. The 404 is uniform for enumeration safety, so the tool merges those cases in its message instead of claiming a distinction it does not have. Registered in the operatorOnly group, whose allow-list includes `agent` — which is exactly why the backend self-scopes an agent principal to its own roster. Advertisement is not authorization, and an agent-scoped key resolves to its owner carrying the owner's role. Refs trinity-enterprise#500 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(assignments): pin the seam's three degrade paths and the rename cross-wire Four public files, and each one exists for a failure that ships GREEN without it. * `test_ent500_assignment_provider.py` — no provider / a raising provider / a malformed non-raising answer all degrade to None. The third is the one try/except cannot give you: a `str` where a list was promised iterates into single characters and renders the WRONG prompt without raising, which is strictly worse than a missing line because nothing looks broken. Also asserts the seam file is in enterprise-docs-guard's hardcoded SEAM_FILES and in BOTH path filters — an omission there is invisible forever (#1461). * `test_ent500_execution_context_fields.py` — rendering, bounds, and the two wiring properties that would otherwise fail silently: the provider is called exactly ONCE per compose, and `needs_assignment` is part of the `replace` guard, so a caller that pre-filled collaborators AND platform_url still gets the assignment lines. Plus: a raising provider still yields the full block, platform prompt included. * `test_ent500_public_turn_no_pii.py` — an outside turn renders no assignment line, an inside turn still does, an unrecognised label is suppressed by default, and the answer contract has no email-shaped key at all. Pins the fact the suppression rests on: a Workspace/portal turn is labelled `public`, so there is no third label to remember — asserted over the source, so a new outside surface under its own label fails here instead of disclosing. * `test_ent500_cascade_rename.py` — verified to FAIL on the pre-fix `cascade_rename` (3 of 5 red), using a table deliberately absent from the OSS MetaData, because a test against an OSS table passes on the broken code. Refs trinity-enterprise#500 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(assignments): the seam, the context fields, and the cascade fix Rule #1's requirements-first pass, plus the architecture and flow updates. The public tree describes the generic open-core seam only — the record itself, its schema and the module that owns it stay in the private repo. * `requirements/infrastructure.md` §12.6.2 — the four execution-context fields live beside §12.6.1, which is where execution-context injection is actually documented (the plan named core-agent.md; that is not its home). * `architecture/backend.md` — the seam entry beside the other open-core seams, and a paragraph on the agent-cascade registry stating why BOTH lists are swept by BOTH paths, with the recycled-name inheritance that made it matter. * `architecture/mcp-server.md` — the tool count RECOUNTED (129 across 33), not incremented: it read 121/30 and the table was already missing `canvas.ts` and `credential_vault.ts`, so those rows land too and the count is true again. * `feature-flows/role-assignments.md` — the new flow. * `feature-flows/execution-context-injection.md` — the two new lines in the block, the auto-resolved list, the `replace`-guard contract, the per-field bounds and the audience gate, and the tests. * `feature-flows.md` — Recent Updates row + a Collaboration & Permissions row. Deliberately NOT added: a route row under api-endpoints.md's Enterprise Modules section. That section ends with the standing rule that the module catalog is not documented publicly, and the credential-vault routes are absent for the same reason. The seam and the OSS tool are public; the module is not. enterprise-docs-guard re-run locally over docs/ + CLAUDE.md + all six seam files: clean. Refs trinity-enterprise#500 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(cleanup): consolidate the EXTRA_AGENT_REFS rename sweep, and correct the claim Two things, and the second matters more than the first. **The correction.** The previous commit here said `cascade_rename`'s missing `EXTRA_AGENT_REFS` sweep meant a registered private table kept the OLD agent name after a rename, so the renamed agent lost its rows and the next agent to take that name inherited them. That was overstated. Reading every caller — `db/agent_settings/metadata.py::rename_agent` is the only production one — shows it carried its OWN `EXTRA_AGENT_REFS` loop (ent#46) immediately after the `cascade_rename` call. The rename endpoint was correct the whole time. Corrected in the docstring, `architecture/backend.md`, `role-assignments.md`, `agent-rename.md` and the Recent Updates row. **What was actually wrong, and the fix.** The behaviour lived in the CALLER while `cascade_rename` — the shared function whose contract advertised it — did not do it. That is exactly what made it survivable and exactly what made it worth fixing: a cross-repo module author reads `register_agent_owned_table`'s docstring, not one caller's body, and a second caller of `cascade_rename` would have silently dropped the sweep. The #1819 shape one level down — two places answering one question. So this is a CONSOLIDATION, not a bug fix: the sweep now lives in `cascade_rename` (previous commit) and the duplicate loop in `rename_agent` is deleted here. Leaving both would have been a third copy, and a redundant pass is how the two lists drifted apart in the first place. Tested at both levels, because moving a loop is otherwise an unverified refactor of the one path that mattered: `cascade_rename` re-keys a registered table; the production `rename_agent` path still re-keys after the move (verified RED with the shared sweep removed — 4 of 7 fail, including the caller-level test); and a structural guard forbids `rename_agent` regrowing a loop of its own. Found by the doc pass, not by a test — which is its own small argument for writing the flow doc rather than only the code. Refs trinity-enterprise#500 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(assignments): mirror the tightened audience rule on the public side The provider's allow-list dropped `mcp`, `agent` and `room` (each has a reader who may be outside the organisation) and gained `manual` / `retry` while losing the dead `task`. The public contract test's stub mirrors that rule, so it should say the same thing. Adds a case for the three transitive/ambiguous labels specifically, because the source scan beside it CANNOT catch them: no outside-facing router emits `mcp`, `agent` or `room`, so asserting that `public.py` / `paid.py` / `client_portal/service.py` use only the suppressed set says nothing about them. Two different gaps, two different checks. Refs trinity-enterprise#500 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(assignments): the seam validates types, not contents — the provider owns "never an email" The public flow doc claimed an address "cannot ride in even by a provider's mistake" because the answer contract has no email-shaped key. The provider found out otherwise: a display name resolved from an account whose only name-shaped column IS its address is a `str`, and it passes every check the seam makes. The seam validates types, not contents, so the property has to hold where the value is resolved. Both the doc and the seam docstring now say which side owns it. (The provider-side fix is in the private module.) Refs trinity-enterprise#500 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6P6bjT1RRiHosQCAEgJE7 * docs(learnings): two classes from the ent#500 review — SQL-reproduced predicates, and value-vs-field-name contracts Refs trinity-enterprise#500 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6P6bjT1RRiHosQCAEgJE7 * docs(assignments): record the Testing status on the role-assignments flow /validate-pr's feature-flow format check wants a status indicator on the Testing section — the one thing a reader cannot reconstruct from the file list. States what was actually run (71 assertions across three pytest-randomly seeds, 326 mcp-server tests, the 14456-test unit island) and, separately, that the cascade_rename suite was verified RED against the pre-fix code: "there is a test" and "the test would have caught it" are different claims and only the second is worth writing down. Refs Abilityai/trinity-enterprise#500 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6P6bjT1RRiHosQCAEgJE7 * fix(enterprise): drop the accidental submodule gitlink bump from this PR 9e77626 carried a `src/backend/enterprise` gitlink bump along with its code change. It is invisible in a normal `git diff` because this clone sets `ignore = all` on the submodule, which is exactly how it got past the working-tree check. It must not be here. The gitlink bump is its own PR, opened only after the private one merges, and it is the moment the feature actually goes live — not this PR, which is inert by construction. Worse, the pinned commit is local-only: it is on no remote branch, so any clone that did initialize the submodule would fail to resolve it. Restored to the commit `dev` pins. Net diff for the submodule is now empty; the working-tree submodule is left where it was. Refs Abilityai/trinity-enterprise#500 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6P6bjT1RRiHosQCAEgJE7 * fix(tests): resolve the cascade registry the way the production path does CI seed 99999 failed `test_the_production_rename_path_still_rekeys_a_registered_table` with `(1, 0) == (0, 1)` — the probe row never moved — while seeds 12345 and 67890 and every isolated run passed. The rename itself returned True, so the production path was fine; the test was looking at the wrong registry. `registry_isolated` resolved the module with `from db import agent_cleanup`, which reads the `db` package ATTRIBUTE. The production call site does `from db.agent_cleanup import cascade_rename` inside `rename_agent`, which goes through `sys.modules` at call time. Those two diverge in a suite that drops the module from `sys.modules`: the package attribute keeps pointing at the stale object, so the registration lands on a list nothing reads, the sweep finds an empty registry, and the row stays put. The test then reports the exact wording of the regression it was written to catch — an isolation failure wearing a real bug's face, which is the expensive kind. Resolved via `importlib.import_module` so both sides agree by construction. Also asserts the two pieces of process-global state the test does not own — that the probe table is in the registry the production path actually reads, and that it is visible on the engine the rename will use — BEFORE the rename. Both fail the same way at the final assert, so without this a future isolation break impersonates a rename regression again. Refs Abilityai/trinity-enterprise#500 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6P6bjT1RRiHosQCAEgJE7 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 9, 2026
feat(pull): async fan-out join + sync edge adapter — no autonomous trigger is stranded (#2524)
#2532
Open
dolho
added a commit
that referenced
this pull request
Sep 10, 2026
… on ownership (ent#553)
Three review findings, all in the same direction — the backend was right and
the user-facing half did not arrive — plus the two smaller ones.
1. **The stated bound now reaches the user.** `canvasLimit` was a `ref(0)`
nothing ever assigned, so `canvasHeadroom(n, 0)` returned `{label: null}` and
the early warning could not render at any count. The ceiling rides
`GET /api/settings/feature-flags` as `canvas_max_per_agent` — the established
home for a value the browser needs to render a surface, and where
`platform_default_model` / `install_source` already set the precedent for a
non-boolean. Not a new route (Invariant #13 would owe three surfaces for one
integer) and not an envelope around the canvas list (the MCP tool and the
Workspace both read it as a bare array). It is a CONSTANT, not per-agent
state, and the client already holds the count. `0` still means "not told" and
still renders nothing, so an older backend is unchanged.
2. **The Workspace canvas writes are audited.** The three portal routes recorded
nothing while their operator twins have logged since they shipped, and
`docs/user-docs/agents/agent-canvas.md` tells users deletion is audited — so
the claim was false for exactly the client-facing surface. `_audit_canvas_change`
is the shared helper; the actor is `actor_email` (the documented #848
inline-auth path) rather than a fabricated `User`, which is honest because
`_require_canvas_manager` is platform-only and owner-or-admin, so a real
Trinity user is always behind it. Ids and counts only (G-04). The three
routes become `async def` to await it, matching their operator twins, which
already call the same sync db functions from an async handler.
Pinning is audited too, on BOTH surfaces — the operator route was the one
recording nothing. A pin decides which canvas an entire roster sees first, so
it is an administrative act on a shared surface, not a per-viewer preference.
3. **`canManage` comes from the parent.** It was hardcoded `true` on the
argument that the server decides. It does — but a merely-shared user was then
shown Manage → Delete / Pin and got a 403, which is the failing-control
problem `can_manage_canvases` exists to prevent on the Workspace. Agent
Detail passes `agent.can_share`, the same predicate `ReportsPanel` five lines
above already reads and the same one `_gate_human_removal` enforces. The prop
defaults FALSE, so a caller that forgets it hides an affordance rather than
offering one that refuses.
4. **`get_agent_card` resolves `can_manage_canvases`.** It omitted it, so the
same owner read `true` in the sidebar and `false` on the agent's own page —
the disagreement #2160's own docstring says that function exists to prevent.
5. **An agent genuinely cannot pin its own canvas now.** The user doc said so;
`_gate_human_removal` allowed it (right for delete — an agent tidying up
after itself — and wrong for pin), and "no MCP tool exposes it" is a property
of the client, not of the route. `_gate_pin` is humans-only, which makes the
documented sentence true rather than aspirational.
Tests: the audit guard now walks the portal routes as well as `routers.canvas`
(it only ever inspected the latter, which is why three unaudited routes passed
it), plus pin-audit parity, the humans-only pin gate beside the still-permitted
agent self-delete, the feature-flags constant being the same object the refusal
is raised from, the agent-card/roster agreement, and four frontend wiring cases.
1025 backend / 2538 frontend tests green.
Related to Abilityai/trinity-enterprise#553
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RSjVEjay9ztC1oDXXkh9uN
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds comprehensive SMARTS trading pipeline support:
SMARTS Agent Templates
config.yamlformat (SMARTS-style agents)system.yaml)Telegram Notifications
NotificationConfigin process enginesmarts_summary_service.pyfor generating trading summariesAPI & Scheduler
Miro Visualization
<br>tags) for card textdata/smarts-flows/directory for JSON exportsOther
find_template_fileusage across codebaseTest plan
🤖 Generated with Claude Code