security: Fix token logging and add HTML reports to gitignore - #7
Merged
vybe merged 2 commits intoJan 18, 2026
Merged
Conversation
Previously, the client.ts logged first 20 characters of JWT tokens and token length on every API request, which could expose sensitive information in production logs (CloudWatch, Datadog, etc.). Changes: - Add environment-aware debug logging (DEBUG_MCP_CLIENT or NODE_ENV=development) - Replace token content logging with simple auth presence indicator - In production: no token information is logged - In development: only logs whether auth is present/missing
Prevents auto-generated pytest HTML test reports from being accidentally committed. These reports are development artifacts that should remain local.
oleksandr-korin
added a commit
that referenced
this pull request
Jan 19, 2026
Test Results: - T4.1: Agent error (AGENT_UNAVAILABLE) ✅ - T4.2: Agent timeout⚠️ (bug: step status not updated) - T4.3: Retry policy ✅ - T4.4: Skip on error (on_error:skip_step) ✅ - T4.5: Cancel execution ✅ Key Findings: - Non-existent agent triggers clean AGENT_UNAVAILABLE error - on_error: {action: skip_step} works correctly - Cancel API works immediately BUG FOUND (Issue #7): - Step timeout detected (error.code='TIMEOUT') - But step status remains 'running' instead of 'failed' - Execution doesn't transition to failed state - Impact: Timeout processes may hang indefinitely Running Total: 15/22 tests passing (68%) - ABOVE TARGET ✅ Refs: PROCESS_ENGINE_ROADMAP.md Phase 1
oleksandr-korin
added a commit
that referenced
this pull request
Jan 19, 2026
Test Results: - T4.1: Agent error (AGENT_UNAVAILABLE) ✅ - T4.2: Agent timeout⚠️ (bug: step status not updated) - T4.3: Retry policy ✅ - T4.4: Skip on error (on_error:skip_step) ✅ - T4.5: Cancel execution ✅ Key Findings: - Non-existent agent triggers clean AGENT_UNAVAILABLE error - on_error: {action: skip_step} works correctly - Cancel API works immediately BUG FOUND (Issue #7): - Step timeout detected (error.code='TIMEOUT') - But step status remains 'running' instead of 'failed' - Execution doesn't transition to failed state - Impact: Timeout processes may hang indefinitely Running Total: 15/22 tests passing (68%) - ABOVE TARGET ✅ Refs: PROCESS_ENGINE_ROADMAP.md Phase 1
vybe
added a commit
that referenced
this pull request
Apr 4, 2026
Define 16 structural invariants in architecture.md that must be preserved across changes (layering, DB patterns, router ordering, auth, etc.). Reference them from CLAUDE.md as rule #7 with weekly validation cadence. Add /validate-architecture skill to check codebase compliance. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8 tasks
This was referenced Apr 20, 2026
Closed
3 tasks
vybe
added a commit
that referenced
this pull request
May 8, 2026
…checks Add governing principle #7 to TARGET_ARCHITECTURE.md: data exchange over conversation chains as the default multi-agent composition pattern. Add Composability category (I-001–I-005) to agent-validation-spec.md: checks that agents declare output contracts, produce structured file-based outputs for downstream consumers, and enforce contracts via post-check hooks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced May 11, 2026
4 tasks
oleksandr-korin
added a commit
that referenced
this pull request
May 21, 2026
The chat path (claude_code.py) was missing the _classify_signal_exit call that was added to the headless path (headless_executor.py) for Issue #516. When a SIGKILL terminates the claude subprocess at 0 turns (cgroup OOM, host SIGKILL, watchdog cancel), the chat handler would fall straight into _diagnose_exit_failure, which returns "Subscription token may be expired or revoked. Generate a new one with 'claude setup-token'." even when no auth signal was observed. This misclassification: - Misleads operators into chasing token regeneration when the actual cause is OOM / timeout / external kill - Pollutes the SUB-003 auto-switch trigger pattern matcher (which reads the error string), causing spurious subscription rotations on agents whose subscriptions are provably healthy - Burns the auto-switch 2-hour skip-list slot on phantom auth failures Fix mirrors the existing pattern in headless_executor.py:683 — call _classify_signal_exit first, fall through to _diagnose_exit_failure only for non-signal exits. No new logic; the classifier already produces the honest "Execution terminated by SIGKILL after N tool calls / N turns" message. Adds a structural regression test (parametrized over both files) that pins the call ordering — _classify_signal_exit must appear before _diagnose_exit_failure in both call sites, otherwise the auth-fallback heuristic re-introduces the misclassification. Deployment: requires base image rebuild + agent restart for the fix to take effect on running agents (per CLAUDE.md note #7). Out of scope: Fix 2 (gate auto-switch on observed wire 401/403/429) and Fix 3 (cgroup OOM event reading) — both flagged in #906 as follow-up improvements. Fixes #906 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
12 tasks
oleksandr-korin
added a commit
that referenced
this pull request
May 27, 2026
New `GET /api/agents/{name}/schedules/{schedule_id}/analytics` endpoint
returns counts, success rate, duration p50/p95/p99, cost total, tool-call
top-5 by total wall time, and a UTC daily timeline. Default window 7d
(also 24h / 30d). Inline `ScheduleAnalyticsCard.vue` renders inside
`SchedulesPanel.vue`'s expanded-schedule region — pure CSS, no Chart.js.
Implementation notes (locked by /autoplan + /review):
- Percentiles via `statistics.quantiles(method="inclusive")` over the
newest 5,000 success rows (`_PERCENTILE_ROWSET_CAP`). Counts and
timeline use the full unsampled rowset. `sampled` flag in response.
- Tenant boundary in DB layer (`schedule.agent_name != agent_name`
→ None → 404). `AuthorizedAgent` only validates the URL agent name;
user-supplied `schedule_id` is verified against ownership.
- Tool-call top-5 weighted by `sum(duration_ms)` per tool (not count),
avoiding `Read`/`Bash` dominating low-signal frequency leaderboards.
- Timeline gap-filled Python-side; UTC bucketing via
`substr(started_at, 1, 10)`; documented on the route.
- `window_hours` server-validated to `{24, 168, 720}` → 422 otherwise.
- Soft-deleted schedules return 404 (matches `get_schedule()` policy).
- Frontend uses the shared `api` client (CLAUDE.md invariant #7) and
`useFormatters().formatDuration` composable.
Per-agent rollup and per-chat-session analytics deferred — see issue
body for the destination map (#18 / follow-up).
12 unit tests cover percentile correctness, time-window boundary,
empty + all-running edge cases, NULL duration exclusion, malformed
JSON skip, cross-tenant 404, soft-deleted 404, sampling boundary,
timeline gap-fill, tool-call duration weighting.
CSO diff scan: zero findings (8/10 confidence gate).
Fixes #868
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
vybe
pushed a commit
that referenced
this pull request
Jun 1, 2026
…957) (#976) Avatar Generate dialog showed only "Failed to generate avatar" with no diagnostic info — operators couldn't tell whether the failure was a missing API key, an upstream rate limit, a safety-filter rejection, or a network timeout. Root causes: - Backend returned the raw upstream exception string as the HTTP detail. In several real failure modes (nginx 504 with HTML body, network abort) the frontend got no JSON detail at all and fell back to a hardcoded generic message. - Frontend used bare `axios` instead of the shared `@/api` client (Invariant #7), with no per-status fallback chain. Backend: - `ImageGenerationResult.error_kind` — coarse classification (`not_configured` | `invalid_input` | `safety_filter` | `rate_limited` | `upstream_error` | `timeout` | `unknown`) set on every failure path. - `_classify_exception()` maps httpx + RuntimeError exceptions to a kind. - Catch blocks now use structured logging via `extra={...}` so Vector indexes agent_name, error_kind, exception_type, etc. as fields. - `_AVATAR_ERROR_HTTP` map → kind to (HTTP status, friendly detail). `generate_avatar` and `regenerate_avatar` use the map instead of hardcoded 422 + raw exception text. Service-not-available early-exit uses the same friendly text. Frontend: - `AvatarGenerateModal.vue` switched from bare `axios` to `@/api` and bumped the per-request timeout to 180s (image gen can take >30s). - `describeAvatarError(err, verb)` falls back gracefully on 502/503/504 and no-response cases so the user gets a directional message even when the upstream strips the JSON detail. Tests: - 7 new cases in `tests/unit/test_image_generation_service.py` cover `_classify_exception` and the `error_kind` field default. Related to #957 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Jun 2, 2026
…Settings (#995) (#996) * feat(enterprise-ui): User & Org Management view on the #847 seam (#995) Public OSS-bundle frontend for the private user_management module (Abilityai/trinity-enterprise#2). Gated entirely server-side by the `user_management` entitlement — hidden in OSS-only builds and bounced by the route guard on direct URL visits. - views/enterprise/UserManagement.vue: org list + create, membership add/remove, seat counts. Light + dark. No algorithmic IP (CRUD glue over the private /api/enterprise/user-management/* endpoints). - stores/orgManagement.js: domain store, calls via shared axios + auth header (Invariants #6/#7). - router: /enterprise/user-management gated meta.requiresEntitlement: 'user_management' (mirrors the audit route). - views/enterprise/Index.vue: add the catalogue card (available). No public backend/schema/model changes — the entire data model + logic lives in the private submodule per the enterprise open-core split. The submodule pointer is intentionally NOT bumped here; it advances after trinity-enterprise#2 merges. Verified: all four files compile; the only build blocker is the pre-existing unrelated `mermaid` import in AgentWorkspace.vue (stale local node_modules; resolved by CI npm ci). Related to #995, #847 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(enterprise-ui): per-user activity audit in Settings → User Management (#995) Integrates the enterprise activity view INTO the existing OSS user management table (not a separate page). When user_management is entitled, each user row gets a "View activity" action opening a drawer with that user's audit summary + timeline, fetched from the private /api/enterprise/user-management/users/{id}/activity endpoint. - Gated entirely by enterpriseStore.isEntitled('user_management') — column + drawer hidden in OSS-only builds. - No change to the existing role-CRUD behaviour; purely additive column. - loadFeatureFlags() in onMounted (cached/no-op when NavBar already ran). Related to #995 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(user-mgmt): OSS deactivation primitive + enterprise lifecycle UI (#995) Pivots #995 from Organizations to the real net-new gap — user onboarding/offboarding — integrated into the existing Settings → User Management table (not a separate page). OSS primitive (edition-agnostic, small): - users.suspended_at column + migration + surfaced in get_user/list_users. - get_current_user rejects suspended users (both JWT + MCP-key paths), so setting the column blocks new logins AND invalidates live tokens on the next request. - /api/users exposes suspended_at (read-only) so the gated UI can render Deactivate/Reactivate. Enterprise UI (gated by user_management entitlement, hidden in OSS): - Settings → User Management gains an "Invite user" form, per-row Deactivate/Reactivate (not for self or the built-in admin), and the per-user Activity drawer. All call the private /api/enterprise/user-management/* endpoints. Removed: the separate /enterprise/user-management Orgs page, its route, store, and Index card (orgs dropped — single-tenant). Index card now points at Settings. No change to the existing OSS role-CRUD behaviour. Verified live: /api/users carries suspended_at; suspend/reactivate/invite/activity all work; OSS-only builds hide every enterprise control. Related to #995, #847 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(user-mgmt): stop Management column clipping in User Management table (#995) The users table wrapper was overflow-hidden; the extra entitlement-gated Management column pushed total width past the card and clipped the right-side action buttons. Switch to overflow-x-auto so the wider table scrolls within the card instead of clipping. Related to #995 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(user-mgmt): fit User Management table in the card (no h-scroll) (#995) Replace the overflow-x-auto stopgap with an actual fit: trim cell padding px-6→px-4 across the table and let the Management actions wrap within their column (flex-wrap, text-xs, no whitespace-nowrap). The 5-column table now fits the max-w-4xl settings card without clipping or a horizontal scrollbar. Related to #995 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: add suspended_at to schedule-soft-delete test users DDL (#995) The #995 users.suspended_at primitive added the column to _USER_COLUMNS, so get_user_by_*() now SELECTs it. test_schedule_soft_delete builds its own users table with a hardcoded DDL that lacked the column, causing "no such column: suspended_at" (4 regression-diff failures). Mirror the schema change in the test DDL. Related to #995 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(architecture): document enterprise modules + two-track migrations (#995/#997) - users.suspended_at deactivation primitive (OSS column + enforcement; enterprise-only setter) on the users table + a callout. - Invariant #3 extended: enterprise migrates enterprise_* tables via a separate runner tracked in enterprise_schema_migrations (one file per migration; never ALTERs OSS tables; runs after OSS init). - feature-flags doc gains enterprise_features. - New "Enterprise Modules (#847 seam)" section: audit / user_management / siem entitlements, surfaces, and the gating model. Related to #995, #997, #847 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(enterprise): enterprise registration failure must not crash core boot (#995/#997) main.py wrapped register_enterprise(app) in `except ImportError` only — so a bug in enterprise registration (schema init, migration, router mount, pusher start) would propagate and crash backend startup on an enterprise build. Add a broad `except Exception` that logs loudly + a traceback and continues in OSS-only mode. Modules registered before the failure stay active; the rest are simply absent from feature-flags. The core platform always boots. OSS-only builds are unaffected (still the ImportError path). Verified: happy path still boots (health 200) and registers ['audit','siem']. Related to #995, #997, #847 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 2, 2026
vybe
added a commit
that referenced
this pull request
Jun 9, 2026
Phase 2 of #740: adds a Loops tab on the Agent Detail page over the existing dev backend (routers/loops.py, loop_service.py) — no backend changes. - stores/loops.js: agent-scoped Pinia store on the shared api.js client (Invariant #7). Filters fleet-wide loop_run_completed/loop_completed WS events by the mounted agent, targeted-refreshes only the affected loop, and runs a 12s backstop poll while any loop is queued/running to recover a missed terminal event. - components/LoopsPanel.vue: Run-loop form (message template w/ {{run}} + {{previous_response}} helper, max_runs, stop_signal, delay, timeout, ModelSelector, allowed_tools), loop list with status/runs/stop_reason, expandable per-run table, last response via DOMPurify renderMarkdown, cooperative Stop control. - AgentDetail.vue: Loops tab between Schedules and Playbooks. - websocket.js: route loop events to the store in the type-keyed branch. - e2e/loops-panel.spec.js + architecture/feature-flow docs. Verified live: tab renders, form submits, loop row reaches terminal state via the live-update path, expanded detail renders the per-run table. Closes #1106 Co-authored-by: Eugene Vyborov <eugene@beingluminous.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dolho
added a commit
that referenced
this pull request
Aug 13, 2026
An agent had no home. A roster row emitted `new-chat-with-agent`, so there was
nowhere to see what an agent had been doing, nowhere for it to ask you something
while no chat was open, and nowhere to show what it can do. Clicking an agent
now opens its page; Start a chat is an explicit button there.
Header (avatar, name, description, health, last active), a stats strip
(activity chart, tasks in window, completed rate, first-try rate), and five
tabs: Overview · Reports · Files · What it can do · Activity. Overview leads
with what the agent is waiting on you for, then recent work, then your chats
with it.
**The interesting half is what it does NOT carry, and where that is enforced.**
The page reports, it does not configure — no schedules, no skill editing, no
logs, no costs, and model/plan are not shown at all. And the same page serves an
external portal-token client as well as a platform user, which makes it a
security surface rather than a layout exercise.
So every exclusion is a PROJECTION in the service, before the payload exists,
not a filter in the template. A template filter is correct until somebody adds a
column to a list view, and nothing fails when they do; a field that never leaves
the service cannot be surfaced by a later edit. Three that matter:
* `recent_work` drops `message` (another user's prompt), `cost` and
`model_used` (excluded by AC #7), and `source_user_email`.
* `asks` admits only agent-authored approval/question items — never platform
`alert`s, which are ops telemetry (sync-failing, git-bloat, breaker) rather
than an agent asking a person anything — and never their `context`, which is
free-form agent JSON and a known credential-leak surface (canary G-04).
* report reads are agent-scoped. Report ids are global and the roster gate
proves only that the caller may reach THIS agent, so without the ownership
check the page would read every report in the install. A foreign id answers
with the same 404 as a missing one.
Everything is DB-sourced, so a stopped agent renders degraded rather than empty
and a failing data source degrades only its own section. Health reports
`unknown` rather than `unhealthy` when nothing has checked the agent —
monitoring is default-OFF, so on many installs that is every agent.
Reuses the analytics accessor the issue names, via the DB layer rather than over
HTTP (the platform endpoint is JWT-gated and a portal client cannot call it),
and "what it can do" projects the roster briefing rather than building the
competing mechanism ent#178 will own.
Two AC #3 metrics, opposite outcomes. **First-try rate ships** and is real:
successes with `retry_count` 0, deliberately distinct from the success rate,
which counts a retried-then-succeeded execution as a success. **Rating tally
does not**: there is no rating, thumbs or feedback mechanism anywhere in Trinity
— no table, no column, no endpoint — so it has no data source and was omitted
rather than invented. Recorded in the requirement as the one bullet not met.
Supersedes ent#359's interim roster-click behaviour (a row with unread opened
the unread chat). Nothing is lost: the badge still shows on the row, and the
page's Overview lists the chats it belongs to with their counts.
Verified live: 37 executions, 89% completed, 33/37 first try, `recent_work`
carrying exactly the six safe keys, and 404/422/401 on the gates.
Related to Abilityai/trinity-enterprise#360
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Aug 13, 2026
* feat(workspace): give each agent a page (ent#360)
An agent had no home. A roster row emitted `new-chat-with-agent`, so there was
nowhere to see what an agent had been doing, nowhere for it to ask you something
while no chat was open, and nowhere to show what it can do. Clicking an agent
now opens its page; Start a chat is an explicit button there.
Header (avatar, name, description, health, last active), a stats strip
(activity chart, tasks in window, completed rate, first-try rate), and five
tabs: Overview · Reports · Files · What it can do · Activity. Overview leads
with what the agent is waiting on you for, then recent work, then your chats
with it.
**The interesting half is what it does NOT carry, and where that is enforced.**
The page reports, it does not configure — no schedules, no skill editing, no
logs, no costs, and model/plan are not shown at all. And the same page serves an
external portal-token client as well as a platform user, which makes it a
security surface rather than a layout exercise.
So every exclusion is a PROJECTION in the service, before the payload exists,
not a filter in the template. A template filter is correct until somebody adds a
column to a list view, and nothing fails when they do; a field that never leaves
the service cannot be surfaced by a later edit. Three that matter:
* `recent_work` drops `message` (another user's prompt), `cost` and
`model_used` (excluded by AC #7), and `source_user_email`.
* `asks` admits only agent-authored approval/question items — never platform
`alert`s, which are ops telemetry (sync-failing, git-bloat, breaker) rather
than an agent asking a person anything — and never their `context`, which is
free-form agent JSON and a known credential-leak surface (canary G-04).
* report reads are agent-scoped. Report ids are global and the roster gate
proves only that the caller may reach THIS agent, so without the ownership
check the page would read every report in the install. A foreign id answers
with the same 404 as a missing one.
Everything is DB-sourced, so a stopped agent renders degraded rather than empty
and a failing data source degrades only its own section. Health reports
`unknown` rather than `unhealthy` when nothing has checked the agent —
monitoring is default-OFF, so on many installs that is every agent.
Reuses the analytics accessor the issue names, via the DB layer rather than over
HTTP (the platform endpoint is JWT-gated and a portal client cannot call it),
and "what it can do" projects the roster briefing rather than building the
competing mechanism ent#178 will own.
Two AC #3 metrics, opposite outcomes. **First-try rate ships** and is real:
successes with `retry_count` 0, deliberately distinct from the success rate,
which counts a retried-then-succeeded execution as a success. **Rating tally
does not**: there is no rating, thumbs or feedback mechanism anywhere in Trinity
— no table, no column, no endpoint — so it has no data source and was omitted
rather than invented. Recorded in the requirement as the one bullet not met.
Supersedes ent#359's interim roster-click behaviour (a row with unread opened
the unread chat). Nothing is lost: the badge still shows on the row, and the
page's Overview lists the chats it belongs to with their counts.
Verified live: 37 executions, 89% completed, 33/37 first try, `recent_work`
carrying exactly the six safe keys, and 404/422/401 on the gates.
Related to Abilityai/trinity-enterprise#360
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(workspace): scope the #2128 rooms-gate guard to PortalRoom's own element
F18 asserts the rooms capability is a term in `<PortalRoom>`'s mount condition.
Its mechanism was `indexOf('v-if=', roomAt)` — unscoped, and matching `v-if=`
literally.
ent#360 puts the agent page ahead of the room in the chain, so PortalRoom became
`v-else-if`. That string does not contain `v-if=`, so the search walked straight
past it into the NEXT branch's `v-if` and reported a missing gate that was in
fact present, one line above where it looked.
Now scoped to PortalRoom's own element and accepting either form. Verified by
mutation: deleting `store.multiAgentChatAvailable` from the condition still
fails the test, so the protection is intact rather than relaxed to fit.
Related to Abilityai/trinity-enterprise#360
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>
vybe
added a commit
that referenced
this pull request
Aug 17, 2026
* test(e2e): List-mode spec + five specs adjusted for the retired Agents page (ent#260)
NEW dashboard-list-view.spec.js: system-row render (data-agent hooks, SYSTEM
badge, detail link; deliberately no toggle-visibility assertions — CI's only
agent is the guarded system agent), AC-4 mode persistence, AC-2 redirect
(param stripped, saved mode NOT rewritten), ?onboarding=1 survives the
redirect + opens the wizard, name-filter narrowing + filtered-empty Clear-all
recovery, three-mode round-trip.
smoke.spec.js: Agents nav-link assertion dropped (nav list comment updated);
'agents page loads' becomes the redirect assertion. browser-tab-titles: the
Agents hop removed from the SPA chain (Dashboard → Templates still proves the
client-side repaint); /agents → 'Trinity — Dashboard' joins the redirect-title
test. dashboard-grid-view: header comment (three modes; exact-name toggle
selectors unaffected). dashboard-stats-overflow: 'list' added to MODES — the
third toggle + chassis Create button widen the controls cluster in every mode.
navbar-overflow test 3: the 640px squeeze branch is now conditional on the
MEASURED overflow (4-link OSS bar may fit where 5 links overflowed; an
entitled build still exercises the scroll-recovery branch) — sm=640 is the
floor, below it the link row hides, so narrowing further was not an option.
Full e2e run defers to CI (frontend-e2e on the ui label) — the live local
stack is not exercised from this worktree.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs: Dashboard List view — architecture block, new feature flow, index + superseded banner (ent#260)
architecture.md: Grid-view paragraph reworded to three modes; new Dashboard
List view block (AgentListPanel extraction, visibleAgents seam ownership,
N+1 deletion, redirect + NavBar consolidation, zero backend change).
NEW feature-flows/dashboard-list-view.md (overview, data-flow diagram,
decisions D1/D2/D7/D8/D11 + the two flagged plumbing calls, teardown
state-loss note, testing). dashboard-grid-view.md mode-count wording.
agents-page-ui-improvements.md superseded banner (kept as history).
feature-flows.md: List-view index row, Agents-page row → superseded pointer,
changelog entry.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(flows): repoint cross-cutting flow docs off the retired Agents page (ent#260)
/sync-feature-flows coverage pass over the full branch diff: eleven flow docs
carried live current-state claims about views/Agents.vue (avatar/toggle/meter
usage tables, sync-health dot renderer, system-agent display, tag bulk-ops
entry points, skills-on-start entry, dark-mode coverage row, timeline-view
mode-toggle description). Live references now point at
components/AgentListPanel.vue / the Dashboard chassis; dated changelog entries
and explicitly-labelled historical sections are left as history. Also fixes
dashboard-timeline-view.md's pre-existing stale '[Graph] [Timeline]' toggle
claim to the current three-mode set.
Touched: agent-avatars, agent-lifecycle, agent-tags, autonomy-mode,
autonomy-toggle-component, dark-mode-theme, dashboard-timeline-view,
git-sync-health, internal-system-agent, parallel-capacity, read-only-mode,
skills-on-agent-start.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(dashboard): review fixes — Create-button icon-only degrade + gated fetch write-through (ent#260)
Two review-stage fixes on the ent#260 branch:
1. Create Agent label degrades to icon-only below `md` (title +
aria-label kept). The controls cluster is flex-shrink-0; measured
against the #1830 stats-ladder constants (agents-only floor 71px),
the full label at 640px in grid mode (+ the third mode button)
leaves ~47px — the stats-overflow spec's clip assertion would fire.
This is the plan's pre-decided degrade, applied ahead of CI.
2. The fetchAgents → agentsStore.agents write-through is now gated on
no active quick-tag filter (params.tags narrows the response
server-side; a filtered subset must not clobber the full-fleet list
agentsStore consumers read — the Executions dropdown's cold-start
self-heal is length===0-gated and never recovers from a non-empty
wrong value) and writes a shallow copy (array identity per store,
shared row objects so in-place status/label patches still propagate).
Docs synced (architecture block, flow doc, agents.js contract comment)
+ a learnings.md entry for the write-through class.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(requirements): §9.9 seam wording matches the implemented deviation (ent#260)
The plan's D8 wired both convertAgentsToNodes call sites through the
visibleAgents computed; the implementation deliberately deferred the
timeline wiring to ent#261 (ReplayTimeline :agents prop switch —
rewiring the node paths was rejected as timeline-mutation risk) and the
code comment + flow doc + architecture all record that. §9.9 still
claimed the node-rebuild call sites consume the seam — align it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor(frontend): rename Templates.vue to Library.vue (pure move) (ent#263)
Byte-pure git mv with zero content edits so git rename detection holds
and a parallel edit to Templates.vue (ent#260) resolves as a content
merge inside Library.vue rather than a modify/delete conflict. All
content changes land in follow-up commits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(frontend): Library page + route/nav rename with /templates redirect (ent#263)
- Library.vue: h1 'Library', new subtitle, templates content wrapped in an
'Agent Templates' section (own loading/error/empty states; inner headings
demoted h2->h3); fetch migrated to the shared api client (Invariant #7)
- router: /library route (meta.title Library) + /templates function-form
redirect carrying query AND hash; route name Templates->Library (no named
pushes exist)
- NavBar: label Library, to=/library, active via startsWith('/library')
- CreateAgentModal: its single raw-axios /api/templates call migrated to the
shared api client so no half-migrated consumer of the endpoint remains
Page-identity naming only (AC#4 reading): the asset-kind noun 'template'
survives inside the Library (Starter/GitHub Templates sections, Use Template).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(frontend): Library skills section — fleet browse over the skills library (ent#263)
- stores/skillsLibrary.js (new): fleet-scoped store, deliberately separate
from stores/skills.js (KeepAlive-cached AgentDetail means SkillsPanel's
clear() never fires on nav-away — shared refs would poison the cached tab);
imports nothing from stores/skills.js. 4-state emptyReason discriminator
(unconfigured/not_cloned/empty + error carried separately); sync() with a
180s timeout and ECONNABORTED -> status-refetch (a first clone can outlive
the 30s api.js default; client timeout != server failure)
- components/LibrarySkillsSection.vue (new): sync-state header leads with
commit_sha + skill_count (disk-derived; last_sync is per-worker in-memory
and renders only when truthy); repo URL admin-only, userinfo-stripped,
labeled 'Primary source', hidden when status.sources reports >1 (#1901
forward-compat); admin Sync now; per-kind empty states teaching the next
action; dormant source_name/shadowed_by slots; interpolation only
- components/skills/{SkillContractChips.vue,contract.js} (new): the #183
contract-chips seam extracted from SkillsPanel so both the per-agent tab
and the Library browse render package facts from one seam
- SkillsPanel.vue: consumes the shared seam (local SkillMeta/formatBytes/deps
removed); stores/skills.js untouched
- Library.vue: skills section wired in + header jump anchors (no ?kind=)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: Library e2e anchors + remove dead template-endpoint tests (ent#263)
- smoke.spec.js: '@smoke library page loads' asserts chrome-only getByRole
headings (h1 Library + both section headings — the old getByText(/template/i)
passed on the un-renamed page and proved nothing); new '@smoke templates path
redirects to library'; stale nav comment fixed (Health/Ops merged in #1109)
- browser-tab-titles.spec.js: nav click asserts 'Trinity — Library'; redirect
test also covers /templates -> /library title resolution
- tests/test_templates.py: remove TestEnvTemplate + TestTemplateRefresh —
GET /api/templates/env-template and POST /api/templates/refresh no longer
exist (router has exactly 2 GET routes; live-stack probe returns 404);
test_get_template_by_id now exercises the REAL detail endpoint instead of
the dead env-template detour that always skipped. 5 remaining tests pass
against the live stack.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs: Library page — requirements, architecture, feature flow, stale-endpoint rot sweep (ent#263)
- requirements/core-agent.md: new §4.5 Library Page — unified /library surface
(agent templates + fleet skills browse), query+hash-preserving /templates
redirect, stacked sections, per-kind empty states, the AC#4 page-identity
naming rule; fleet assignment visibility named as Not Built
- requirements/skills.md (surgical — §21.3/§22.2/new §22.3 only, avoiding PR
#1901's §21.1/§21.5 hunks): §21.3 stale 'Skills tab is hidden' note corrected
(visible since ent#235/PR #1877); §22.2 rewritten as visible/rebuilt; new
§22.3 Library Page fleet skills browse — browse-only over the existing
/api/skills/library reads, own skillsLibrary store + the KeepAlive rationale,
admin-only URL/Sync, #1901 forward-compat, assignment read = Not Built
- architecture.md: 'Top-nav IA — Library (ent#263)' paragraph beside the #1109
Operations one; stale 'Templates (4 endpoints)' table corrected to the 2 real
routes (POST /refresh AND GET /env-template both verified absent)
- feature-flows: templates-page.md git-mv'd to library-page.md + full rewrite
(the old file was deeply stale — AgentSubNav, dead endpoints); index row +
platform-settings.md Related-Flows link repointed
- template-processing.md + CREDENTIAL_MANAGEMENT.md: dead env-template
endpoint references removed/replaced (same rot class as the architecture
table); Templates.vue references repointed at Library.vue
- user docs (creating-agents.md, faq/agents.md): Templates page → Library
(+ redirect note)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(requirements): TGRAM-PROGRESS — Telegram in-progress status indicator (ent#264)
New §15.1h: reaction ack + elapsed-time placeholder + channel-agnostic
start/progress/resolve seam, group gating (mention/reply OR all-mode;
observe stays silent), degradation ladder, default-ON per-binding toggle,
GET /telegram access hardening. §15.1c/§15.1e touch-ups.
Requirements-first per Rules of Engagement #1.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(channels): in-flight progress indicator seam + Telegram reaction ack and elapsed-time placeholder (ent#264)
Channel-agnostic start/progress/resolve seam (Invariant #9):
- base.py: default-no-op indicate_progress hook + progress_threshold_seconds/
progress_interval_seconds capability attrs (None => the router never arms a
driver, so Slack/WhatsApp/VoIP behave byte-identically).
- message_router.py: router-owned per-turn driver — _arm_progress_driver after
step 8 (call + arm wrapped so a raising hook can never abort the turn),
_progress_loop (elapsed origin captured BEFORE the threshold sleep; per-tick
try/except), _resolve_indicator at all three terminals (cancels AND awaits
the driver dead, settles the shielded in-flight placeholder send, THEN
indicate_done — closes the tick-after-resolve and resolve-vs-first-send
races by construction; carries success for the adapter's neutral fallback
line), and an idempotent try/finally _cancel_progress_driver backstop.
- telegram_adapter.py: indicate_processing upgraded (single binding read,
per-turn cfg stash, typing preserved verbatim, 👀 reaction ack — gated on
the default-ON per-binding toggle and ack-eligibility; whole body never
raises); NEW indicate_progress (placeholder send-then-edit, message_id
recorded INSIDE the shielded helper, disable_notification, explicit HTML,
2-consecutive-failure degraded flag over sends/edits/timeouts alike) and
indicate_done (clear reaction at every terminal — no success-👍 swap;
delete placeholder, neutral edit-to-done fallback); fail-soft primitives
_set_message_reaction/_edit_message_text/_delete_message/
_send_placeholder_message mirroring _send_message's shape (429 retry_after
capped 30s; never log the token-bearing URL).
- parse_message stashes progress_ack_eligible: DMs, @mention/reply group
turns, and `all`-trigger-mode groups; observe mode stays typing-only.
- All per-turn state on NormalizedMessage.metadata (the adapter is a shared
singleton); decrypt_telegram_bot_token facade passthrough so the hook
decrypts from the row already in hand.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(telegram): per-binding progress-indicator toggle — dual-track migration, API, UI (ent#264)
Default-ON toggle for the in-progress indicator, per-agent == per-binding:
- Dual-track migration (Invariant #3 / Rule #9), all six touch points:
SQLite `telegram_progress_indicator` in db/migrations.py + Alembic
0031_telegram_progress_indicator (down_revision 0030; renumber-at-rebase
rule vs ent#265 applies to whichever PR merges second) + schema.py +
tables.py DDL + _BINDING_COLUMNS + _row_to_binding. No backfill UPDATE:
ADD COLUMN ... DEFAULT 1 populates existing rows (default ON is the AC).
- Read predicate evaluated in Python, never SQL (`NULL != 0` is NULL):
enabled ⇔ `v is None or v != 0` — only an explicit 0 disables.
- db: set_progress_indicator_enabled + set_telegram_progress_indicator facade.
- API: GET /api/agents/{name}/telegram surfaces progress_indicator_enabled
(pinned in the hand-built response, #1809 lesson — configure PUT too) and is
access-hardened get_current_user → AuthorizedAgentByName (the response
carries webhook_url, which embeds the webhook secret — previously readable
by ANY authenticated user; uniform-404 accessor, Invariant #8). New
PUT /api/agents/{name}/telegram/progress-indicator — OwnedAgentByName +
reject_agent_principal (behavior toggles are human-only, ent#223 lesson),
404 when no binding; dedicated route so toggling never re-sends the token.
- models.py: TelegramProgressIndicatorRequest + response field (Invariant #14).
- UI: TelegramChannelPanel.vue switch (SlackChannelPanel allow_proactive
precedent), optimistic flip with revert-on-error, dark-mode aware.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(architecture): channel-adapter progress-indicator seam deltas (ent#264)
base.py indicate_progress hook + capability attrs; message_router.py per-turn
driver (arm/tick/resolve + backstop, inline-sync coupling named for #1081
pickers); telegram_adapter.py reaction ack + elapsed placeholder + default-ON
per-binding toggle.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(telegram): progress-indicator unit suite — adapter, router driver, toggle (ent#264)
61 tests, no backend required. Highlights per the plan's scrutiny list:
resolve-vs-first-send race (shielded id recording → delete finds it),
cancel-and-await ordering at both terminal flavors, 429-backoff cancel
promptness, [NO_REPLY]-still-resolves, step-8 wrap regression guard,
singleton metadata isolation, degraded-quiesce after 2 consecutive failures,
static-template-only egress pin (ent#224 class), live-select column test
(4-file schema rule), real-DB facade round-trip (#1533 no-MagicMock),
legacy-table SQLite migration + renumber-safe Alembic-twin existence,
GET JSON field pinning (#1809) + AuthorizedAgentByName hardening pin,
PUT reject_agent_principal 403.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(feature-flows): telegram in-progress status indicator section (ent#264)
New section in telegram-integration.md: three fail-soft layers, the
channel-agnostic start/progress/resolve seam, race closures (tick-after-
resolve, resolve-vs-first-send, singleton concurrency), gating matrix
(incl. `all`-mode groups; observe stays typing-only), toggle surface,
degradation ladder / Bot API constraints, restart residual + the inline-sync
coupling named for #1081/#1083 pickers, test inventory. Index unchanged
(existing flow). Revision-history row added.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(telegram): access-harden the groups GET; document the bounded settle residual (ent#264)
Review-stage fixes:
- GET /api/agents/{name}/telegram/groups moves from bare get_current_user
to the uniform-404 AuthorizedAgentByName accessor — the sibling read of
the panel flow this PR already hardened. Group chat ids/titles/welcome
text are tenant data; the only follow-up action (the group-message POST)
was already OwnedAgentByName, so the read tier is strictly broader than
every usable consumer and no flow narrows (incl. the MCP
list_channel_groups path, whose paired send is owner-gated).
- Route-dependency test pinning the groups-GET accessor (mirrors the
binding-GET hardening pin).
- Docs honesty: the resolve-path in-flight settle is bounded at 10s — a
first placeholder send slower than that (429 retry_after >= 10s) is
abandoned and can strand a self-dating placeholder. Named in the feature
flow Residuals + requirements Known residuals instead of implying the
race is closed unconditionally.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(db): source_channel_agent + telegram allow_proactive columns (ent#265)
Dual-track migration (Invariant #3 / #1183): SQLite entry
channel_report_back_columns + Alembic 0031_channel_report_back (single
linear head off 0030), plus db/schema.py + db/tables.py so fresh builds
stay correct.
- schedule_executions.source_channel_agent (nullable TEXT): the agent
whose channel binding owns the execution's INHERITED context (D1
Option A). Written only at the /task row-creation point; NULL for
direct rows (reporter falls back to the executing agent).
- telegram_group_configs.allow_proactive INTEGER DEFAULT 1: per-group
completion-report consent, default ALLOW for existing AND new groups
(opt-out mute; no backfill UPDATE needed).
- create_task_execution accepts + inserts source_channel_agent; row
mapper + ScheduleExecution model surface it.
- AgentRef(schedule_executions.source_channel_agent, KEEP) so
cascade_rename re-keys the binding agent (D1a); parity-test locks
consciously updated (_AGENT_ID_COLUMNS + KEEP set).
- Telegram group-config ops carry the flag (columns tuple, row mapper
default-allow on NULL, update_group_config arm, explicit insert);
database.py facade adds get_telegram_chat_link and converts the
group-config passthrough to keyword args (eng M3).
- TelegramGroupConfigResponse/UpdateRequest carry allow_proactive
(Invariant #14; the response model IS the GET field allowlist).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(chat): persist inherited channel context at /task row creation (ent#265 D0)
The ent#224 inheritance wire was severed: run_async_task threaded
_inherited_channel_context's values into execute_task(source_channel=...),
but execute_task writes channel columns ONLY in its no-execution_id
creation branch — and the /task path always pre-creates the row in
create_task_execution_and_activities. Every delegated row carried NULL
channel context at terminal time, so the shipped Slack delegated
report-back was latent dead code on its flagship path.
- Resolve inheritance at the single row-creation point both the async and
sync /task branches route through (the fork to _dispatch_async/_dispatch_sync
happens AFTER creation), and persist all four fields on the row itself
via db.create_task_execution.
- _inherited_channel_context returns a 4-tuple: + source_channel_agent
(D1 Option A — parent's own binding agent, else the parent's agent name;
transitive across A→B→C; all-None when the parent lacks source_channel,
so a channel-less child never carries a dangling agent pointer).
- Provenance guard (security): db.get_execution(parent_id) is a global
lookup; the inherited identity now also resolves a bot token. Agent
caller (x_source_agent, already past the SELF-EXEC-001 spoof guard in
derive_source_and_trigger) must BE the parent's executing agent; human
caller must have access to the parent's agent. Failure → no inheritance,
info log — fail-open to no-context, never to someone else's chat.
- Remove the dead source_channel* threading from run_async_task
(execute_task keeps its channel params — message_router uses them for
direct turns).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(channels): Telegram completion report-back + failure-applier hook (ent#265)
Telegram edition of the ent#224 Slack report-back: a Telegram-triggered
long/delegated task posts its terminal back to the originating chat,
threaded to the triggering message.
channel_completion_report.py rework:
- D10 resolver dispatch map (_CHANNEL_RESOLVERS: slack + telegram);
SUPPORTED_CHANNELS derives from the map keys, so WhatsApp is additive.
- D1 binding-agent resolution for BOTH channels: consent + bot token
evaluate against binding_agent = row.source_channel_agent or
row.agent_name — the bot the user actually addressed delivers (on
Telegram no other bot even CAN deliver the DM). NULL column = legacy
fallback, byte-identical pre-#265 behavior. Also fixes Slack's
delegated case (previously suppressed whenever worker B wasn't bound
to the originating channel); narrowing direction (channel consented to
B but not to originating A → now suppressed) is intentional.
- D2 Telegram consent: known group → is_active AND allow_proactive
(default allow, opt-out mute); known DM chat link →
consent-by-construction (the user cold-started the bot; a block is a
403 the send handles gracefully); unknown destination → suppress+log.
- D5 rendering: pre-escape &/</> before _markdown_to_html (unescaped
`<class 'ValueError'>` trips "can't parse entities" and the strip-HTML
fallback deletes the substring), then post-conversion re-cap to 4096
("message too long" is a 400 the parse-fallback does not catch).
- D6 threading: reply_to_message_id when thread is numeric, DMs too;
allow_sending_without_reply makes a deleted original safe.
- D1c attribution: Telegram has no per-message sender name — when
binding_agent != executing agent the head line names the worker.
Slack keeps username=executing agent (unchanged).
- D9 pin: effect_guard keeps agent_name = the EXECUTING agent
(row.agent_name), never binding_agent — resolve_and_validate_execution
fail-opens on mismatch, silently disarming dedup for exactly the
delegated rows this feature exists for.
- D4: failed send returns False INSIDE the guard (claim completes) —
at-most-once bias, never blind-retry an ambiguous send.
task_execution_service.py (D3, channel-agnostic — fixes Slack AC#2 too):
the failure applier's CAS-won block emitted the #1578 event but never
the channel report — the path agent-reported failure envelopes take.
Sibling spawn added; CANCELLED envelopes report too (uniform with
_write_terminal_and_gate).
routers/telegram.py: PUT group-config passes allow_proactive through;
the allow_proactive arm ONLY is human-only (reject_agent_principal —
an agent-scoped key resolves to the owner and could self-grant consent,
ent#223's own post-ship pitfall).
TelegramChannelPanel.vue: per-group "Completion reports" opt-out
checkbox in the panel's house idiom (updateGroup passthrough).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(channels): ent#265 suite + facade passthrough fix caught by it
New tests/unit/test_265_telegram_completion_report.py (41 tests):
- Wired-mock layer: Telegram delivery/consent/rendering decisions —
delegated group report + threading, DM consent-by-construction,
muted/inactive/unknown-destination suppression, graceful bot-cannot-post,
inline-trigger no-double-post, D5 escape+cap, D1c attribution, D1
binding-agent resolution (telegram + slack) incl. the intentional Slack
consent-narrowing pin (D1b).
- Chokepoint layer: apply_result failure branch spawns the report on
CAS-win only (D3), success-branch pin, CANCELLED-envelope report.
- Real-DB layer (db_harness): D0 row READ-BACK inheritance tests (a
SimpleNamespace mock row cannot see a severed write path — the exact
reason ent#224 shipped broken), both provenance-guard arms, channel-less
parent all-NULL, two-hop transitive root binding agent; REAL effect_guard
replay with source_channel_agent != agent_name (D9/M1 — a binding_agent
passthrough fail-opens resolution and posts twice → red); fan-out
one-report-per-child (G3); live column SELECTs through db/tables.py
metadata; group-config default-allow + kwargs round-trip (eng M3);
router PUT round-trip incl. agent-principal 403 and the surgical-gate
proof that trigger_mode stays agent-callable.
Conscious test_224 edits: non-slack scope test → whatsapp-only (telegram
grew a leg); consent fixture keyed on the binding agent.
The read-back tests immediately caught a REAL gap: database.py's
create_task_execution facade passthrough was missing source_channel_agent
(the ops layer had it) — every /task dispatch would have raised TypeError.
Fixed here; the exact severed-wire class D0 exists to kill.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(requirements): Dashboard type-to-filter — new §9.10, §9.9 seam note updated (ent#261)
Trinity Rule #1: requirements before implementation. §9.10 specifies the /
hotkey filter across Timeline/Grid/List — store-seam predicate, pre-query node
invariant, pill honesty, Esc layering, chassis query-empty overlay, kbd hint,
list-mode composition, and the deliberate timeline owner-filter behavior change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(dashboard): / type-to-filter across timeline, grid, and list (ent#261)
Store seam (network.js): non-persisted filterQuery + setFilterQuery; the
visibleAgents seam splits into ownerFilteredAgents (tag ∘ owner, pre-query)
and the query-aware visibleAgents (slug + display label via agentDisplayName,
#1642). ALL THREE convertAgentsToNodes call sites now read the pre-query
ownerFilteredAgents — incl. the 30s poll that previously rebuilt from the RAW
list (nodes must never be query-filtered: timeline row enrichment would
degrade after Esc).
Dashboard chassis: document / keydown with guards (defaultPrevented/repeat,
chords, IME, editable targets, open modals) + Firefox quick-find preventDefault;
floating filter pill (open OR active — an applied-but-hidden filter is the
dishonest state) with live 'X of Y match', Esc hint, x button; input-scoped
Esc + gated document Esc backstop (tag-dropdown layered dismissal, native
select skip); Enter blurs and keeps the filter; header kbd / toggle button
(mouse/touch parity); ONE chassis query-empty overlay with panes MOUNTED
underneath; true-empty onboarding CTA branches guarded && !filterActive;
timeline :agents switched to the visibleAgents seam (owner filter now applies
to timeline rows too — deliberate, release-noted); query cleared on unmount.
AgentListPanel: N/M badge suppressed while the chassis query is active (two
disagreeing denominators never render simultaneously). ReplayTimeline:
data-agent test hook on row labels (no logic change).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(e2e): dashboard type-to-filter spec — 8 tests across all three modes (ent#261)
Covers: grid live-filter + query-empty with the pane MOUNTED + Esc restore
(@smoke); timeline row hiding via the :agents seam; list query-empty preceding
the onboarding CTA + pill-x clear; editable-target guard + literal '/' inside
the pill; kbd-hint toggle; non-persistence across reload; cross-mode filter
survival + the document-Esc backstop after focus wanders; Enter blur-and-keep.
Spec rules per plan: focus-wait before keyboard.type (nextTick focus race);
regex count assertions (/^1 of \d+ match$/ — X is the claim, Y is
environment). Full run defers to CI via the ui label.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(flows): type-to-filter folded into the three dashboard flows + architecture note (ent#261)
architecture.md: Grid-view block gains the type-to-filter seam sentence; the
List-view block's seam description now covers all three panes. Flow deltas:
grid — filter rides the absence-as-filtering layout path (pane stays mounted
under the query-empty overlay); timeline — FILTER-001 gains a deliberately
NOT-persisted row + a new ':agents = visibleAgents seam' section naming the
owner-filter behavior change and the pre-query node invariant; list — D8 seam
updated to landed state + the chassis-query ∘ panel-filter composition rules
(badge suppression, overlay precedence). feature-flows.md changelog entry (no
standalone flow doc, per plan D11).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(dashboard): review fixes — modal z-order, dual empty-state CTA, Esc scope, structural e2e backstop (ent#261)
Four review findings on the type-to-filter, fixed in place:
- CreateAgentModal was fixed z-10 — UNDER the new z-30 filter pill and z-20
query-empty overlay, so chassis chrome floated above the open modal.
Raised to z-50, the house modal tier (SystemViewEditor / OnboardingWizard).
Safe in all three mounts (wizard renders it v-if-exclusive with its own
z-50 chrome).
- AgentListPanel's filtered-empty card assumed the panel never mounts with a
zero-agent prop — no longer true under a chassis query zero-match, so it
rendered a second contradicting CTA under the query-empty overlay. Gated
on a non-empty prop; the chassis overlay owns query-zero messaging (the
behavior the list-view flow doc already described). e2e test 3 now pins it.
- Document-Esc backstop generalized from the select-only guard to all
editable targets: Esc inside the list panel's search box (or any other
input/textarea/contenteditable) belongs to that control — it must not
clear the chassis filter. Pill input unaffected (own handler stops).
§9.10 wording updated to match.
- e2e test 7's coordinate click (400,250) on the timeline pane could land on
an agent row/toggle on seeded fleets — replaced with a structural
pillInput.blur().
npm run build green; spec collects 8 tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(learnings): overlay-chrome z-order re-pricing + mount-invariant comments (ent#261 review)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(learnings): sibling-route access-hardening class from the ent#264 review
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(flows): sync pass — seam note in skill-assignment, Recent Updates row, dead skills-management link purge (ent#263)
/sync-feature-flows verification pass over the branch diff:
- skill-assignment.md: note that SkillsPanel's package-fact chips now render
via the shared components/skills/ contract seam (extracted in ent#263)
- feature-flows.md: Recent Updates row for ent#263; removed the stale 'Skills
Management UI' index row — skills-management.md was split/archived long ago
(the archive table records it) and the live row pointed at a missing file
- library-page.md: repointed its two skills-management.md links (propagated
from the stale index row) at skill-assignment.md
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(library): harden stripUserinfo against silent-no-op and non-parseable URL shapes; share it with SkillsPanel (ent#263 review)
Adversarial testing of the display-layer credential scrubber proved five
leak shapes. Two families: the regex fallback under-matched what new URL()
rejects (git+ssh:// schemes, protocol-relative //user:token@host, leading
whitespace), and — worse — schemeless/scp user:token@host shapes PARSE as
a WHATWG URL with an opaque hostless path where .username/.password
assignment is a silent no-op, so the credential sailed through the success
lane untouched.
- move stripUserinfo to the shared components/skills/contract.js seam
- trust the parsed lane only when host is present AND the strip verifiably
took (post-assignment username/password empty); otherwise fall through
to a widened textual scrub (scheme charset [A-Za-z][\w+.-]*, protocol-
relative, schemeless colon-user, trim)
- variant analysis: SkillsPanel's library_empty state rendered the RAW
stored URL to any agent accessor — now scrubbed through the same helper
- verified by an executed 26-case suite (23 adversarial leak shapes ALL
PASS + 3 must-survive-unmangled regressions: plain https, scp
git@host:path, path-@ preserved)
- durable class captured in docs/memory/learnings.md
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs: purge the second dead skills-management link; cso --diff report (ent#263 review)
- feature-flows.md Archived Flows table pointed at
archive/skills-management.md, which does not exist (the archive/ dir
never received it) — same dead-link class d23005af purged from the
skills index; row now says 'document not preserved' and names the
dedicated flows it split into
- add the /cso --diff report for the ent#263 review: zero backend
surface delta; one MEDIUM display-layer scrubber finding (proven,
remediated in 820895bf with variant coverage); secrets/enterprise-
disclosure/XSS/supply-chain/CI categories clean
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(channels): channel report-back flow + requirements — pays the ent#224 debt (ent#265)
- requirements/public-access.md: NEW §15.1h "Channel Completion Report-Back
(CHANNEL-REPORT — ent#224 Slack, ent#265 Telegram)" — generic mechanism
(inherited-context-only, D0 row-creation persistence + provenance guard,
binding-agent resolution, chokepoints incl. D3, effect-guard at-most-once,
sanitize-before-truncate), per-channel consent units, the two-DM-consent-
regimes rationale (F6), known v1 limits, the deliberate ungated
proactive-send scope cut. (§15.1f was already taken by WHATSAPP-001 —
plan's placement kept, id shifted to h.)
- feature-flows/channel-completion-report.md: NEW flow doc — entry points,
D0/D3 fixes, chokepoint coverage table with the v1 boundaries
(lease-reaper, bulk sweeps, pull sink, operator-terminate, restart
mid-inline-turn, FAILED→SUCCESS resurrection, fan-out per-child,
pre-migration NULL rows), destination/consent resolution per channel,
D1 identity design with every rejected alternative, failure modes,
Testing. Pays the #1763 flow-doc debt (ent#224 shipped undocumented).
- feature-flows.md: Recent Updates row + Collaboration-table entry.
- telegram-integration.md: ent#265 section (column + toggle + pointer) +
revision row.
- task-completion-events.md: sibling-spawn paragraph (report rides beside
the #1578 emit at the same CAS-won chokepoints incl. the failure applier).
- architecture.md: services-catalog entry for channel_completion_report.py,
schedule_executions DDL line for source_channel_agent, telegram DB-module
line mentions allow_proactive.
- learnings.md: the D0 class — a threaded parameter only one callee branch
consumes is a severed wire; mock-row suites are blind to it AND to its
facade-passthrough sibling (caught pre-merge by the row-read-back tests).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(channels): key the inheritance provenance guard on the principal, not the header (ent#265)
The D0 provenance guard selected its arm from the raw `X-Source-Agent`
header. The SELF-EXEC-001 spoof guard in `derive_source_and_trigger` only
fires when `current_user.agent_name` is set, so for a human caller that
header is unvalidated client input (routers/chat.py documents the identical
trap for the resume-session IDOR). Setting it to the parent execution's own
agent name — a value the row itself discloses — satisfied the agent arm
trivially and skipped the human arm entirely: any user with access to ANY
agent could POST /task with someone else's `parent_execution_id` and have
that task's terminal reported into the parent agent's Telegram DM or Slack
thread, delivered by a bot binding they do not own.
Second, narrower issue in the same guard: the human arm used
`can_user_access_agent`, which admits share recipients. Posting into a
channel chat is a proactive-send capability and every other proactive
surface is owner-gated (`OwnedAgentByName` for group sends) or
per-recipient-consented (#321); a share recipient can already read the
owner's execution ids (`GET /api/executions` is accessor-scoped), so an
accessor arm let them push a report into the owner's chat.
Fix: arms are selected by the authenticated principal —
- agent-scoped key must BE the parent's executing agent,
- human must OWN the parent agent (`can_user_share_agent`) or be admin,
- connector key (consumption-only, ent#46) never inherits,
- no principal at all refuses.
`x_source_agent` is still passed, now for logging only.
Three regression tests, each verified red against the pre-fix guard.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(learnings): header-selected auth-guard arms collapse to the weakest arm (ent#265)
Second instance of a class the codebase already warned about: routers/chat.py
documents the same X-Source-Agent trap for the resume-session IDOR, and the
ent#265 provenance guard reproduced it a few hundred lines away.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(design-system): system of record, builder contract, reference page + raw-color scanner & ratchet baseline
Stands up the frontend design system's written layer per #1430:
- docs/memory/design-system.md — system of record: token taxonomy,
both-theme rules incl. the dark ink ladder, type/spacing/radius scales,
10-primitive catalog with exact token recipes, the data-loading motion
standard (scanline beam + wipe reveal; first load animates, background
refresh is invisible), and 28 UI Construction Principles
- docs/memory/design-system-contract.md — condensed binding contract to
load before any src/frontend change
- docs/memory/design-system-reference.html — approved visual spec (self-
contained; both themes; live motion demo)
- src/frontend/scripts/scan-raw-colors.mjs — raw-color scanner aligned
with check-design-tokens.mjs token families
- src/frontend/raw-color-baseline.json — ratchet seed at dev@5b28999:
753 raw non-gray / 383 hardcoded colors; counts may only shrink
- CLAUDE.md — Rule of Engagement #10 + Memory Files row making the
design system the mandatory reference for frontend work
Refs #1430
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: fleet restart adopts rebuilt base images through the canonical lifecycle path (#1860) (#1912)
* fix: route fleet restart through the canonical lifecycle path so agents adopt rebuilt base images (#1860)
POST /api/ops/fleet/restart stopped/started agents with raw Docker calls,
bypassing start_agent_internal — no config-drift predicate ran and a rebuilt
trinity-agent-base was never adopted on "Restart All" (#1809's cold-start
gate never fired).
- lifecycle.restart_agent_internal(): the canonical stop→cold-start helper
(explicit stop is load-bearing for the #1809 image predicate; future home
of #1817's per-agent start lock)
- restart_fleet routes through it; per-agent recreated/recreate_reason
surfaced via explicit allowlist copy, summary.recreated count
- skips ephemeral ghosts (config predicates aren't ephemeral-gated — a
recreate would destroy a volume-less ghost workspace, ent#69)
- reject_agent_principal beside assert_admin (the endpoint now replaces
containers — Invariant #8 escalation rule, #1816 precedent)
- single-flight Redis SETNX lock ops:fleet_restart (409 on contention,
own-lease refresh, compare-and-delete release, fail-open) — guards the
client-timeout→retry overlap (#799/#1817 wedge class)
- partial-safe fleet_restart audit entry with a per-agent recreate map
(restores the entry dropped in 0ec3a7fc); sync cleanup ordered before the
awaited audit so a shutdown CancelledError can't leave the lock held
- actionable containerless-recovery errors (#1559), context-stats cache
invalidation, 16 mocked unit tests, flow/architecture docs, learnings,
CSO diff report (PASS)
Fixes #1860
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: never flow a raw exception message into fleet-restart results (CodeQL py/stack-trace-exposure)
Per-agent failure rows now carry HTTPException .detail (platform-authored)
or the exception class name only; the full message + traceback go to the
backend log (exc_info). The #1559 containerless recovery hint is preserved.
Tests strengthened into leak regression guards (raw message asserted absent);
flow-doc line refs re-verified.
Refs #1860
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(auth): bulk portal-session revocation primitive + operator client panel (#1902)
* feat(auth): bulk portal-session revocation primitive + operator client panel
The OSS half of an operator kill switch for signed-in portal clients. A client
who signs in through sharing holds a 12-hour portal session with no way to end
it short of a backend restart, which logs out everyone.
`revoke_portal_sessions_for_email(email)` is the edition-agnostic primitive:
OSS owns the mint, the decode and this bulk revoke; the entitled module that
mints portal sessions decides WHEN to call it (the same split as the delegated
mint).
Mechanism is a per-email CUTOFF, not a jti list. `jti` is random per token and
nothing indexes email -> issued jtis, so answering "which tokens does this
address hold?" would need a write-side index maintained at every mint. One
timestamp per email is O(1) to write and read, self-expiring at the max session
lifetime (same bounded-growth property as the #187 blacklist, no sweep), and —
the property that matters — covers every mint path by construction, since they
all go through `create_portal_session_token`. That is the failure mode a
hand-maintained index has and this does not.
`create_portal_session_token` now stamps an explicit `iat` so tokens can be
dated against the cutoff; set there rather than in `create_access_token` so no
other token type's claim set changes.
`decode_portal_session` rejects `iat <= cutoff`. Rounding toward revoking is
deliberate: for a kill switch, a token minted in the same second as the revoke
must die. A token with NO `iat` is treated as revoked — fail closed. Only
sessions minted before this shipped lack one (all expired within
PORTAL_SESSION_EXPIRE_HOURS of the upgrade), and only for an email an operator
actively revoked; letting an undatable token survive an explicit kill switch is
the worse failure.
The Redis read stays fail-OPEN, matching #187 and the platform posture. That is
why revocation is not the whole feature: the durable half lives in the entitled
module and keeps working with Redis down. `revoke_...` returns a bool so a
caller can report honestly instead of claiming a success the operator would act
on.
Frontend: `PortalClientsPanel.vue` on the Sharing tab — per-client log out /
block / unblock with current state. Entitlement-gated (`client_portal`), so an
OSS build renders nothing. Block is hidden for non-admins rather than offered
and 403'd, and the panel reports a failed revoke as a failure rather than
"signed out". It deliberately shows no live-session count: portal sessions are
stateless JWTs with no server-side store, so any number would be a guess.
Related to trinity-enterprise#281
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(ui): use the real status-danger design token in the portal clients panel
`status-error-*` does not exist — the token family is
success/warning/danger/info/urgent (tailwind.config.js), and
`npm run check:tokens` caught every reference. No visual change intended:
danger is the red alias the invented name was reaching for.
Related to trinity-enterprise#281
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(skills): library lifecycle automation — auto-sync, fleet re-inject, removal-on-unassign (abilityai/trinity-enterprise#236) (#1883)
* feat(skills): library lifecycle automation — auto-sync, fleet re-inject, removal-on-unassign
Closes the three "Not Built" gaps in the skills lifecycle (requirements §21.1/§21.4).
All three default OFF/no-op, so a zero-config install is unchanged.
Removal-on-unassign
- `compute_removal` is `compute_prune` against an empty new manifest, so path
confinement, `..` rejection, and the cap live in one place and cannot drift.
- `remove_skills` takes the SAME per-agent lock as injection (both mutate
~/.claude/skills and read-modify-write CLAUDE.md). Only manifest paths are
deleted, so agent-authored files and runtime artifacts survive; a directory the
removal empties is reaped via os.rmdir, which refuses a non-empty dir.
- Wired into BOTH the single DELETE and the bulk PUT — the bulk PUT is the primary
UI/MCP path and drops skills far more often.
- The DB unassign is authoritative and always succeeds; a stopped agent, busy lock,
or dead transport degrades to a named `removal_deferred:*`.
- A partially-failed removal keeps the meta AND the .gitignore line: dropping the
meta strands survivors as unmanaged orphans, and dropping the ignore line lets
the 15-min auto-sync commit leftover injected files (#1595/#1596 class).
Start-path reconciliation
- The assignment row is gone by the time a stopped agent starts, so removal is
reconciled, not replayed: the agent's platform-managed skill dirs are diffed
against the assignment set. No tombstone table, no migration, and every removal
route converges. Runs after injection and also at zero assigned skills — that is
exactly the "unassigned the last skill" case.
- Blast-radius guard: >10 removals for one agent refuses wholesale and alarms. A
wiped agent_skills table is indistinguishable from a mass-unassign, and keeping
files is the recoverable direction (#1638/#1644).
Scheduled auto-sync + fleet re-inject
- New leader-locked backend service (skills:sync:leader). Backend-hosted because the
sweep must reach agent containers and the scheduler is platform-network-only.
- Sweeps only when the library commit actually changed; running non-ghost agents;
force=False so the ent#183 tree-SHA skip makes unchanged skills free; bounded
concurrency; inject-lock contention is skip-and-report.
- Honest aggregate report in Settings + operator alarm only when an agent failed.
- Config re-read each cycle, so an interval change needs no restart.
Durable sync status
- `_last_sync`/`_last_commit_sha` were per-process; under `--workers 2` the worker
answering /status was usually not the one that synced, so the panel showed a stale
timestamp and could never show an error at all. Now mirrored to system_settings on
both the success and failure branches, and the commit-changed comparison reads the
durable row (the in-memory field would make every restart look like a change).
Hardening found by /review and /cso on this branch
- The persisted sync error is PAT-scrubbed: sync_library's outer handler passed a raw
str(e), and the authenticated remote URL is an argument in the subprocess command
list, so an OSError could carry a token into system_settings and the admin panel.
- Cross-worker lock around the shared clone: scheduled + manual sync both run
`git fetch` + `git reset --hard` on /data/skills-library. Contention returns 409 and
writes no status row, so a contended click cannot paint "Last sync failed".
- remove_skills re-reads assignments inside the lock and refuses a still-assigned
skill, closing the bulk-PUT check-then-act race (#1445 pattern).
- PUT /api/settings/skills-library is human-only (reject_agent_principal): it is the
on-switch for an unattended fleet-wide write of SKILL.md files, which Claude
executes as instructions. assert_admin answers "what role", never "is this a
human" — third occurrence of the trinity-ops-agent#232 class. The remaining half of
that chain (skills_library_url writable by an agent key via the generic settings
PUT) is pre-existing and filed as abilityai/trinity-enterprise#293.
Dedicated range-validated GET/PUT /api/settings/skills-library; the three keys are
blocked on the unvalidated generic PUT. 58 new tests.
Fixes abilityai/trinity-enterprise#236
* fix(skills): wire ent#236 tuning vars into both compose files + .env.example
`SKILLS_RECONCILE_MAX_REMOVALS` and `SKILLS_FLEET_INJECT_CONCURRENCY` were read
via os.getenv() but never reached the container — the #1056 / trinity-enterprise#31
packaging class. The reconcile-refusal alarm names the first var in its own
remediation text ("raise SKILLS_RECONCILE_MAX_REMOVALS"), so on a deployed stack
the operator was told to turn a lever that does not exist, leaving a legitimate
mass-unassign permanently blocked at the default cap of 10.
Found by /validate-pr on PR #1883.
* fix(ui): agent tab panel polish — card borders, section spacing, and an honest Sync-now affordance
Three cosmetic fixes plus one UX gap on the agent detail tabs.
- SkillsPanel had no card wrapper, so its content sat flush against the tab
bar unlike every sibling tab. Wrapped in the same card the other panels use.
- FoldersPanel and InfoPanel each had a `v-else` content wrapper with no
spacing class, so the root `space-y-6` never reached the cards inside and
they rendered touching. Same bug in both files.
- SkillsPanel: saving assignments writes the DB rows, but the files only reach
the container on a sync or the next agent start. That was stated in muted
text above the button, which is easy to miss — an operator can save, message
the agent, and be told the skill doesn't exist, because it isn't there yet.
"Sync now" now goes prominent while that gap is open, and only while the
agent is running (shouting at a disabled control helps nobody). A failed or
409-busy sync keeps it lit, since the gap is still open.
* fix(ui): inset the Settings and Skills tab content like every other tab
8 of the agent-detail tab wrappers use p-6 (overview, info, brain, dashboard,
schedules, playbooks, git, folders). Settings and Skills had none, so their
cards ran flush into the enclosing panel's left and right edges instead of
sitting inset like the rest.
* fix(skills): re-clone a library path that is not a repository, and make the panel's failures actionable
Three fixes found while testing this PR on a live instance. All pre-existing on
dev, but each is sharpened by putting sync on an unattended timer.
1. sync_library() chose pull-vs-clone on `library_path.exists()` — directory
existence, not repo-ness. A path holding no `.git` (clone interrupted by a
full disk, a stray mkdir, a restored backup) failed `git pull` with "not a
git repository" on every attempt, with nothing able to re-clone: permanent,
and recoverable only by shell access. Now tests for `.git` and re-clones.
Detecting it is only half a fix, since `git clone` refuses a non-empty
destination — so a non-repo directory is moved aside first. Renamed, never
deleted: the platform owns the path exclusively so removal would probably be
safe, but "probably safe" is not the standard for an unattended timer
deleting a directory derived from an operator-supplied setting (#1638/#1644).
One quarantine is kept, so a recurring fault cannot grow without bound.
2. syncSkillsLibrary() saves settings first (setting showSuccess), then syncs;
the catch set `error` without clearing it, so a failed sync showed "Settings
saved successfully!" and "Clone failed" together. Both true, which is
precisely what makes the pair untrustworthy.
3. The panel never said a private library needs a GitHub PAT — it is in two
internal docs only, and what an operator hits is raw git ("could not read
Username"), which names no remedy and does not hint that the PAT field is
one section above. Added that line. The error-string mapping is deliberately
NOT done: matching git stderr is brittle across versions and locales, and
the raw error is at least surfaced rather than swallowed.
Tests: 5 new (4 fail without the fix). Also fixes 7 existing tests that created
the library path without `.git` — under the corrected predicate they took the
clone path and reached for the network; the suite drops 5.01s -> 0.62s.
* fix: ownership-checked fleet-restart lease with loss detection + acquire inside try/finally (#1919) (#1928)
The per-iteration refresh was a bare EXPIRE gated on a local flag — after a
TTL lapse it extended a concurrent caller's lease while both loops ran. The
refresh is now a pre-action ownership gate (GET-compare via the shared
redis_breaker_util.lock_token_matches): a foreign token stops the run with
honest partial accounting (summary/audit gain processed + stopped_early), an
absent token is re-acquired via SETNX so an unraced run completes instead of
aborting, EXPIRE→0 routes to the absent path, and refresh Redis errors stay
fail-open with one throttled warning per run. list_all_agents_fast() moved
inside the try/finally so nothing can ever leak the lock between acquire and
release; an abnormal exit audits as stopped_early="error" + exception class
name only. TTL 900→2100, sized above the slowest single agent (skill
injection alone is bounded at 1800s) so a mid-agent lapse is no longer
arithmetically guaranteed. Release stays compare-and-delete, attempted even
after detected loss (foreign-safe by construction).
+11 unit tests (27 total in the file); live-validated on the local stack
(foreign takeover mid-run, absent re-acquire to completion, 409 concurrency,
TTL 2098 observed, release verified). Sibling hand-rolled lock sites and the
system_seed_service unconditional release → #1920.
Fixes #1919
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(templates): contain local: template ids and stop deriving credential paths from name: (#1900) (#1935)
* fix(templates): contain local: template id resolution on the read path (#1900)
`GET /api/templates/{template_id:path}` handed `local:<name>` straight to
`get_local_template`, which joined `<name>` onto the templates root with no
validation. The `:path` converter permits `/`, so `local:../<x>`,
`local:/<abs>/<x>` and a root-escaping symlink each read
`<escaped-dir>/template.yaml` and echoed its contents in an authenticated 200.
Reachable by any authenticated principal of any role — including an
agent-scoped MCP key, so a prompt-injected agent qualifies. In a container the
reachable set includes `/data/deployed-templates/<victim>`, where every user's
uploaded template archive lands: a cross-tenant read.
Two corrections to the issue's framing, both verified here:
* it is NOT arbitrary file read — the filename is fixed (`template.yaml`), it
must parse as a YAML mapping, and only a fixed key set is echoed. But those
keys' VALUES are arbitrary YAML subtrees, not just strings.
* `local:..` alone is not an existence oracle: a directory with no
`template.yaml` returns the same 404 as an unknown id.
Fix: `contained_template_dir(name, root)` — the two-step barrier the CREATE
path has had since #950 (`crud._safe_local_template_path`), brought to the read
path. A name allowlist runs BEFORE any path math (this is also what CodeQL
recognises as a `py/path-injection` barrier; resolve-only was flagged
high-severity twice on this codebase), then `resolve()` on BOTH sides plus
`is_relative_to`. `str.startswith` is not equivalent — it passes the sibling
escape `<root>-evil`.
An escaping id returns `None`, so the router's 404 stays byte-identical to an
unknown template: no error code, no path, no root name. A distinct error would
be a NEW enumeration oracle, which is what #1759's single-sentence 404 exists
to close. Rejections log at DEBUG, sanitized — the endpoint has no rate limit,
so a per-rejection WARNING would be an authenticated log-flood primitive.
The helper is public: the remote-template-registry work (trinity-enterprise#14)
edits this same resolver family in this same module and should import it rather
than copy it.
Tests: every rejection test PLANTS a real `template.yaml` at the escaped
location, because unpatched code returns `None` for any id whose target simply
does not exist — a rejection test with nothing planted is green before and
after and proves nothing. Each is labelled REPRO (verified red pre-fix) or
HYGIENE (cannot be made red; contract only). The router guard lives in
`tests/unit/` because no gating CI job collects `tests/test_templates.py`.
`test_1900_containment_survives_a_symlinked_root` is the landmine guard for
resolving both sides: a half-resolved variant passes all 130 pre-existing tests
and only that test catches it.
Refs #1900
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(templates): stage .mcp.json from the validated template dir, not template name: (#1900)
The second traversal sink, found by this issue's own AC #4 audit ("audit the
by-name create path for the same join"). It is NOT the create path's `local:`
id resolution — that has been contained since #950/#1759 via
`crud._safe_local_template_path`, applied at both seams #1759 named. It is the
create path's CREDENTIAL STAGING, which threw that validated path away and
re-derived a directory from scratch:
template_name = template_data.get("name", "") # untrusted
mcp_template_path = templates_dir / template_name / ".mcp.json"
`name:` comes from an uploaded template.yaml, so any `creator` reaches it via
`deploy_local_agent`. `name: ../../data/deployed-templates/<victim>` read
another tenant's `.mcp.json` — a credential-bearing file type under
Invariant #12 — into the attacker's OWN agent, where they read it at leisure.
A victim who hardcoded a token rather than a `${VAR}` placeholder leaks it.
Assessed on its own axes, NOT inherited from the read sink: different trigger
(`creator` role + an upload + an agent create, vs a bare authenticated GET) and
a higher impact ceiling (credential values, not template metadata). Also P2, for
different reasons.
The derivation was also simply wrong. `name:` is not a directory name — 5
shipped templates declare a display string there ("Test Echo Agent"), so
`local:test-echo` resolved to `<curated>/Test Echo Agent/.mcp.json`, which does
not exist. That kills the "validate the name" framing: the value should not
resolve paths at all.
Fix is root-cause, not another guard: `_stage_config_files` already calls
`_safe_local_template_path` itself for the `/template` bind decision, so the
validated directory is available in the same function. Extract the two-root
ladder as `_resolve_local_template_dir` and pass its result as
`template_base_path`. The untrusted join is gone from the live path, and the
#1759 "seams must agree" property becomes structural across all THREE seams
(resolver, bind decision, credential stager) instead of two.
Deliberately NOT threaded through `_TemplateResolution` or the return tuple:
`_resolve_local_template` returns a 2-tuple that three existing tests depend
on, two as monkeypatched `lambda config: ({}, None)` doubles — widening it
breaks the test doubles, not just the callers. Its signature, its return arity,
`_safe_local_template_path`, `_LOCAL_TEMPLATE_ROOTS`, and everything inside the
CodeQL-sensitive `if template_yaml.exists():` block are untouched (#1793 had to
revert exactly that reshape).
The residual `template_base_path is None` arm is kept and made fail-closed: it
is a public function with a `template_base_path=None` default, so a future
caller can still reach it. It now contains through the same barrier as the id,
which also absorbs a non-string `name:` — `Path(root) / 123` raised TypeError,
i.e. an uncaught HTTP 500 during agent creation (the ent#128 bug class, one
seam over).
One disclosed behaviour change: a deploy-local template that BOTH declares
`credentials.mcp_servers` AND ships a `.mcp.json` now gets `${VAR}`
substitution, where the old curated-root lookup always missed. Verified no
collision with `deploy._prepopulate_workspace_from_template`, which writes the
archive's raw copy into the workspace volume: `startup.sh` copies
`/generated-creds/.mcp.json` unconditionally (gated only on the directory
existing) and AFTER the template-copy block (gated on `.trinity-initialized`),
so the substituted file deterministically wins — which is the intended
behaviour, the raw copy still carrying unsubstituted placeholders. Not one of
the 26 shipped curated templates contains a `.mcp.json`, so the curated rows
are provably unchanged.
The crud seam tests are mandatory, not decorative: every service-level test
calls `generate_credential_files` directly, so an "extracted but never wired"
mistake leaves all of them green while deploy-local resolution silently
regresses into the fallback arm.
Refs #1900
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: record the #1900 containment contracts for template id + credential staging
Rule #1 (requirements before implementation) — `core-agent.md`:
* §4.1 gains the **read-path resolution contract** as a sibling to the existing
create-time contract: `GET /api/templates/{id}` resolves `local:<name>`
through the same two-step barrier the create path has had since #950, and a
failing name returns a 404 byte-identical to an unknown template (the #1759
non-disclosure rule). Records the deliberate, known asymmetry that
`get_local_templates()` still enumerates by `iterdir()` and so could LIST a
root-escaping symlink that detail and create both refuse — the listing is the
outlier, and planting one needs local filesystem write access, not a request.
* §4.3 records that the `credentials.mcp_servers` template lookup now resolves
from the validated path rather than the template's own untrusted `name:`
field, including the one disclosed behaviour change (deploy-local templates
now get `${VAR}` substitution) and why the substituted file wins over the
archive's raw copy.
`architecture.md` gets one clause on the `templates.py` router catalog entry
(the catalog rule caps entries at 2 lines, and no Cross-Cutting Subsystems
block is warranted). A bug fix would normally be commit-message-only under the
tiered-docs rule; the exception is that this ships a public, importable
containment primitive in the exact module and resolver family the remote
template registry (trinity-enterprise#14) will edit, and one catalog clause is
the cheapest way that author finds it instead of copying the flaw.
Not updated, deliberately: no feature-flow doc (no new vertical slice), no user
docs (no user-visible change for honest callers), no schema/migration (no DB
change, so the dual-track SQLite/Alembic rule does not apply).
Refs #1900
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(feature-flows): sync template-processing + local-agent-deploy for #1900
`/sync-feature-flows` was NOT a no-op here — three concrete staleness points in
`template-processing.md`, which owns the `local:` resolution surface:
* The inlined two-root ladder is now `_resolve_local_template_dir`; the code
block showed the pre-extraction form.
* "**Two** seams read `_LOCAL_TEMPLATE_ROOTS` and must stay in agreement" was
the #1759 claim and is now wrong in the direction that matters: there was
always a third seam (the credential-file stager) which did NOT agree — it
re-derived the directory from the template's untrusted `name:`. Corrected to
three, with the extraction as the structural guarantee.
* `generate_credential_files` was cited by stale line range (`:228-299`) and
documented none of where the `.mcp.json` template is actually located.
Replaced the fragile line-range citation with a symbol reference and added
the provenance, the residual fail-closed arm, and the disclosed deploy-local
substitution delta.
Also documents the read-path containment (`get_local_template` →
`contained_template_dir`) beside the existing #1513 catalog-curation note,
including the deliberate list-vs-detail asymmetry for a planted symlink.
`local-agent-deploy.md` gets one line at the credential-merge step: a
deploy-local template's `.mcp.json` now resolves from the deploy-local
directory, so a hostile `name:` cannot read another tenant's file and
`${VAR}` substitution finally applies to that template.
`credential-injection.md` was checked and NOT touched — its `.mcp.json`
references are unrelated (credential inject/export/import), and the template
lookup live…
This was referenced Aug 24, 2026
dolho
added a commit
that referenced
this pull request
Aug 25, 2026
…457 AC#3) "Dispatch → monitor → report back is a contract, not a habit" — the issue's words. The machinery has existed since ent#224/#265 for Slack and Telegram: a terminal chokepoint (#1578), a destination on the row (ent#117), a resolver per channel, an effect guard for at-most-once. The Workspace was excluded by ONE missing field. #2157 stamped portal executions with the SURFACE (`source_channel = "portal"`) and never a destination, so every portal terminal died at `report_completion`'s `if not source_channel_chat_id` gate. This gives the row its session id at both creation sites (#2157 FR-7's rule: both, or which path made the row decides whether the promise holds) and adds a portal resolver. Delegated work then inherits the destination through the EXISTING ent#265 chain — no new inheritance, no new transport, no new table. Two rules carried over deliberately, because both are the ways this goes wrong: * **No double-post.** A Workspace turn is synchronous — `portal_chat` persists the reply itself — so `public` joins INLINE_CHANNEL_TRIGGERS. Without it every chat message would gain a duplicate "done". Public links and x402 share that trigger and are unaffected: they carry no chat id, so they never reach the check. * **The recipient comes from the session row, not the stamp.** The stamp is a string that rode an inheritance chain; the session row is the platform's own record of whose chat this is. A delegated child may execute as a different agent (A asks B) — the message is filed under A, whose chat it is, and names B in the body. Consent is by construction, as for a Telegram DM: the session belongs to one client and the report goes into their own conversation, so there is no third party and no flag to consult. Delivery is a persisted assistant message read through the history the client already polls, which is why AC #7's "degrades to poll" holds here with no new transport. **This supersedes half of a #2157 invariant, deliberately.** `test_portal_source_channel_is_not_a_messaging_channel` asserted the portal must miss BOTH the voice service's supported set and `_CHANNEL_RESOLVERS`. The voice half is permanent — there is no outbound audio leg. The completion half was true only because the row carried no destination, which is precisely what ent#457 (operator ruling 2026-08-22, committed to the release cut) changes. The guard is narrowed to what remains true and now also pins the no-double-post rule; the reasoning is in the test, not only in this message. Verified live: a delegated portal execution posts "**Finished** — Reconciled 42 invoices…" into the client's thread; a second call delivers nothing (the effect guard holds, still one message); the turn's own `public` execution reports nothing at all. Scope: this is AC #3 only. The execution card and pipeline view (AC #1/#2) are gated on AC #5's design pass, which is published separately for review — no card code until it is approved. Related to Abilityai/trinity-enterprise#457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dolho
added a commit
that referenced
this pull request
Aug 25, 2026
…457 AC#3) "Dispatch → monitor → report back is a contract, not a habit" — the issue's words. The machinery has existed since ent#224/#265 for Slack and Telegram: a terminal chokepoint (#1578), a destination on the row (ent#117), a resolver per channel, an effect guard for at-most-once. The Workspace was excluded by ONE missing field. #2157 stamped portal executions with the SURFACE (`source_channel = "portal"`) and never a destination, so every portal terminal died at `report_completion`'s `if not source_channel_chat_id` gate. This gives the row its session id at both creation sites (#2157 FR-7's rule: both, or which path made the row decides whether the promise holds) and adds a portal resolver. Delegated work then inherits the destination through the EXISTING ent#265 chain — no new inheritance, no new transport, no new table. Two rules carried over deliberately, because both are the ways this goes wrong: * **No double-post.** A Workspace turn is synchronous — `portal_chat` persists the reply itself — so `public` joins INLINE_CHANNEL_TRIGGERS. Without it every chat message would gain a duplicate "done". Public links and x402 share that trigger and are unaffected: they carry no chat id, so they never reach the check. * **The recipient comes from the session row, not the stamp.** The stamp is a string that rode an inheritance chain; the session row is the platform's own record of whose chat this is. A delegated child may execute as a different agent (A asks B) — the message is filed under A, whose chat it is, and names B in the body. Consent is by construction, as for a Telegram DM: the session belongs to one client and the report goes into their own conversation, so there is no third party and no flag to consult. Delivery is a persisted assistant message read through the history the client already polls, which is why AC #7's "degrades to poll" holds here with no new transport. **This supersedes half of a #2157 invariant, deliberately.** `test_portal_source_channel_is_not_a_messaging_channel` asserted the portal must miss BOTH the voice service's supported set and `_CHANNEL_RESOLVERS`. The voice half is permanent — there is no outbound audio leg. The completion half was true only because the row carried no destination, which is precisely what ent#457 (operator ruling 2026-08-22, committed to the release cut) changes. The guard is narrowed to what remains true and now also pins the no-double-post rule; the reasoning is in the test, not only in this message. Verified live: a delegated portal execution posts "**Finished** — Reconciled 42 invoices…" into the client's thread; a second call delivers nothing (the effect guard holds, still one message); the turn's own `public` execution reports nothing at all. Scope: this is AC #3 only. The execution card and pipeline view (AC #1/#2) are gated on AC #5's design pass, which is published separately for review — no card code until it is approved. Related to Abilityai/trinity-enterprise#457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dolho
added a commit
that referenced
this pull request
Aug 25, 2026
…vent loop (ent#457) **A failure terminal wrote raw secrets into an external client's thread.** `_summarize` (Slack/Telegram) runs `sanitize_text` over a 2x window before truncating, and its own comment says why. `_portal_body` did a bare strip and slice — no sanitizer anywhere on its path. The failure call sites pass raw text (`_write_terminal_and_gate` passes `error`; `apply_result`'s failure branch passes `envelope.error` — only the success branch passes something already sanitized), so a traceback carrying `ANTHROPIC_API_KEY=sk-ant-…` or a `https://x:ghp_…@github.com/…` clone URL landed verbatim in `enterprise_portal_messages` for a CLIENT, permanently, and was replayed into the agent's own history context on the next cold turn. It also contradicted #2320's rule that raw failure text is operator-only. The rule is now `_sanitized_detail`, shared by both — extracted rather than copied, because a second implementation of a redaction rule is a second place to forget it, which is how this happened once. Order stays load-bearing: sanitise over the 2x window BEFORE truncating, or a slice can cut a secret so the pattern no longer matches and the tail survives. **A raise in `deliver()` released the effect-guard claim, so a re-delivery double-posted.** `effect_guard` calls `fail()` on an exception, which frees the claim; `add_portal_message` commits before `touch_portal_session` runs, so a transient "database is locked" on the second write is exactly that shape, and a #1083 callback or the lease-reaper then appends a second identical report. Both existing resolvers catch and return False for this reason — the file documents it as "D4: failed send claims completed — the at-most-once bias". The portal leg was the one that opted out. **The writes ran on the event loop.** Sync SQLAlchemy, while both existing resolvers do their I/O with `await`. A batch of delegated terminals at 03:30, while `db_backup_service` holds SQLite's read lock, blocks the single backend loop for up to the 30s busy timeout per task — stalling every in-flight chat, heartbeat and WS fan-out on that worker. Now `asyncio.to_thread`. The session READ is left on the loop deliberately: it is one indexed SELECT, and moving it would make the resolver async and change the dispatch contract for all three channels. **"The Workspace polls its threads" was false**, and the docs said it twice. `PortalConversation.vue` loads history only on mount and on a prop change, `refreshThreads()` is documented as event-driven, and the only interval in the Workspace is the 20s asks poll on a different surface. The report is durable but not immediately visible; the docstring and architecture.md now say so, and stop claiming AC #7's "degrades to poll" holds by construction. An idle history poll is the follow-up. NOT fixed, and recorded rather than bodged: a report landing mid-turn can be returned AS that turn's answer, because the client detects a reply by an assistant-row count delta and this is now a second writer of those rows. The honest fix needs a per-row discriminator `enterprise_portal_messages` does not carry (no `execution_id`), i.e. a dual-track migration; overloading `role` was rejected because two server-side readers and the client's rendering branch on it. Written into the docstring and architecture.md where the next reader will hit it. Related to Abilityai/trinity-enterprise#457
dolho
added a commit
that referenced
this pull request
Aug 27, 2026
…nd converge the migration line (ent#457) Review finding 4, the half that was left. The claim was downgraded in the `_resolve_portal` docstring and in `architecture.md`, and left standing in the two places the reviewer rightly called the ones that matter most: - `channel-completion-report.md` — "the Workspace polls its threads, so 'degrades to poll' holds by construction". It is what the next person builds on. - `test_2157_portal_narration.py` — "the assistant message the client reads on the next poll … AC #7's 'degrades to poll' is the only mode it has". A sentence in a test docstring reads as VERIFIED rather than as claimed, and a green suite beside a false one is what makes the gap permanent. Neither is true: `PortalConversation.vue` loads history on mount and on a prop change, `stores/clientPortal.js` says outright that `refreshThreads()` is event-driven rather than periodic, and the Workspace's only interval is the 20s asks poll, which fetches asks and nothing else. Both now say what actually happens — the row is durable and arrives at the client's next reload or thread switch — and neither claims the delivery the follow-up poll would provide. ## The migration fork, now decidable 0047 (#2384, ent#366) and this 0048 both declared `down_revision = "0046_report_audience"`. That is a fork, and a fork is worse than it looks: `alembic upgrade head` is singular and resolves its target BEFORE applying anything, so two heads apply ZERO revisions — not merely the offending one, but everything merged since the fork — and PostgreSQL boots on a schema that has silently stopped advancing. `check_alembic_heads` cannot catch it on either PR alone, which is why both were green. #2384 merged first, so the order is no longer hypothetical: 0047 is on `dev` and may already be applied, while this revision is applied nowhere. Re-parenting the unapplied one onto the applied one converges the line and needs no merge revision — `alembic merge` is for the case where BOTH forked revisions may exist in some database, and taking it here would leave a permanent extra node for nothing. Verified: `check_alembic_heads` goes from FAIL (2 heads, forking at 0046) to PASS (1 head). Merges `origin/dev`, which also brings 0047 and its SQLite twin; both migration functions are registered and both ledger entries kept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
13 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>
dolho
added a commit
that referenced
this pull request
Sep 8, 2026
Reported from hands-on testing: pressing PDF offered to export the entire page. Correct report — the print stylesheet only STYLED the document and never hid anything else, so `window.print()` printed the nav bar, the tabs, the on-screen panel AND the print copy. That is not "one clean column" by any reading (AC #4), and it is the first thing anyone pressing the button hits. Two halves, both required: * a print rule that hides every `body` child except `.canvas-print-root`; * the print copy TELEPORTED to <body>, so it is a body child and the rule can spare it. Nested inside the app the rule would hide its ancestor and print nothing at all — worse than the bug. `body > *` rather than a class on the app root: it needs no knowledge of how the app is mounted and works identically on the standalone shared page, which keeps AC #7's "identical from every surface" true rather than approximately true. The copy is rendered only while printing (`v-if="printing"` + a `nextTick` flush before `print()`, since printing a not-yet-rendered teleport yields a blank sheet), so the DOM carries no permanent hidden duplicate. `canvasPrintIsolation.spec.js` pins all three structural facts. Nothing automated can inspect a print preview, which is exactly why the bug shipped — so the guard asserts the mechanism instead: the hiding rule exists, the root is teleported to body, and the document mounts before print() is called. Mutation-tested: removing the hiding rule fails it. Also fixes a design-system violation the ratchet caught in the same file: `SharedCanvas` gated its skeleton on a bare `v-if="loading"`. Now `viewState()` — loading means "no data yet", never "a fetch is in flight" (#1927, design-system p13-p15). The page fetches once today, so this is the rule holding rather than a bug fixed; it stays correct if a refresh is added. Baselining my own new violation was the alternative and would have been the wrong one. Related to Abilityai/trinity-enterprise#554 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ
vybe
pushed a commit
that referenced
this pull request
Sep 11, 2026
…tion (#2638) (#2645) * fix(subscriptions): a rate-limited turn completes on another subscription (#2638) A Workspace message to an agent whose Claude subscription was rate-limited failed outright, the message was lost to a FAILED execution, and the client was told the failure was not retryable. Every SUB-003 mechanism was working — switch on the first 429 (#441), re-issue once (#792), rank by headroom (#2409). Four gaps between them left the turn failing anyway. **1+2 — the 2h skip-list is now overridable, per candidate, on evidence.** `list_viable_alternative_subscriptions` drops any subscription with ANY failure event in a flat 2h window, so on a two-subscription install one stale event means "no viable alternative" while an alternative the provider would serve sits there — which was #2320's own evidence. `recovery_verdict` readmits on positive evidence ONLY: a FRESH reading saying the provider is not refusing (`serving_now` — #447's rule that a probe beats an inference from past failures), or a blocked window's own reset instant ELAPSED **and predating the failure** (`window_reset`). That ordering is load-bearing: without it, a subscription that 429'd a minute AFTER its rollover would be readmitted on a reset it had already consumed. Instants come from an AGED snapshot on the asymmetry this codebase already states — a utilisation number decays, an instant does not. Three properties keep #444's ping-pong closed: absence of evidence readmits nothing (that loop was caused by FORGETTING a failure); a fresh refusal does not fall through to the weaker instant arm; and the fail-open ranking branch readmits nobody by construction, because it is precisely where the evidence could not be read. One interaction found by the end-to-end test rather than by reading: a `window_reset` candidate's reading carries a `blocked` flag describing the window that just rolled over, and `rank_subscriptions` drops a blocked candidate as refused — so the readmission was inert in exactly the case it exists for. Those readings are handed to the ranker as UNKNOWN. **3 — the switch can happen BEFORE the first dispatch.** SUB-003 was purely reactive, so the first message after a wall always burned a failed attempt — on the Workspace, a person watching their message fail. `ensure_serviceable_subscription` moves the agent when its subscription is already known-refused (a fresh provider refusal, or a 429 in the platform's own 2h window — the 429-only DISPLAY predicate, deliberately, since an auth failure is a credential problem another subscription may share). It never raises, records NO failure event (nothing failed, and a synthetic one would poison the skip-list it feeds), dispatches anyway when there is no alternative, and performs the SAME `_perform_auto_switch` so the activity, notification and hot-reload happen whichever path fired. **1 last resort — the platform API key.** When the switcher declines, `fallback_to_api_key` clears the assignment, sets `use_platform_api_key` and restarts rather than hot-reloading (the reload endpoint pushes an OAuth token; this needs the opposite change, which lifecycle already derives from DB state). Setting `subscription_api_key_fallback`, default ON, fail-OPEN on a read error, with `key_configured` on the read — a toggle reading only "on" with no key stored describes a remedy that cannot run. **4 — the client is told what changed.** `TaskExecutionResult. subscription_switch` carries the switch; the portal answers 503 `auth_switched` retryable=True naming the new subscription instead of #2320's `retryable=False`, which was true only while nothing changed underneath. With no switch it still refuses, but names the earliest reset the sampler already caches instead of "try again later". Gap 5 (pull-dispatched terminals never trigger SUB-003) is filed as #2643 per AC #7 — a different blast radius, and inert until an agent is piloted. Tests: `tests/unit/test_2638_subscription_switch_on_turn.py` (37) — the verdict as a pure table, readmission through the real selector, the pre-dispatch contracts, the fallback's setting semantics, and three end-to-end turns through the real `execute_task` + real switcher: completes on a never-failed alternative, completes on a READMITTED one, and the honest negative. `test_2409` (87), `test_447`, `test_792`, `test_2352` and the ping-pong suite are unchanged in substance and green. Related to #2638 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP * fix(2638): don't hand CodeQL a permanent false positive on the fallback log `logger.warning(..., API_KEY_FALLBACK_SETTING, ...)` trips `py/clear-text-logging-sensitive-data`: the rule flags any `*_KEY`-shaped name reaching a log call, and this one is a hard-coded settings key NAME, not a secret. Removed the interpolation rather than dismissing the alert. The constant is one line above the log, so the message loses nothing an operator wanted, and a dismissal would leave every future PR touching this file re-litigating the same finding. Related to #2638 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP * chore(ratchet): re-freeze SubscriptionsPanel raw_gray for the fallback toggle `rawColorRatchet.spec.js` fails: `SubscriptionsPanel.vue raw_gray 165 -> 173`. The +8 is the API-key fallback toggle row #2638 adds — the label, the help text and the toggle's off-state. Not paid down, because there is nothing to pay it down TO: `gray` is the sanctioned chrome family and `tailwind.config.js` defines no semantic neutral (status-* / state-* / brand-* / accent-* / action-* all carry meaning). The eight classes are copied verbatim from the three identical toggles immediately above this one; inventing a one-off token for the fourth would make it the odd one out while leaving its siblings unconverted. Re-frozen in its OWN commit, as the guard's failure message prescribes, and scoped to that ONE entry by hand rather than regenerated — a full regeneration would silently absorb any unrelated drift that has landed on dev since. Related to #2638 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP * fix(2638): don't make the pre-dispatch log a sink for a tainted record CodeQL flags `py/clear-text-logging-sensitive-data` in the switch+retry block: a subscription NAME read off the switch result is tainted from `subscription_credentials`, whose row carries an encrypted token, so the whole record reads as a credential. The destination is dropped from the pre-dispatch line rather than dismissed. `_perform_auto_switch` already logs "Auto-switching agent 'X' from 'A' to 'B'" one frame down, so the interpolation duplicated the frame below it and was not worth a standing false positive on the hot path. The sibling alert on `platform_audit_service.py:391` is untouched by this PR — it has been open on `dev` since 2026-06-04 and is attributed here only by the diff-scan. Related to #2638 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP * fix(2638): declare auth_switched — an undeclared category is coerced to internal `regression diff` caught a real defect, not a stale test. `test_every_declared_category_is_actually_raised_somewhere` keeps `PORTAL_FAILURE_CATEGORIES` closed in BOTH directions, and the new `category="auth_switched"` raise site was not in it. That is not bookkeeping: `record_turn_outcome` coerces an undeclared category to `internal` — silently. So the switch outcome would have been recorded as an uncategorised crash, NOT retryable, with the fixed internal copy in place of the sentence naming the new subscription. The gap-4 fix would have shipped inert while its raise site read as correct. Declared as a TENTH token rather than folded into `auth`, because the two disagree about the only thing the client acts on: `auth` means retrying re-fails, which holds exactly while nothing changed underneath, and a switch is something changing underneath. It needs no client branch — `cancelled` and `invalid_model` are the only categories the client branches on; everything else renders its message and its `retryable` flag. Also drops the destination name from #792's switch log, the second CodeQL `py/clear-text-logging-sensitive-data` sink on this path: a name read off the switch result is tainted from `subscription_credentials`, whose row carries an encrypted token. `_perform_auto_switch` already logs "Auto-switching agent 'X' from 'A' to 'B'" one frame down and the audit row still carries `new_subscription`, so no operator loses anything — only a duplicated interpolation goes. The sibling alert on `platform_audit_service.py:391` is untouched by this PR and has been open on `dev` since 2026-06-04. Related to #2638 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP * fix(subscriptions): a 429 is BILLING, the refusal predicate is three-state, one remediation per turn (#2638) Four review findings. 1. **The client-facing half was inert for the 429 path that produced the bug.** A Claude usage limit surfaces from the agent as 429; `_handle_http_error` classified only 503, so `error_code` stayed None. `TaskExecutionErrorCode. BILLING` had NO assignment site anywhere in `src/backend` — enum definition, comments, and the portal's gate tuple, never set. A Workspace turn is `triggered_by="public"`, which is not async-eligible, so it takes exactly that sync path: AC 4/5 fired only when a failure happened to arrive as 503. A 429 now sets `BILLING`. Downstream is safe by construction — the dispatch breaker counts `auth` only (#526 D10); the #1085 governor does count `billing`, which is what it was written for and which has never been reachable from the sync path, behind a default-OFF flag. 2. **`_assigned_subscription_is_refused` re-opened the #447 OR.** A fresh reading now ends the question in both directions — refusing → switch, not refusing → dispatch — and only the absence of a usable reading falls through to the 2h event predicate. As written, a subscription the provider was demonstrably serving was readmitted by `recovery_verdict` and evacuated by this on every dispatch: a hot-reload and a high-priority notification per turn, and with two such subscriptions, a flap. An unreadable snapshot still falls through rather than clearing, so a Redis blip cannot silently disable the arm. 3. **The pre-dispatch path now spends the turn's one remediation.** It set `subscription_switch`, not `subscription_switch_attempted`, so a turn moved before its first attempt and refused again switched a second time and re-issued — the cascade that flag exists to stop. 4. **`_with_switch` at every terminal return.** `BackendAgentCallBudgetExhausted` and the generic `except Exception` were unwrapped, and both are reachable after a pre-dispatch switch, so the portal would say "not retryable" while the agent sat on a fresh subscription. Tests: `TestTheRefusalPredicateIsThreeState` (five cases, including the two doors agreeing on ONE reading rather than being checked in isolation, and the fail-closed unreadable snapshot); `TestEveryTerminalCarriesTheSwitch`, an AST guard over `execute_task`'s return sites with the two pre-dispatch returns named so a later addition has to be justified; the E2E negative now asserts `error_code` is BILLING — by `.value`/`.name`, since #1085's fieldless-dataclass quirk makes `BILLING == AUTH` True — which is the one assertion that would have caught (1); and a new E2E turn proving a pre-dispatch switch does not switch twice. Each of the four fails against the pre-fix source, verified by reverting them one at a time. 1008 passed across every subscription / task-execution / portal / headroom test. Fixes #2638 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RSjVEjay9ztC1oDXXkh9uN * fix(subscriptions): the serving verdict is trusted at display freshness, not the selection bound (#2638) Found re-reviewing my own #2638 fix. Making `_assigned_subscription_is_refused` three-state was right, but the reading it trusts came from `cached_headroom_readings` with no `max_age_seconds` — i.e. the SELECTION bound, `MAX_READING_AGE_SECONDS` (>= 2h). That bound is calibrated for RANKING candidates, where a stale reading beats none. This call does a different job: it decides whether a provider verdict may OVERRULE the 2h event predicate. A reading as old as the window it overrules cannot — so a two-hour-old "serving" snapshot could suppress a five-minute-old 429 and pin an agent on a subscription that is refusing it right now. The #447 rule is "a probe is ground truth about NOW", and this was applying it to a probe that is no longer about now. It now asks for `FRESHNESS_SECONDS` (30 min) — the same bound `_headroom_indicates_healthy` uses for the same judgement one module over, and the one this file already declares for the mirror case (`REFUSAL_FRESHNESS_SECONDS = FRESHNESS_SECONDS`, "a refusal is trusted exactly as long as the LIMIT badge trusts one"). It tightens the refusing arm too, which is deliberate and safe: a stale refusal falls through to the event predicate rather than evacuating on its own. Tests: the fixture now models the AGE GATE rather than only the lookup (a fake that ignores `max_age_seconds` makes the distinction untestable — the trap the E2E harness already documents for the readmission path), plus three cases — a stale serving verdict falling through to the event predicate, a display-fresh one still winning, and the bound asserted as the ARGUMENT, since omitting it is the bug and a behavioural test alone would pass again the day the default moves. Both new cases red against the unbounded call. 49 passed. Fixes #2638 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RSjVEjay9ztC1oDXXkh9uN * fix(subscriptions): readmission orders the reading against the failure, and reads at the display bound (#2638) The 09-10 merge-train ejection: `recovery_verdict`'s serving_now arm readmitted a skip-listed subscription on ANY non-refusing reading inside the ≥2h selection bound, with none of the ordering the window_reset arm has. A reading taken before the 429 is exactly what a subscription at the wall carries for up to one refresh interval after hitting it, so on a two-subscription install both at the wall every user turn readmitted the other on its pre-wall "ok", switched, failed, and flapped A<->B until a probe recorded rate_limited — #444's ping-pong re-opened from the other side. - `recovery_verdict`: serving_now readmits only when the reading POSTDATES `last_failure_at` (reading instant = now - age_seconds); no failure instant, or an unparseable one, readmits nothing on that arm (fail closed, like the instant arm). A pre-failure "ok" falls through to the window_reset arm, which orders itself. - `_readmit_recovered`: the fresh read is bounded at FRESHNESS_SECONDS — the same bound the evacuation door uses — not the selection bound. - `test_subscription_auto_switch_pingpong.py`: the headroom stub gains the names the merged selector reads (FRESHNESS_SECONDS, RECOVERY_*, recovery_verdict) plus a name-parity assertion, so the suite fails instead of taking the fail-open branch and going inert — the ejection's second finding. - Regression tests for both repro cases (age 600 s / 429 two minutes ago; the unorderable failure) at the pure verdict and through the selector; the "doors agree" invariant restated with the failure predating the reading, plus the one permitted disagreement pinned (both doors shut is not a flap). - Docs: subscription-auto-switch.md "Not covered" and requirements/security.md no longer claim pull terminals never trigger SUB-003 (#2643 gave the sink the hook). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
vybe
added a commit
that referenced
this pull request
Sep 14, 2026
…#554) (#2623) * feat(canvas): delete, pin, search and a stated bound for the canvas pile An agent that uses its canvas the way ent#438 intends accumulates dozens: one per report, per topic, per run. The Workspace could only ever ADD to that pile — the client-portal surface had no delete at all, the only ordering was "newest updated", and nothing bounded the table. Two decisions, both by operator ruling 2026-09-08, recorded because each had a plausible alternative: **Deleting is owner-or-admin.** The answer ent#548 gives for files — the owner deletes the shared artifact. This NARROWS the platform DELETE route, which accepted any user with agent access; safe because no UI called it, so no workflow depended on the wider gate. A canvas is one shared surface with no per-user copy, so a non-owner has no "hide it from my list" middle ground: per AC #2 they see no control at all rather than one that 403s. Agents keep clearing their own (`clear_canvas`, the #918 self-gate). **The bound is a per-agent CAP, not a retention window.** ent#438 recorded "no retention window" because the composite key bounds rows per canvas — but `canvas_id` is agent-chosen, so the COUNT was unbounded; the axis was missed, not decided. `CANVAS_MAX_PER_AGENT` (100, env-tunable) is checked inside `upsert_canvas`'s insert branch, in the same transaction as the INSERT, so it is not a check-then-act race. Updating an existing canvas is NEVER refused — a cap that froze updates would punish exactly the agent that reuses ids — and the refusal is a named 409 telling the agent to retire one, never an eviction: deleting a person's surfaces on a timer is the #1638 failure direction. Both surfaces resolve permission through `db.can_user_share_agent`, the same predicate `assert_agent_owner` uses, so Agent Detail and the Workspace cannot disagree about who owns an agent. The Workspace learns it from `PortalAgentCard.can_manage_canvases` — the portal's only capability channel (#2128), since a portal principal cannot read `/api/settings/feature-flags` — and it fails closed. `pinned` (dual-track: `agent_canvases_pinned` + Alembic 0058, NOT NULL DEFAULT 0, no backfill) is written only by the human pin route and is deliberately absent from every agent-facing tool: `audience` is the agent's decision about who may read, `pinned` is the reader's about what they see first, and an agent that could pin itself to the top would defeat the ordering. A pin survives the agent rewriting the canvas. Living with many is `CanvasPanel.vue`, shared by both surfaces (one rendering layer, per ent#475): search once the list passes six, a height-bounded strip so a long list does not cost the rail its other tabs, and a Manage mode giving each row its age, stale mark, pin and delete. Decidable rules are pure in `canvasUtils.js` — vitest runs `environment: 'node'` with no mount harness, so a rule inside the SFC is one no test can reach. Bulk delete is a POST, not a body-carrying DELETE (bodies on DELETE are permitted-but-unreliable and this one is not optional), declared above the parameterized routes on both routers (Invariant #4), and it reports the ids that EXISTED rather than the ids requested so "3 of 5 removed" is sayable. Three pre-existing guards failed and each was right to: `empty_canvas` was missing the new field (a real bug in this change, fixed), the self-gate guard needed to learn the new gate's name, and the positional-read guard needed its synthetic row extended — that one exists precisely because `_row_to_summary` reads by index. Tests: 20 new backend cases (cap refusal + update-at-cap, pin ordering and survival, bulk scoping, the permission matrix on both surfaces, route ordering, dual-track migration parity, and that the MCP tools cannot pin) and ~24 vitest cases for the pure rules. Verified against a real database: the cap refuses the 4th of 3, updates still succeed at the cap, a pin outranks recency and survives a rewrite, and bulk delete returns only the ids that existed. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * feat(canvas): audit the single delete, cover the default canvas, document the lifecycle Closes the three acceptance criteria the first commit left open: * AC #1 asked for the deletion to be audited and only the BULK route was — the single-canvas route is the one a person actually clicks. Logged only when something was removed, since the route is idempotent and a repeat click would otherwise fill the trail with events where nothing happened. * AC #8: deleting the DEFAULT canvas is allowed, comes back empty on the next write, and frees a slot against the cap. `main` is the id both the MCP tools and the voice panel fall back to, so it is the one most likely to be deleted by accident and the one whose deletion must strand nobody. * AC #9: the user doc gains a "Removing canvases" section stating the permission rule, the cap, and that nothing is ever deleted to make room. Also records a latent pre-existing mismatch found while testing: `empty_canvas` returns None timestamps while `models.Canvas` requires strings, so `Canvas(**empty_canvas(...))` raises. Not live — its only caller declares no `response_model` — but adding one there would turn the voice teardown poll into a 500. Left as a comment where the next person to reach for that will meet it, rather than fixed out of scope. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * feat(canvas): share a canvas at a link, and download it as a PDF A canvas is where an agent's real output lives, and until now it could not leave the Workspace: no share link, no export, nothing in the tree that renders a PDF. **Sharing never widens the audience by accident.** Two scopes, and the default is the narrow one: `authorized` makes the link a DEEP link — opening it requires signing in and the server re-checks `can_user_access_agent`, so it reaches "the people who could already see it" and nobody else. `public` is an explicit, separate, audited choice. Failing narrow is enforced in five independent places (column default, Pydantic default, `normalize_scope`'s fallback, the order of `SHARE_SCOPES`, the radio the dialog preselects), because a link that reaches further than the sharer understood is the one failure this feature must not have. **`agent_canvas_shares` is deliberately its own table.** `agent_public_links` has a `type` column that looks purpose-built for this, and reusing it would have been a real vulnerability: nothing in that table's read path filters on type — `get_public_link_by_token`, `is_link_valid` and `routers/public.py::_validate_public_link` all resolve a token whatever it is — so a canvas row there would ALSO be a working public-CHAT token, and anyone sent a canvas could talk to the agent. (`type='site'` is the same trap already laid; it is unexploited only because nothing creates those rows today.) A separate table makes the isolation structural instead of dependent on every consumer remembering to check. **Live, and it says so** (AC #3, operator ruling): the link renders the canvas as it is now, carrying its `updated_at` and stale mark, and the page states it is not a copy taken at share time. That follows ent#438's model — a canvas is a surface an agent keeps current — and means a share stores nothing. The cost is drift after sharing; the mitigation is revocation, not freezing. **Revocation keeps the row.** `revoked_at` is stamped, never deleted, because a revoked link has to be able to SAY it was revoked (AC #2), which it cannot do once the row is gone. The status vocabulary splits along disclosure: `revoked` and `expired` are returned only for a token that MATCHED a row — whoever holds such a link was already told the canvas exists — while an unknown token and a canvas deleted out from under a link both collapse into one `not_found`, so a stranger guessing tokens learns nothing from the difference. An unparseable `expires_at` reads as expired: a lifetime we cannot read is one we cannot promise is live. **PDF is print-first**, the path the issue recommended: a print stylesheet plus the browser's own PDF. No headless service to run, and — the deciding reason — no second renderer to keep in step with `CanvasBlock`. A server-side renderer was the stated fallback and was not needed: pagination (`break-inside: avoid` per block) and fidelity both fall out of the same markup the screen uses. `CanvasDocument.vue` is the one printable form, rendered by both the shared page and every authenticated surface, so AC #7's "identical from every canvas surface" is true by construction rather than by three surfaces agreeing. It carries title, agent and generation date, forces the light rendering under `@media print`, and when `window.print` is unavailable the control says so and the share link still works. `ent#425` (hosted deliverable pages) is still open, so per this issue's own boundary rule this ships the narrower share link and #425 adopts it later. Verified against a real database, not stubs: the full resolution matrix (public/anonymous → ok, authorized/anonymous → sign-in, authorized/owner → ok, authorized/stranger → refused, unknown → not-found, revoked, expired, unparseable expiry → expired, canvas deleted → not-found), views counted only on a successful render, cross-agent revoke refused, and a second revoke keeping the first revocation time. 23 backend tests, 20 vitest cases for the pure rules. Related to Abilityai/trinity-enterprise#554 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * test(canvas): patch the cap where the live code reads it, not where the test imports it Two ent#553 tests passed in isolation and failed in a full-suite run: `test_the_cap_refuses_a_new_canvas_by_name` and `test_deleting_the_default_canvas_frees_a_slot_against_the_cap`. Order-dependence, not a defect in the feature. They patched `db.canvas.CANVAS_MAX_PER_AGENT` via a fresh `import db.canvas`. Some earlier test in the suite evicts that module from `sys.modules`, so the fresh import hands back a NEW module object while the live `db._canvas_ops` is still an instance of the OLD class — whose `upsert_canvas` reads the OLD module's globals. The patch lands somewhere nothing consults, the cap stays at its default of 100, and the "refuses the 4th of 3" assertions fail. `_set_cap` patches the bound method's own `__globals__`, which is whichever module dict the running code actually closes over — correct whether or not an eviction happened, so it does not depend on knowing which test pollutes. Same failure and same fix as #2589, where the identical shape bit `mark_stale_activities_failed`. Worth noting the class: a monkeypatch on a module attribute is only as good as the assumption that the live object came from that module object, and in a suite that evicts modules that assumption is not free. The feature is unchanged — this touches only the test file. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * fix(canvas): print the canvas, not the whole page Reported from hands-on testing: pressing PDF offered to export the entire page. Correct report — the print stylesheet only STYLED the document and never hid anything else, so `window.print()` printed the nav bar, the tabs, the on-screen panel AND the print copy. That is not "one clean column" by any reading (AC #4), and it is the first thing anyone pressing the button hits. Two halves, both required: * a print rule that hides every `body` child except `.canvas-print-root`; * the print copy TELEPORTED to <body>, so it is a body child and the rule can spare it. Nested inside the app the rule would hide its ancestor and print nothing at all — worse than the bug. `body > *` rather than a class on the app root: it needs no knowledge of how the app is mounted and works identically on the standalone shared page, which keeps AC #7's "identical from every surface" true rather than approximately true. The copy is rendered only while printing (`v-if="printing"` + a `nextTick` flush before `print()`, since printing a not-yet-rendered teleport yields a blank sheet), so the DOM carries no permanent hidden duplicate. `canvasPrintIsolation.spec.js` pins all three structural facts. Nothing automated can inspect a print preview, which is exactly why the bug shipped — so the guard asserts the mechanism instead: the hiding rule exists, the root is teleported to body, and the document mounts before print() is called. Mutation-tested: removing the hiding rule fails it. Also fixes a design-system violation the ratchet caught in the same file: `SharedCanvas` gated its skeleton on a bare `v-if="loading"`. Now `viewState()` — loading means "no data yet", never "a fetch is in flight" (#1927, design-system p13-p15). The page fetches once today, so this is the rule holding rather than a bug fixed; it stays correct if a refresh is added. Baselining my own new violation was the alternative and would have been the wrong one. Related to Abilityai/trinity-enterprise#554 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * fix(canvas): re-parent the pinned revision, and make the canvas cap reachable Two review items from #2619. **Alembic head fork (#2068 class).** `0058_agent_canvases_pinned` shared `down_revision = 0057` with #2608's `0058_portal_file_dismissals`, which has since landed on `dev` — two heads, and `alembic upgrade head` resolves its single target before applying anything, so EVERY revision merged since the fork stops arriving, not just one. Re-parented onto `0058_portal_file_dismissals` and renumbered to `0059` so the prefix keeps being a usable ordering cue; the id is not applied anywhere yet, so the rename costs nothing. `check_alembic_heads.py` reports 1 head. **`CANVAS_MAX_PER_AGENT` was inert (#1039 class).** The refusal message names the number, but the variable was read only from `os.getenv` in `models.py` and appeared in no compose file — so an operator following the refusal's own advice would raise a lever that never reaches the container. Wired into `docker-compose.yml`, `.prod.yml` and `.hosted.yml` (the last two launch standalone, no base merge / no `env_file`) plus `.env.example`. Related to #2619 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP * chore(design-system): re-freeze CanvasPanel's raw-gray ceiling for ent#553 The raw-colour ratchet became enforceable on dev while this branch was open (#2605/#2609), and the merge brings it here: this PR's delete/pin/ search chrome takes `components/canvas/CanvasPanel.vue` from 25 to 46 `raw_gray`, so `tests/unit/rawColorRatchet.spec.js` fails the frontend build. That growth is the honest kind. The design-system contract SPELLS the neutral ink ladder as `gray-N` — surfaces gray-50/100/800/900, borders gray-200/300/700/800, ink gray-300/400/500/600 — and there is no semantic token for a neutral, which is exactly why the spec's own comment says gray is ratcheted but never held to zero for new files. The rule it does hold new code to is `raw_nongray`, and this file stays at **0**. Re-frozen in its OWN commit with the increase named in the baseline's `refrozen` block, which is what the ratchet's error message asks for — not absorbed silently into the feature diff. The entry is hand-edited rather than regenerated so #2605's provenance block survives; no other file's ceiling moves (verified: nothing grew, nothing is stale, no un-baselined file carries `raw_nongray`). Related to #553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP * fix(canvas): state the bound, audit the Workspace writes, gate Manage 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 * fix(canvas): the Workspace audit names the operator, not the platform (ent#553) Found re-reviewing my own audit fix. Adding the rows was right; the attribution was wrong, and a row that lands under the wrong actor is worse than the missing row it replaced — nothing fails, so the wrong answer is believed. `_audit_canvas_change` passed `actor_email` only. But `platform_audit_service._resolve_actor` derives `actor_type` from `actor_user` / `actor_agent_name` / `mcp_scope` / `mcp_key_id` and never from the email, so an email-only call falls through to its last branch: _resolve_actor(None, None, None, None) -> ("system", "trinity-system", None) So every Workspace canvas delete and pin was recorded as `actor_type="system"`, `actor_id="trinity-system"` — a named operator's action attributed to the platform, invisible to any `actor_type=user` query and to the audit UI's per-actor filter. Verified against the real resolver, not by reading the call. The `actor_email`-only path I cited (#848 inline auth) is right where the caller genuinely has no `users` row. That is not this route: `_require_canvas_manager` is platform-only and resolves through `db.can_user_share_agent`, so a row exists by construction. It now resolves that row and passes `actor_user`, producing the same `("user", <id>, <email>)` shape the operator twin has always written — which is the point, since auditing the two surfaces differently buys little more than auditing one of them. Best-effort by construction: the action has already happened, so a lookup that raises or misses must not drop the row. It falls back to the email-only call with a WARNING, since a miss would mean the gate admitted someone the user table does not know. Tests: the regression is pinned against the REAL `_resolve_actor` (both the shape the fix must not return to and the shape it produces now), plus a source guard that the helper resolves a row, passes `actor_user`, keeps the email as a fallback and cannot raise. Removing `actor_user=` reds it. 31 passed on the ent#553 file; 953 across canvas / portal / audit. 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 * fix(migrations): chain 0060_agent_canvas_shares off the renamed pinned revision ent#553 renamed its revision 0058_agent_canvases_pinned -> 0059 when it absorbed dev's 0058_portal_file_dismissals; this revision still pointed at the old id, so after the merge the directory resolved to two heads and `alembic upgrade head` would have applied nothing. Renumbered to 0060 as well so the numeric prefix stays a unique ordering cue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * chore(frontend): re-freeze CanvasPanel.vue raw_gray 46 -> 62 for ent#554 The share/PDF controls add gray chrome copied from the panel's existing header; the branch predates the #2605 ratchet, so the guard first bit when dev was merged in. Scoped to this one entry, in its own commit, as the guard's own message prescribes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * test(canvas): resolve CanvasLimitExceeded from the live method's globals The two cap tests imported the class from `db.canvas` while `_set_cap` already patches the cap through `upsert_canvas.__globals__` — because an earlier test can evict and re-import the module. The same eviction gives the test a different class object than the one the live code raises, and `pytest.raises` then reports the correct refusal as an unexpected exception. Seen once in a full local run after the dev merge (both tests pass in isolation and under CI's three seeds); resolve the class from the same globals the cap comes from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * fix(canvas): keep the selector visible when a search narrows to one match (ent#553 review) `CanvasPanel.vue` gated the chip strip on `visible.length > 1 || manage`, where `visible` is the FILTERED list. Searching down to exactly one canvas hid the strip while the previously selected canvas stayed on screen, and the auto-select watcher — keyed off the unfiltered `props.canvases` — never selected the match. Proven by execution: 7 canvases, query "Topic 3" → strip false, no-match message false. The one canvas the user just searched for was unreachable. Fix: - `canvasSelectorVisible({visible, manage, query})` — with a query, any hit shows the strip; without one, a single canvas is no choice (unchanged). - `canvasAutoSelect(visible, selectedId, query)` — while a query is active the selection follows the matches; no-op with no query or when the current selection already matches. - `CanvasPanel.vue` consumes both: `v-if="selectorVisible"` and a watcher on `[visible, query]`. Tests: - `canvasUtils.spec.js`: the two pure rules. - `canvasPanelSelectorGate.spec.js`: slices the `selectorVisible` computed out of the SFC and RUNS it against the ejection's numbers; pins that the template reads the computed, not a re-derived length test, and that the watcher calls `canvasAutoSelect`. - `test_ent553_canvas_lifecycle.py`: the second review ask — the per-agent cap reaches the wire as a 409 through the real router → service → db chain (only the Redis rate limiter stubbed), names the remedy, and the same PUT against an existing id stays an update. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * fix(canvas): the search box outlives a shrink below the threshold (ent#553 review) `query` has exactly one writer — the search input's `v-model` — and that input was `v-if="showSearch"` with `showSearch = ordered.length > 6`. Seven canvases, type "Topic 3", delete the one match: six canvases, the box unmounts, `visible` still filters on the stale query, the strip collapses, and the panel says *No canvas matches "Topic 3"* with no control left to clear it. Every remaining canvas is unreachable via the chips until navigation. Also reachable with no operator action: the agent's own `clear_canvas` plus a rail refresh while a query is typed. The rule is pure — `canvasSearchVisible(count, threshold, query)` — and keeps the box while a query is active regardless of the count: the typed intent survives the shrink, and the no-match line keeps the one control that clears it. Resetting `query` when the box would flip off was the other option and was rejected: it erases a search the user was mid-way through because a sibling canvas went away. The gate spec that pinned the previous ejection drove `visible`/`query` in isolation from `showSearch`, which is why it could not see this one. It now slices the real `showSearch` computed out of the SFC and RUNS it against the ejection's own numbers (7 → 6 with "Topic 3" typed → box stays; 6 with no query → box gone), and pins that the input is gated on that computed and is the sole writer of `query`. Mutation-checked: reverting the gate to the old length test reds three cases. Four mechanical items from the same review ride along: - requirements/core-agent.md: the ent#438 "deliberately no retention window: bounded by construction" line now says why that reasoning was wrong (rows are bounded per canvas, the count was not) and what bounds it instead; FR-18..FR-22 record delete / bulk / cap / pin / search, which had no requirements entries at all. - raw-color-baseline.json: the `_ent553_note` naming CanvasPanel.vue's 25 → 46 raw_gray was added in 2794388 and dropped by the dev merge aa248f7; re-added so the growth is named in the file. - routers/canvas.py `# mcp:` header now says pin and bulk-delete are unexposed on purpose, and why — Invariant #13's deliberate-vs-forgotten signal. - feature-flows/agent-canvas.md: the two search-state rules and the defect class they close. Verified: vitest 2696 passed (121 files); canvas backend suites 101 passed; raw-colour ratchet and loading-gate ratchet unchanged. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RS8FBA7oEtAAv6GamZKong * fix(canvas): a share link is a grant, so only a human may mint one (ent#554 review) `create_canvas_share`'s docstring said "Owner-or-admin and human-only via `_gate_human_removal`". That gate is not human-only — its own docstring, one screen above, says an agent-scoped key may act on its own agent, which is correct for `clear_canvas` ("an agent tidying up after itself") and wrong for every verb that decides what someone OTHER than the agent may see. So a prompt-injected agent could POST /api/agents/<self>/canvas/<id>/share {"scope": "public"} with the TRINITY_MCP_API_KEY already in its container and publish its own canvas at an unauthenticated URL. Three things make that worse than it first reads: * the share is LIVE, not a snapshot, so one link is a self-updating channel rather than a one-time disclosure; * the agent is the only writer of canvas blocks, so anything it can read it can copy into a canvas and publish; * `audience` is not consulted on the share path, so ent#438's fail-closed "a canvas reaches a client only because the agent said so" would not have applied — the agent would have been choosing for itself. `list_canvas_shares` had the same gate and returns the TOKEN, which is the capability itself; `revoke_canvas_share` too, so an agent could also turn off a person's link. The fix is the grant-vs-use line (Invariant #8): the endpoint that USES a capability may be agent-callable, the one that GRANTS one is human-only. * `_gate_human_only(current_user, name, *, agent_detail)` is factored out of `_gate_pin` — the predicate was always right, only its NAME described one caller. A gate named for a verb ("removal") is one a fourth caller reaches past by accident; a gate named for its rule is not. `_gate_pin` and the new `_gate_share` both delegate to it, with per-caller refusal text because an agent reads that message to decide what to do next. * The three share routes now call `_gate_share`. * The delete routes deliberately KEEP `_gate_human_removal`, and a test guards that boundary in the other direction — the first attempt at this fix swept `clear_canvas` into the human-only gate, because one `str.replace` matched both bodies. That would have broken a real MCP tool for every agent: a security fix breeding the next bug, the /review §4.14 class. Six regression tests; four of them fail against the previous commit (the other two are the over-correction guards, which must pass both ways by design). The 23 tests already here covered scope defaults, expiry, revocation and enumeration, but none used an agent principal on any share route — which is how this shipped. Docs: the user doc now states that sharing is the owner's alone and that the routes refuse an agent's own key, beside the same sentence for pin; the flow doc records the decision, the blast radius, and why the delete routes stay permissive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ * docs(canvas): record the share routes in the file's own mcp: convention (ent#554 review) The header comment lists which canvas routes are deliberately NOT exposed as MCP tools and why. ent#554 added three that qualify — minting, listing and revoking a share link — and the list did not grow with them. Worth more than a comment here: the ent#553 entry states the rule the share routes then failed to follow ("no tool exposes it" is a property of the client), so a reader consulting this header to decide a fourth route's gate would have found the reasoning but not the precedent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ * docs(learnings): a gate named after a verb gets reached for by the wrong route (ent#554 review) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Eugene Vyborov <eugene@ability.ai>
vybe
added a commit
that referenced
this pull request
Sep 14, 2026
#2628) * feat(canvas): delete, pin, search and a stated bound for the canvas pile An agent that uses its canvas the way ent#438 intends accumulates dozens: one per report, per topic, per run. The Workspace could only ever ADD to that pile — the client-portal surface had no delete at all, the only ordering was "newest updated", and nothing bounded the table. Two decisions, both by operator ruling 2026-09-08, recorded because each had a plausible alternative: **Deleting is owner-or-admin.** The answer ent#548 gives for files — the owner deletes the shared artifact. This NARROWS the platform DELETE route, which accepted any user with agent access; safe because no UI called it, so no workflow depended on the wider gate. A canvas is one shared surface with no per-user copy, so a non-owner has no "hide it from my list" middle ground: per AC #2 they see no control at all rather than one that 403s. Agents keep clearing their own (`clear_canvas`, the #918 self-gate). **The bound is a per-agent CAP, not a retention window.** ent#438 recorded "no retention window" because the composite key bounds rows per canvas — but `canvas_id` is agent-chosen, so the COUNT was unbounded; the axis was missed, not decided. `CANVAS_MAX_PER_AGENT` (100, env-tunable) is checked inside `upsert_canvas`'s insert branch, in the same transaction as the INSERT, so it is not a check-then-act race. Updating an existing canvas is NEVER refused — a cap that froze updates would punish exactly the agent that reuses ids — and the refusal is a named 409 telling the agent to retire one, never an eviction: deleting a person's surfaces on a timer is the #1638 failure direction. Both surfaces resolve permission through `db.can_user_share_agent`, the same predicate `assert_agent_owner` uses, so Agent Detail and the Workspace cannot disagree about who owns an agent. The Workspace learns it from `PortalAgentCard.can_manage_canvases` — the portal's only capability channel (#2128), since a portal principal cannot read `/api/settings/feature-flags` — and it fails closed. `pinned` (dual-track: `agent_canvases_pinned` + Alembic 0058, NOT NULL DEFAULT 0, no backfill) is written only by the human pin route and is deliberately absent from every agent-facing tool: `audience` is the agent's decision about who may read, `pinned` is the reader's about what they see first, and an agent that could pin itself to the top would defeat the ordering. A pin survives the agent rewriting the canvas. Living with many is `CanvasPanel.vue`, shared by both surfaces (one rendering layer, per ent#475): search once the list passes six, a height-bounded strip so a long list does not cost the rail its other tabs, and a Manage mode giving each row its age, stale mark, pin and delete. Decidable rules are pure in `canvasUtils.js` — vitest runs `environment: 'node'` with no mount harness, so a rule inside the SFC is one no test can reach. Bulk delete is a POST, not a body-carrying DELETE (bodies on DELETE are permitted-but-unreliable and this one is not optional), declared above the parameterized routes on both routers (Invariant #4), and it reports the ids that EXISTED rather than the ids requested so "3 of 5 removed" is sayable. Three pre-existing guards failed and each was right to: `empty_canvas` was missing the new field (a real bug in this change, fixed), the self-gate guard needed to learn the new gate's name, and the positional-read guard needed its synthetic row extended — that one exists precisely because `_row_to_summary` reads by index. Tests: 20 new backend cases (cap refusal + update-at-cap, pin ordering and survival, bulk scoping, the permission matrix on both surfaces, route ordering, dual-track migration parity, and that the MCP tools cannot pin) and ~24 vitest cases for the pure rules. Verified against a real database: the cap refuses the 4th of 3, updates still succeed at the cap, a pin outranks recency and survives a rewrite, and bulk delete returns only the ids that existed. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * feat(canvas): audit the single delete, cover the default canvas, document the lifecycle Closes the three acceptance criteria the first commit left open: * AC #1 asked for the deletion to be audited and only the BULK route was — the single-canvas route is the one a person actually clicks. Logged only when something was removed, since the route is idempotent and a repeat click would otherwise fill the trail with events where nothing happened. * AC #8: deleting the DEFAULT canvas is allowed, comes back empty on the next write, and frees a slot against the cap. `main` is the id both the MCP tools and the voice panel fall back to, so it is the one most likely to be deleted by accident and the one whose deletion must strand nobody. * AC #9: the user doc gains a "Removing canvases" section stating the permission rule, the cap, and that nothing is ever deleted to make room. Also records a latent pre-existing mismatch found while testing: `empty_canvas` returns None timestamps while `models.Canvas` requires strings, so `Canvas(**empty_canvas(...))` raises. Not live — its only caller declares no `response_model` — but adding one there would turn the voice teardown poll into a 500. Left as a comment where the next person to reach for that will meet it, rather than fixed out of scope. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * feat(canvas): share a canvas at a link, and download it as a PDF A canvas is where an agent's real output lives, and until now it could not leave the Workspace: no share link, no export, nothing in the tree that renders a PDF. **Sharing never widens the audience by accident.** Two scopes, and the default is the narrow one: `authorized` makes the link a DEEP link — opening it requires signing in and the server re-checks `can_user_access_agent`, so it reaches "the people who could already see it" and nobody else. `public` is an explicit, separate, audited choice. Failing narrow is enforced in five independent places (column default, Pydantic default, `normalize_scope`'s fallback, the order of `SHARE_SCOPES`, the radio the dialog preselects), because a link that reaches further than the sharer understood is the one failure this feature must not have. **`agent_canvas_shares` is deliberately its own table.** `agent_public_links` has a `type` column that looks purpose-built for this, and reusing it would have been a real vulnerability: nothing in that table's read path filters on type — `get_public_link_by_token`, `is_link_valid` and `routers/public.py::_validate_public_link` all resolve a token whatever it is — so a canvas row there would ALSO be a working public-CHAT token, and anyone sent a canvas could talk to the agent. (`type='site'` is the same trap already laid; it is unexploited only because nothing creates those rows today.) A separate table makes the isolation structural instead of dependent on every consumer remembering to check. **Live, and it says so** (AC #3, operator ruling): the link renders the canvas as it is now, carrying its `updated_at` and stale mark, and the page states it is not a copy taken at share time. That follows ent#438's model — a canvas is a surface an agent keeps current — and means a share stores nothing. The cost is drift after sharing; the mitigation is revocation, not freezing. **Revocation keeps the row.** `revoked_at` is stamped, never deleted, because a revoked link has to be able to SAY it was revoked (AC #2), which it cannot do once the row is gone. The status vocabulary splits along disclosure: `revoked` and `expired` are returned only for a token that MATCHED a row — whoever holds such a link was already told the canvas exists — while an unknown token and a canvas deleted out from under a link both collapse into one `not_found`, so a stranger guessing tokens learns nothing from the difference. An unparseable `expires_at` reads as expired: a lifetime we cannot read is one we cannot promise is live. **PDF is print-first**, the path the issue recommended: a print stylesheet plus the browser's own PDF. No headless service to run, and — the deciding reason — no second renderer to keep in step with `CanvasBlock`. A server-side renderer was the stated fallback and was not needed: pagination (`break-inside: avoid` per block) and fidelity both fall out of the same markup the screen uses. `CanvasDocument.vue` is the one printable form, rendered by both the shared page and every authenticated surface, so AC #7's "identical from every canvas surface" is true by construction rather than by three surfaces agreeing. It carries title, agent and generation date, forces the light rendering under `@media print`, and when `window.print` is unavailable the control says so and the share link still works. `ent#425` (hosted deliverable pages) is still open, so per this issue's own boundary rule this ships the narrower share link and #425 adopts it later. Verified against a real database, not stubs: the full resolution matrix (public/anonymous → ok, authorized/anonymous → sign-in, authorized/owner → ok, authorized/stranger → refused, unknown → not-found, revoked, expired, unparseable expiry → expired, canvas deleted → not-found), views counted only on a successful render, cross-agent revoke refused, and a second revoke keeping the first revocation time. 23 backend tests, 20 vitest cases for the pure rules. Related to Abilityai/trinity-enterprise#554 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * test(canvas): patch the cap where the live code reads it, not where the test imports it Two ent#553 tests passed in isolation and failed in a full-suite run: `test_the_cap_refuses_a_new_canvas_by_name` and `test_deleting_the_default_canvas_frees_a_slot_against_the_cap`. Order-dependence, not a defect in the feature. They patched `db.canvas.CANVAS_MAX_PER_AGENT` via a fresh `import db.canvas`. Some earlier test in the suite evicts that module from `sys.modules`, so the fresh import hands back a NEW module object while the live `db._canvas_ops` is still an instance of the OLD class — whose `upsert_canvas` reads the OLD module's globals. The patch lands somewhere nothing consults, the cap stays at its default of 100, and the "refuses the 4th of 3" assertions fail. `_set_cap` patches the bound method's own `__globals__`, which is whichever module dict the running code actually closes over — correct whether or not an eviction happened, so it does not depend on knowing which test pollutes. Same failure and same fix as #2589, where the identical shape bit `mark_stale_activities_failed`. Worth noting the class: a monkeypatch on a module attribute is only as good as the assumption that the live object came from that module object, and in a suite that evicts modules that assumption is not free. The feature is unchanged — this touches only the test file. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * fix(canvas): print the canvas, not the whole page Reported from hands-on testing: pressing PDF offered to export the entire page. Correct report — the print stylesheet only STYLED the document and never hid anything else, so `window.print()` printed the nav bar, the tabs, the on-screen panel AND the print copy. That is not "one clean column" by any reading (AC #4), and it is the first thing anyone pressing the button hits. Two halves, both required: * a print rule that hides every `body` child except `.canvas-print-root`; * the print copy TELEPORTED to <body>, so it is a body child and the rule can spare it. Nested inside the app the rule would hide its ancestor and print nothing at all — worse than the bug. `body > *` rather than a class on the app root: it needs no knowledge of how the app is mounted and works identically on the standalone shared page, which keeps AC #7's "identical from every surface" true rather than approximately true. The copy is rendered only while printing (`v-if="printing"` + a `nextTick` flush before `print()`, since printing a not-yet-rendered teleport yields a blank sheet), so the DOM carries no permanent hidden duplicate. `canvasPrintIsolation.spec.js` pins all three structural facts. Nothing automated can inspect a print preview, which is exactly why the bug shipped — so the guard asserts the mechanism instead: the hiding rule exists, the root is teleported to body, and the document mounts before print() is called. Mutation-tested: removing the hiding rule fails it. Also fixes a design-system violation the ratchet caught in the same file: `SharedCanvas` gated its skeleton on a bare `v-if="loading"`. Now `viewState()` — loading means "no data yet", never "a fetch is in flight" (#1927, design-system p13-p15). The page fetches once today, so this is the rule holding rather than a bug fixed; it stays correct if a refresh is added. Baselining my own new violation was the alternative and would have been the wrong one. Related to Abilityai/trinity-enterprise#554 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * feat(canvas): the open canvas is shared context for the turn When a user with a canvas on screen says "add a column to this", the agent now knows which canvas they mean. Before this the turn carried the message and nothing about the surface around it, so the agent asked, guessed, or minted a new canvas beside the one being looked at. The mechanism is a per-turn context field — `schedule_executions.open_canvas_id`, the same shape as the `source_channel*` columns beside it — stamped at dispatch and read back by the tools and the prompt. It is CONTEXT, never AUTHORITY, and two independent halves keep it there: - `validated_open_canvas` decides what may be STAMPED. The id is client-supplied, so it is checked against the agent's own canvases and against what that caller can see: an operator-only canvas is invisible to an external client (otherwise the field is an existence oracle for canvases the agent keeps privately), and another agent's canvas is refused outright. Every failure degrades to "nothing open" — never an error, never a wider reach. - `effective_canvas_id` decides what a tool ACTS on, and cannot widen anything: every read and write still passes the existing ownership and audience gates. Precedence is stated once so all three tools agree: `explicit canvas_id > the canvas the user has open > the default canvas`. It returns WHY as well as WHICH, because with nothing named the agent has to be able to say which canvas it wrote to — "I updated the canvas" is not good enough when there are eight and the user is looking at one. Both delivery paths are needed, not one. The MCP tools resolve a missing `canvas_id` through `GET /api/agents/{name}/canvas/context` (declared above `/{canvas_id}` — Invariant #4, since "context" is a valid id shape), AND the turn prompt names the open canvas: a tool default handles a call that omits an id, but an agent must READ a canvas before editing it and cannot read what it cannot name. The prompt line rides the same prefix as the file manifest, so it is present on a resumed turn too — the open canvas changes between turns while the session's memory of it does not. A canvas deleted mid-conversation resolves to nothing, re-checked at read time rather than trusted from the stamp: ent#553 made deleting one click, and a surviving id would have the agent's next write CREATE a canvas under it, silently resurrecting something a person deleted. Voice inherits this by construction — ent#440 submits a spoken utterance through the same `deliver()` a typed one takes. The `canvas` tools in `gemini_voice.py` are deliberately untouched: that is VOICE-001's ephemeral display panel, a different surface with no persisted id. Dual-track migration (`execution_open_canvas` + Alembic `0060`); the column is nullable, so every existing row and every un-updated caller reads as "nothing open". Related to Abilityai/trinity-enterprise#555 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ * fix(canvas): re-parent the pinned revision, and make the canvas cap reachable Two review items from #2619. **Alembic head fork (#2068 class).** `0058_agent_canvases_pinned` shared `down_revision = 0057` with #2608's `0058_portal_file_dismissals`, which has since landed on `dev` — two heads, and `alembic upgrade head` resolves its single target before applying anything, so EVERY revision merged since the fork stops arriving, not just one. Re-parented onto `0058_portal_file_dismissals` and renumbered to `0059` so the prefix keeps being a usable ordering cue; the id is not applied anywhere yet, so the rename costs nothing. `check_alembic_heads.py` reports 1 head. **`CANVAS_MAX_PER_AGENT` was inert (#1039 class).** The refusal message names the number, but the variable was read only from `os.getenv` in `models.py` and appeared in no compose file — so an operator following the refusal's own advice would raise a lever that never reaches the container. Wired into `docker-compose.yml`, `.prod.yml` and `.hosted.yml` (the last two launch standalone, no base merge / no `env_file`) plus `.env.example`. Related to #2619 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP * chore(design-system): re-freeze CanvasPanel's raw-gray ceiling for ent#553 The raw-colour ratchet became enforceable on dev while this branch was open (#2605/#2609), and the merge brings it here: this PR's delete/pin/ search chrome takes `components/canvas/CanvasPanel.vue` from 25 to 46 `raw_gray`, so `tests/unit/rawColorRatchet.spec.js` fails the frontend build. That growth is the honest kind. The design-system contract SPELLS the neutral ink ladder as `gray-N` — surfaces gray-50/100/800/900, borders gray-200/300/700/800, ink gray-300/400/500/600 — and there is no semantic token for a neutral, which is exactly why the spec's own comment says gray is ratcheted but never held to zero for new files. The rule it does hold new code to is `raw_nongray`, and this file stays at **0**. Re-frozen in its OWN commit with the increase named in the baseline's `refrozen` block, which is what the ratchet's error message asks for — not absorbed silently into the feature diff. The entry is hand-edited rather than regenerated so #2605's provenance block survives; no other file's ceiling moves (verified: nothing grew, nothing is stale, no un-baselined file carries `raw_nongray`). Related to #553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP * fix(canvas): state the bound, audit the Workspace writes, gate Manage 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 * fix(canvas): the Workspace audit names the operator, not the platform (ent#553) Found re-reviewing my own audit fix. Adding the rows was right; the attribution was wrong, and a row that lands under the wrong actor is worse than the missing row it replaced — nothing fails, so the wrong answer is believed. `_audit_canvas_change` passed `actor_email` only. But `platform_audit_service._resolve_actor` derives `actor_type` from `actor_user` / `actor_agent_name` / `mcp_scope` / `mcp_key_id` and never from the email, so an email-only call falls through to its last branch: _resolve_actor(None, None, None, None) -> ("system", "trinity-system", None) So every Workspace canvas delete and pin was recorded as `actor_type="system"`, `actor_id="trinity-system"` — a named operator's action attributed to the platform, invisible to any `actor_type=user` query and to the audit UI's per-actor filter. Verified against the real resolver, not by reading the call. The `actor_email`-only path I cited (#848 inline auth) is right where the caller genuinely has no `users` row. That is not this route: `_require_canvas_manager` is platform-only and resolves through `db.can_user_share_agent`, so a row exists by construction. It now resolves that row and passes `actor_user`, producing the same `("user", <id>, <email>)` shape the operator twin has always written — which is the point, since auditing the two surfaces differently buys little more than auditing one of them. Best-effort by construction: the action has already happened, so a lookup that raises or misses must not drop the row. It falls back to the email-only call with a WARNING, since a miss would mean the gate admitted someone the user table does not know. Tests: the regression is pinned against the REAL `_resolve_actor` (both the shape the fix must not return to and the shape it produces now), plus a source guard that the helper resolves a row, passes `actor_user`, keeps the email as a fallback and cannot raise. Removing `actor_user=` reds it. 31 passed on the ent#553 file; 953 across canvas / portal / audit. 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 * fix(migrations): chain 0060_agent_canvas_shares off the renamed pinned revision ent#553 renamed its revision 0058_agent_canvases_pinned -> 0059 when it absorbed dev's 0058_portal_file_dismissals; this revision still pointed at the old id, so after the merge the directory resolved to two heads and `alembic upgrade head` would have applied nothing. Renumbered to 0060 as well so the numeric prefix stays a unique ordering cue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * chore(frontend): re-freeze CanvasPanel.vue raw_gray 46 -> 62 for ent#554 The share/PDF controls add gray chrome copied from the panel's existing header; the branch predates the #2605 ratchet, so the guard first bit when dev was merged in. Scoped to this one entry, in its own commit, as the guard's own message prescribes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * fix(migrations): renumber execution_open_canvas to 0061 behind 0060_agent_canvas_shares Follows the ent#554 renumber so the chain reads 0059 pinned -> 0060 shares -> 0061 open-canvas with unique prefixes and a single head. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * test(canvas): resolve CanvasLimitExceeded from the live method's globals The two cap tests imported the class from `db.canvas` while `_set_cap` already patches the cap through `upsert_canvas.__globals__` — because an earlier test can evict and re-import the module. The same eviction gives the test a different class object than the one the live code raises, and `pytest.raises` then reports the correct refusal as an unexpected exception. Seen once in a full local run after the dev merge (both tests pass in isolation and under CI's three seeds); resolve the class from the same globals the cap comes from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * fix(canvas): keep the selector visible when a search narrows to one match (ent#553 review) `CanvasPanel.vue` gated the chip strip on `visible.length > 1 || manage`, where `visible` is the FILTERED list. Searching down to exactly one canvas hid the strip while the previously selected canvas stayed on screen, and the auto-select watcher — keyed off the unfiltered `props.canvases` — never selected the match. Proven by execution: 7 canvases, query "Topic 3" → strip false, no-match message false. The one canvas the user just searched for was unreachable. Fix: - `canvasSelectorVisible({visible, manage, query})` — with a query, any hit shows the strip; without one, a single canvas is no choice (unchanged). - `canvasAutoSelect(visible, selectedId, query)` — while a query is active the selection follows the matches; no-op with no query or when the current selection already matches. - `CanvasPanel.vue` consumes both: `v-if="selectorVisible"` and a watcher on `[visible, query]`. Tests: - `canvasUtils.spec.js`: the two pure rules. - `canvasPanelSelectorGate.spec.js`: slices the `selectorVisible` computed out of the SFC and RUNS it against the ejection's numbers; pins that the template reads the computed, not a re-derived length test, and that the watcher calls `canvasAutoSelect`. - `test_ent553_canvas_lifecycle.py`: the second review ask — the per-agent cap reaches the wire as a 409 through the real router → service → db chain (only the Redis rate limiter stubbed), names the remedy, and the same PUT against an existing id stays an update. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015owqMKD5QDjzZrUTF2Joht * fix(canvas): the search box outlives a shrink below the threshold (ent#553 review) `query` has exactly one writer — the search input's `v-model` — and that input was `v-if="showSearch"` with `showSearch = ordered.length > 6`. Seven canvases, type "Topic 3", delete the one match: six canvases, the box unmounts, `visible` still filters on the stale query, the strip collapses, and the panel says *No canvas matches "Topic 3"* with no control left to clear it. Every remaining canvas is unreachable via the chips until navigation. Also reachable with no operator action: the agent's own `clear_canvas` plus a rail refresh while a query is typed. The rule is pure — `canvasSearchVisible(count, threshold, query)` — and keeps the box while a query is active regardless of the count: the typed intent survives the shrink, and the no-match line keeps the one control that clears it. Resetting `query` when the box would flip off was the other option and was rejected: it erases a search the user was mid-way through because a sibling canvas went away. The gate spec that pinned the previous ejection drove `visible`/`query` in isolation from `showSearch`, which is why it could not see this one. It now slices the real `showSearch` computed out of the SFC and RUNS it against the ejection's own numbers (7 → 6 with "Topic 3" typed → box stays; 6 with no query → box gone), and pins that the input is gated on that computed and is the sole writer of `query`. Mutation-checked: reverting the gate to the old length test reds three cases. Four mechanical items from the same review ride along: - requirements/core-agent.md: the ent#438 "deliberately no retention window: bounded by construction" line now says why that reasoning was wrong (rows are bounded per canvas, the count was not) and what bounds it instead; FR-18..FR-22 record delete / bulk / cap / pin / search, which had no requirements entries at all. - raw-color-baseline.json: the `_ent553_note` naming CanvasPanel.vue's 25 → 46 raw_gray was added in 2794388 and dropped by the dev merge aa248f7; re-added so the growth is named in the file. - routers/canvas.py `# mcp:` header now says pin and bulk-delete are unexposed on purpose, and why — Invariant #13's deliberate-vs-forgotten signal. - feature-flows/agent-canvas.md: the two search-state rules and the defect class they close. Verified: vitest 2696 passed (121 files); canvas backend suites 101 passed; raw-colour ratchet and loading-gate ratchet unchanged. Related to Abilityai/trinity-enterprise#553 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RS8FBA7oEtAAv6GamZKong * fix(canvas): a share link is a grant, so only a human may mint one (ent#554 review) `create_canvas_share`'s docstring said "Owner-or-admin and human-only via `_gate_human_removal`". That gate is not human-only — its own docstring, one screen above, says an agent-scoped key may act on its own agent, which is correct for `clear_canvas` ("an agent tidying up after itself") and wrong for every verb that decides what someone OTHER than the agent may see. So a prompt-injected agent could POST /api/agents/<self>/canvas/<id>/share {"scope": "public"} with the TRINITY_MCP_API_KEY already in its container and publish its own canvas at an unauthenticated URL. Three things make that worse than it first reads: * the share is LIVE, not a snapshot, so one link is a self-updating channel rather than a one-time disclosure; * the agent is the only writer of canvas blocks, so anything it can read it can copy into a canvas and publish; * `audience` is not consulted on the share path, so ent#438's fail-closed "a canvas reaches a client only because the agent said so" would not have applied — the agent would have been choosing for itself. `list_canvas_shares` had the same gate and returns the TOKEN, which is the capability itself; `revoke_canvas_share` too, so an agent could also turn off a person's link. The fix is the grant-vs-use line (Invariant #8): the endpoint that USES a capability may be agent-callable, the one that GRANTS one is human-only. * `_gate_human_only(current_user, name, *, agent_detail)` is factored out of `_gate_pin` — the predicate was always right, only its NAME described one caller. A gate named for a verb ("removal") is one a fourth caller reaches past by accident; a gate named for its rule is not. `_gate_pin` and the new `_gate_share` both delegate to it, with per-caller refusal text because an agent reads that message to decide what to do next. * The three share routes now call `_gate_share`. * The delete routes deliberately KEEP `_gate_human_removal`, and a test guards that boundary in the other direction — the first attempt at this fix swept `clear_canvas` into the human-only gate, because one `str.replace` matched both bodies. That would have broken a real MCP tool for every agent: a security fix breeding the next bug, the /review §4.14 class. Six regression tests; four of them fail against the previous commit (the other two are the over-correction guards, which must pass both ways by design). The 23 tests already here covered scope defaults, expiry, revocation and enumeration, but none used an agent principal on any share route — which is how this shipped. Docs: the user doc now states that sharing is the owner's alone and that the routes refuse an agent's own key, beside the same sentence for pin; the flow doc records the decision, the blast radius, and why the delete routes stay permissive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ * docs(canvas): record the share routes in the file's own mcp: convention (ent#554 review) The header comment lists which canvas routes are deliberately NOT exposed as MCP tools and why. ent#554 added three that qualify — minting, listing and revoking a share link — and the list did not grow with them. Worth more than a comment here: the ent#553 entry states the rule the share routes then failed to follow ("no tool exposes it" is a property of the client), so a reader consulting this header to decide a fourth route's gate would have found the reasoning but not the precedent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ * docs(learnings): a gate named after a verb gets reached for by the wrong route (ent#554 review) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Eugene Vyborov <eugene@ability.ai>
8 tasks
vybe
pushed a commit
that referenced
this pull request
Sep 18, 2026
Lands the trinity-dev backlog merged on 2026-09-18: 13 PRs (#6, #7, #8, #11, #14, #16, #17, #21, #22, #24, #26, #28, #29), plus the /release lessons from the v0.9.5 cut — the DigitalOcean installer tag moves with VERSION (#24), and the headline commit count comes from the previous release PR's head, not the tag. Private submodule: OSS clones skip it (update = none), so nothing changes for external contributors. Co-authored-by: sim <sim@example.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 addresses two security/housekeeping issues discovered during a security scan analysis:
1. Remove Token Logging from MCP Client (Security Fix)
Problem: The MCP client (
src/mcp-server/src/client.ts) was logging the first 20 characters of JWT tokens and token length on every API request:This could expose sensitive information in production logs (CloudWatch, Datadog, etc.) and potentially aid attackers in token analysis.
Solution:
DEBUG_MCP_CLIENT=trueorNODE_ENV=development)Auth: present/missing)2. Add pytest HTML Reports to .gitignore (Housekeeping)
Problem: Auto-generated pytest HTML test reports (
tests/reports/*.html) were not in.gitignore, potentially leading to accidental commits.Solution: Added
tests/reports/*.htmlto the test artifacts section of.gitignore.Test Plan
DEBUG_MCP_CLIENT=true npm start- debug logs should workNODE_ENV=production npm start- tokens should NOT be loggedImpact