Skip to content

security: implement safe tar extraction with symlink/hardlink validation - #8

Merged
vybe merged 1 commit into
Abilityai:mainfrom
webmixgamer:security/safe-tar-extraction
Jan 18, 2026
Merged

vybe merged 1 commit into
Abilityai:mainfrom
webmixgamer:security/safe-tar-extraction

Conversation

@webmixgamer

Copy link
Copy Markdown
Contributor

Security Fix: Safe Tar Extraction

Description

This PR implements secure tar archive extraction for the local agent deployment service (deploy.py). It addresses the Path Traversal vulnerability identified in the security audit.

Previously, the code only checked for .. and absolute paths in member names, which was insufficient to prevent Zip Slip attacks via symlinks or hardlinks.

Changes

  • Added _validate_tar_member() helper to validate every archive member before extraction.
  • Implemented _is_path_within() using pathlib.Path.resolve() to securely check path containment.
  • Added strict checks:
    • Blocks absolute paths and .. traversal.
    • Blocks symlinks/hardlinks pointing outside the extraction directory.
    • Blocks special file types (device files, FIFOs).
    • Allows safe internal symlinks (required for some agent templates).
  • Added comprehensive unit tests in tests/test_archive_security.py covering all edge cases.

Verification

  • Added new unit tests pass: pytest tests/test_archive_security.py
  • Manual verification with malicious archive (symlink to /etc/passwd) confirms it is rejected with INVALID_ARCHIVE.
  • Valid agent templates still deploy successfully.

Add comprehensive archive validation for local agent deployment to prevent
path traversal attacks via symlinks and hardlinks.

Changes:
- Add _is_path_within() helper for path containment checks using resolve()
- Add _validate_tar_member() to validate each archive member:
  - Rejects absolute paths and path traversal (../)
  - Rejects symlinks/hardlinks pointing outside extraction directory
  - Rejects device files (chr/blk) and FIFOs
  - Allows internal symlinks/hardlinks that resolve within temp_dir
- Add _safe_extract_tar() wrapper that validates all members before extraction
- Replace simple startswith/'..' check with full validation
- Add unit tests for validation logic (tests/test_archive_security.py)

Addresses H-02 finding from security scan.
@webmixgamer
webmixgamer requested a review from vybe January 17, 2026 19:10
@vybe
vybe merged commit fbeac62 into Abilityai:main Jan 18, 2026
oleksandr-korin added a commit that referenced this pull request Jan 19, 2026
Interactive Test Results:
- I1: Approval Routes ✅
- I2: Multi-Stage Approval ✅ (revealed output bug)
- I3: Complex Workflow (Gateway + Approval) ✅
- I4: Parallel Work + Approval ✅

NEW BUGS FOUND:
- Issue #8: Approval decision NOT stored in step output
  - Conditions like 'steps.X.output.decision == approved' fail
  - Impact: Cannot route based on approval value

UI ISSUES CONFIRMED:
- Page doesn't auto-refresh when approval step becomes active
- Must manually refresh to see Approve/Reject buttons
- Confirmed across I1, I2, I3, I4
- Skipped steps don't show WHY they were skipped

4 interactive scenarios created for future testing:
- processes/interactive/i1-approval-routes.yaml
- processes/interactive/i2-multi-stage-approval.yaml
- processes/interactive/i3-complex-workflow.yaml
- processes/interactive/i4-parallel-work-approval.yaml
oleksandr-korin added a commit that referenced this pull request Jan 19, 2026
Interactive Test Results:
- I1: Approval Routes ✅
- I2: Multi-Stage Approval ✅ (revealed output bug)
- I3: Complex Workflow (Gateway + Approval) ✅
- I4: Parallel Work + Approval ✅

NEW BUGS FOUND:
- Issue #8: Approval decision NOT stored in step output
  - Conditions like 'steps.X.output.decision == approved' fail
  - Impact: Cannot route based on approval value

UI ISSUES CONFIRMED:
- Page doesn't auto-refresh when approval step becomes active
- Must manually refresh to see Approve/Reject buttons
- Confirmed across I1, I2, I3, I4
- Skipped steps don't show WHY they were skipped

4 interactive scenarios created for future testing:
- processes/interactive/i1-approval-routes.yaml
- processes/interactive/i2-multi-stage-approval.yaml
- processes/interactive/i3-complex-workflow.yaml
- processes/interactive/i4-parallel-work-approval.yaml
vybe added a commit that referenced this pull request Feb 10, 2026
- Update @modelcontextprotocol/sdk to ^1.26.0 (fixes CVE cross-client data leak)
- Update hono to >=4.11.7 (fixes 4 moderate vulnerabilities)
- Update fastmcp to ^3.32.0
- Add npm overrides to force patched versions in transitive deps

Fixes Dependabot alerts #8-17

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
vybe added a commit that referenced this pull request Apr 18, 2026
…idate-architecture

Backend: unregister 7 Process Engine routers (processes, executions, approvals,
triggers, alerts, process_templates, audit) plus the process-docs router; drop
startup hooks for execution recovery and the process-engine WebSocket publisher.
Services under services/process_engine/ remain in place as dormant code.

Frontend: remove 11 process-related routes (/processes, /processes/new,
/processes/docs, /processes/wizard, /processes/:id, /executions, /approvals,
/executions/:id, /process-dashboard) and the dead isProcessSection computed in
NavBar. Keep /alerts and /events legacy redirects to Operating Room.

Docs: correct stale count claims in architecture.md (main.py line count,
router count 45 -> 53, service count 23 -> 37, MCP tool modules 15 -> 16)
and expand database.py scope description to reflect 27 domain op classes.

Skill: expand validate-architecture to detect drift between arch.md and code:
count alignment (D1), scope coherence (D2), enforced MCP parity under #13
(tool module OR '# mcp: none' opt-out), and inline authorization sprawl
detection under #8. Output now includes suggested arch.md edits with line
numbers, not just pass/fail.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
vybe pushed a commit that referenced this pull request Apr 27, 2026
Add Depends(get_current_user) to the three handlers in
src/backend/routers/docs.py so the file no longer violates
Architectural Invariant #8.

Note: this router is not currently registered in main.py, so
the endpoints are not reachable on the running API. The fix is
applied to the file as written so the invariant validator stops
flagging it and so the file is correct if it is ever remounted.

Closes #452
vybe added a commit that referenced this pull request Apr 30, 2026
* feat(webhooks): agent schedule webhook triggers (WEBHOOK-001, #291)

Add public webhook URLs so external systems (CI/CD, CRMs, monitoring) can
trigger agent schedule executions via a simple HTTP POST with no Trinity
account required — authenticated by a 256-bit opaque token embedded in the URL.

Changes:
- New public router POST /api/webhooks/{token}: rate-limited (10/60s per
  token), audit-logged, 202 Accepted, delegates to existing scheduler trigger
- JWT-auth CRUD: POST/GET/DELETE /api/agents/{name}/schedules/{id}/webhook
- DB migration: webhook_token (TEXT UNIQUE), webhook_enabled (INTEGER DEFAULT 0)
  on agent_schedules; partial unique index for O(1) token lookup
- Scheduler updated to accept triggered_by param in JSON body so executions
  record triggered_by="webhook" correctly
- Webhook context field framed as data to reduce prompt injection surface
- 12 integration tests in tests/test_webhook_triggers.py

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(security): patch 4 Dependabot alerts — happy-dom + vite (#486)

Bumps two dev-only dependencies to patched versions. Production is
unaffected (happy-dom is test-only; vite only runs in local dev).

- src/frontend: vite ^6.0.6 → ^6.4.2 (closes Dependabot #55, CVE-2026-39363)
- tests/git-sync: happy-dom ^15.11.7 → ^20.9.0 (closes #83/#84/#85:
  VM context escape RCE, ESM code exec, fetch cookie leakage)

Verified: frontend `vite build` clean, all 10 git-sync vitest tests pass
under happy-dom 20.9.0. No new critical/high alerts introduced.

Closes #485

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(webhooks): add WEBHOOK-001 to requirements and architecture (fixes #484 review)

Add missing documentation for the webhook trigger feature:
- requirements.md: WEBHOOK-001 entry with description, key features, DB changes,
  API endpoints, security model, and feature flow link
- architecture.md: webhooks.py listed in Routers table; Schedules table expanded
  from 9 to 12 endpoints with the 3 webhook management endpoints; new Webhook
  Triggers section documenting the public POST /api/webhooks/{token} endpoint;
  webhook_token and webhook_enabled columns added to agent_schedules schema block

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(#488): add status-in-dev label + PR-merge automation (#489)

Close the gap between "PR merged to dev" and "released to main".

- New GH Action `issue-status-on-merge.yml`: on PR merge to dev,
  parse Fixes/Closes/Resolves #N from PR body+title, add
  `status-in-dev`, remove `status-in-progress`.
- `/release` skill: read `gh issue list --label status-in-dev` as
  the authoritative shipping list for release notes; include
  `Closes #N` in the release PR body so issues auto-close on merge
  to main.
- `DEVELOPMENT_WORKFLOW.md`: SDLC is now Todo → In Progress →
  In Dev → Done, each stage mapped 1:1 to commit-graph location.

Fixes #488

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(channels): file upload Phase 2 — workspace delivery hardening (#487) (#494)

Phase 2 of #354 polishes the shared channel-agnostic file delivery path
in `message_router._handle_file_uploads`. Phase 1 (#355) added Telegram
extraction/download/validation; the actual workspace write path was
introduced for Slack inbound (#222). This change hardens the shared path
for both channels:

- New `_sanitize_filename` helper: NFKC unicode normalize → basename →
  safe-chars regex → empty/dotfile fallback to `file_{id}` → 200-char
  truncation preserving extension → collision dedup with `-1`, `-2`, …
- Spec injection format: `[File uploaded by {uploader}]: {name} ({size})
  saved to {path}`. Uploader is the verified email when present
  (Issue #311), else `adapter.get_source_identifier(message)`.
- All-writes-failed handling: when every workspace write attempt fails,
  the router replies on the channel with an explicit error and skips
  agent execution (#487 AC6). Validation rejections (size/MIME/download
  errors) still surface in the description block as before.
- Audit log entries gain an `uploader` field.

Per-session upload directory (`/home/developer/uploads/{session_id}/`)
preserved — keeps user uploads isolated and ephemeral, matches the
existing #222 model.

Tests: +17 unit tests across `TestFilenameSanitization` (12),
`TestFileDeliveryFormat` (2), `TestFileDeliveryFailures` (3). 28/28
passing in `tests/unit/test_file_upload.py`.

Docs: `telegram-integration.md` Phase 2 section + revision row;
`slack-file-sharing.md` flow / router / errors / security sections
updated for the shared change; `feature-flows.md` index row.

Closes #487

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(webhooks): import Request in schedules router (#495)

The WEBHOOK-001 commits (c630931 / 8fdf736) added `request: Request`
parameters to `generate_webhook` and `get_webhook_status` without
importing `Request` from fastapi. Backend module import fails with
NameError on startup, blocking all dev deploys.

Integration tests in tests/test_webhook_triggers.py exercise these
endpoints but never caught the bug because the backend never starts —
test setup fails before any test runs.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(backlog): repair drain spawn — lazy-import target after #95 (#496) (#500)

services/backlog_service.py:240 lazy-imported _execute_task_background
from routers.chat, but #95 deleted that function. Every backlog drain
attempt failed with ImportError; the exception was swallowed at
backlog_service.py:218-228, so BACKLOG-001 (#260) was silently dead.
Live observation: 23 drain failures / 24h on a fan-out workload, only
surface signal was the per-execution `error` column.

Why it shipped silently: the unit happy-path test patched
sys.modules["routers.chat"] with a SimpleNamespace stub of whatever
attribute name it expected, masking the production breakage.

Changes:
- Lazy-import _run_async_task_with_persistence (the post-#95
  replacement) and adjust the call shape (drop release_slot, drop
  orphaned task_activity_id; the unified executor handles both).
- Capture self-task fields (is_self_task, self_task_activity_id,
  inject_result) at enqueue time and rehydrate on drain so
  SELF-EXEC-001 (#264) survives backlog overflow.
- Emit a stable log token `backlog_drain_spawn_failed` so log-based
  detection (Vector / dashboards) can catch import drift or similar
  spawn-time regressions at fleet scale rather than per-row.
- AST-based regression guard in tests/unit/test_backlog.py:
  TestLazyImportTarget parses routers/chat.py and asserts the import
  target exists; paired test asserts the lazy-import string matches
  the validated allow-list. Catches both directions of drift without
  booting the backend.
- Update happy-path test to use the new symbol and kwarg surface;
  add self-task enqueue+drain round-trip tests.

Closes #496

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(announce): add Twitter/X support via API v2 + OAuth 1.0a

Bumps announce skill to v1.6. Adds a Python helper (scripts/post_twitter.py)
that reads tweet text from stdin and posts via Twitter API v2 using OAuth 1.0a
User Context — same exit-0/1 + structured-JSON contract as the existing
Discord/Slack/Telegram send paths so the sequential-only and no-blind-retry
rules apply uniformly. Credentials live in .env (gitignored) under
ANNOUNCE_TWITTER_* keys.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(chat): sync /task long-polls on backlog at capacity (#498) (#515)

Sync parallel `/task` calls (parallel=true, async=false) at capacity used
to fail terminally with HTTP 429 — they never touched the BACKLOG-001
backlog because the spill block was nested under `if request.async_mode:`.
Observed in production: ~40% terminal-failure rate from one MCP fan-out
caller (214 capacity rejections / 24h, 0 enqueues from 541 dispatches).

Sync calls now spill to the same backlog the async path uses and long-poll
on the open HTTP connection until the queued execution reaches a terminal
status, then return the result inline. True 429 only when the backlog is
also full. Total connection hold capped at 2 × effective_timeout.

Implementation:
- New `services/sync_waiter.py` owns the in-process registry and the
  `signal_sync_waiter` / `wait_for_sync_terminal` primitives. Wait combines
  an asyncio.Future (set by the drain finally block) with a 5s DB-poll
  fallback that covers terminal flips routed outside the drain
  (corrupt-metadata, expire_stale, cleanup recovery).
- `routers/chat.py` sync branch now mirrors the async branch:
  pre-acquires the slot, on at-capacity calls `backlog.enqueue()` then
  `wait_for_sync_terminal()`, returns the inline result on wake.
- `_run_async_task_with_persistence` wraps its body in try/finally and
  signals any registered sync waiter with the rich TaskExecutionResult
  plus chat_session_id. No-op when no waiter is registered (the common
  async fire-and-forget path).

Tests (`tests/unit/test_chat_sync_backlog.py`, 13 new):
- Signal / wait / poll-fallback / timeout / cleanup / concurrent waiters
- Regression test pins TERMINAL_TASK_STATUSES to the enum so a new
  TaskExecutionStatus value forces a deliberate update (caught a missing
  SKIPPED entry pre-merge)

Trade-off (Policy B): worst-case connection hold doubles to
2 × effective_timeout when the request is queued. Honest envelope —
the caller chose to wait. Documented in the architecture diagram of
`persistent-task-backlog.md`.

Companion issue #505 covers the orchestration-education gap (MCP tool
description + platform prompt) so agents pick the right tool for the
job rather than relying on the platform absorbing every misuse.

Closes #498

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(groom): document SDLC stages and add status-label/board reconciliation

Adds SDLC context (Todo → In Progress → In Dev → Done) so grooming respects
in-flight work, and a Step 1b that reconciles status-* labels with board
columns (labels are authoritative).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(agent): classify signal-killed claude exits as 504, not fake auth failure (#517)

External signal terminations of the claude subprocess (timeout SIGKILL,
OOM-kill, parent SIGTERM, operator cancel) used to fall through to the
auth-fallback heuristics and surface as a misleading "Subscription token
may be expired" 503. Same shape as #361 (max-turns), different exit path.

Adds _classify_signal_exit() consulted before the auth heuristics: matches
Python-native signal exits (return_code < 0) and shell-encoded forms
(130/137/143 for SIGINT/SIGKILL/SIGTERM) and raises HTTP 504 with a clear
"killed by SIGKILL/SIGTERM/SIGINT — likely timeout, OOM, or operator
cancel" message. Tightens the zero-token heuristic with return_code > 0
so signal exits cannot reach it.

The bug became routinely reproducible after #61 (PR #326) added
backend-driven terminate_execution_on_agent() — every timeout now
produces a signal-killed claude subprocess on the agent side, which the
old heuristic block misclassified. Also de-risks PR #508 (auth-class
auto-switch): without this fix, every timeout would trigger an
unnecessary subscription rotation.

Backend's task_execution_service.py only flags AUTH on 503; 504 falls
through to the generic FAILED path. No backend changes required.

Closes #516

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(sprint): align skill with DEVELOPMENT_WORKFLOW.md (#519)

Four divergences between the /sprint playbook and the SDLC documented in
docs/DEVELOPMENT_WORKFLOW.md:

- Step 3 used `gh issue edit --add-label status-in-progress` directly,
  bypassing .github/workflows/claim.yml and skipping self-assignment.
  Now posts `/claim` as an issue comment, which is the workflow's single
  source of truth for the In Progress transition.
- Step 8 invoked pytest directly via `cd tests && source .venv/bin/activate
  && python -m pytest …`. Now defers to `/test-runner [feature]`, with a
  documented fallback for brand-new files outside the runner's catalog.
- Step 10 commit + PR body used `closes #N`. Workflow §1 specifies
  `Fixes #N`; both auto-close on GitHub but the doc is the contract.
- Step 11 final report didn't mention the post-merge automation. Now
  warns that issue-status-on-merge.yml owns the
  status-in-progress → status-in-dev transition, so operators don't
  manually edit labels post-merge.

Non-breaking: argument signature, automation level (gated), state
dependencies, and pipeline overview unchanged. Net +19/-11.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(agent): classify clean-exit empty-result as 502, not silent success (#520) (#521)

* fix(agent): classify clean-exit empty-result as 502, not silent success (#520)

Sibling of #516/#517 on the return_code == 0 path. When the claude
subprocess exits 0 but the final {"type":"result"} JSON line is dropped
before the reader thread captures it (typical cause: a child subprocess
inherited stdout, kept the pipe open past claude exit, the reader thread
leaked, the pgroup unwind closed the pipe), metadata.cost_usd and
metadata.duration_ms stay None. The success path used to return HTTP 200
anyway — agent-server logged "completed successfully" while backend
silently reaped the execution as an orphan minutes later, masking the
real failure with a misleading "completed on agent but recovered by
watchdog" message.

Adds _classify_empty_result(metadata, raw_message_count) consulted after
the return_code != 0 block (#516 + auth heuristics) and before response
building. When both cost_usd and duration_ms are None, raises HTTP 502
with diagnostic context (tools, turns, raw_messages, cause hint).
Backend's task_execution_service.py:542 only flags AUTH on 503, so 502
falls through to the generic FAILED path with the helpful detail
preserved — no backend changes needed.

The two-field check is conservative: single-field nullability could be a
Claude format quirk; both-None is a strong signal that the terminal
result message never arrived. Test coverage pins the scope so a future
edit can't silently broaden it.

Changes:
- docker/base-image/agent_server/services/claude_code.py — new
  _classify_empty_result() helper next to _classify_signal_exit; call
  site between the return_code != 0 block and response building.
- tests/unit/test_empty_result_classification.py — 9 new tests, all
  pass. Covers both-None → 502, populated metadata → None,
  single-field-only → None (Claude format quirk tolerance), zero-cost
  and zero-duration → None (is None vs falsy), missing metadata → None.
- docs/memory/feature-flows/parallel-headless-execution.md — changelog
  entry under Recent Updates.
- docs/memory/feature-flows/task-execution-service.md — row in
  error-translation table + new Empty-Result Pre-Check paragraph.

Requires base-image rebuild after merge:
./scripts/deploy/build-base-image.sh

Closes #520

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(feature-flows): index entry for agent error classification (#516, #520)

Combined Recent Updates entry covering the matching pair of agent-side
error-classification fixes that shipped this week — _classify_signal_exit
(#516, PR #517) and _classify_empty_result (#520, PR #521). Both touch
docker/base-image/agent_server/services/claude_code.py and share the
"agent surfaces the right HTTP status so backend records FAILED with a
useful detail" theme.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(agent): off-load synchronous terminate cleanup off the event loop (#523)

The async terminate_execution endpoint and the outer asyncio.TimeoutError
handlers in execute_claude_code and execute_headless_task were calling
registry.terminate() / _terminate_process_group() / _safe_close_pipes()
synchronously. Those helpers do up to 7s of process.wait() (SIGINT grace
+ SIGKILL grace), which blocks the asyncio event loop for the entire
window. While blocked, agent-server cannot serve /health, the backend
circuit breaker opens, and UI fan-out hangs for 5+ minutes per page.

This is the actual user-visible mechanism behind the #523 "agent-server
wedge" symptom, not the FD-inheritance / leaked-reader-thread theory in
the original report (see issue comment for the corrected diagnosis —
the FD_CLOEXEC fix as written would not have helped because dup2 strips
CLOEXEC during the child's stdout setup, and the existing post-#407
killpg + safe_close path actually works in the vast majority of cases).

Wrap the three call sites in loop.run_in_executor(None, ...) so the
blocking process.wait() runs on a thread-pool worker. Pipe-inheritance
fragility remains a slow-burn cleanup item to be filed separately.

- routers/chat.py: terminate_execution dispatches registry.terminate to
  the default executor
- services/claude_code.py: outer-timeout cleanup in both async paths
  off-loads _terminate_process_group + _safe_close_pipes
- tests/unit/test_terminate_async_executor.py: regression test asserts
  registry.terminate runs on a non-event-loop thread and the event-loop
  yield stays sub-50ms while terminate is in flight

Fixes #523

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(planning): add Tier 2.6 hardening + actor-model destination roadmap

Records the architectural critique from 2026-04-26 review:
- Tier 2.6 (Sprint D′): #524 state machine contract, #525 idempotency
  keys, #526 dispatch circuit breaker. Closes the three contract-level
  gaps that survive even after Sprint D's plumbing consolidation.
- Future considerations: 7 unranked recommendations (durable
  ProcessRegistry, retry-in-funnel, synchronous terminate ack, dual
  streams, fairness, EventBus backpressure, lifecycle contract doc).
- Target architecture section: names the actor model as the destination
  (mailbox + journal + processor), maps existing components to the
  concepts they already implement, defines a 4-phase gated transition
  roadmap, and gates Phase 2 (agent-to-agent experiment) on a one-page
  message-envelope + journal-format postcard.

Issues: #524, #525, #526 created and added to project board (Epic
#411 Orchestration Invariants, Theme Reliability).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(planning): mark #291 (WEBHOOK-001) shipped, Sprint C now 5/5

#291 closed 2026-04-24, shipped via PR #484 (token-in-URL trigger
through TaskExecutionService) with follow-up fix PR #493. The plan
doc still listed it as the next item to pick up; align it with the
ground truth and re-aim "what to do next" at #428 (after #306 soak)
plus Tier 2.6 hardening (#524/#525/#526) in parallel.

* refactor(capacity): consolidate three queue/slot primitives into CapacityManager (#428) (#527)

* refactor(capacity): consolidate ExecutionQueue + SlotService + BacklogService into CapacityManager (#428)

Single public facade for agent execution capacity. Composes SlotService
(Redis ZSET counter) and BacklogService (SQL persistent overflow) as
private internals; owns the in-memory overflow store (Redis LIST, depth 3,
lifted from the deleted ExecutionQueue).

Why:
- 7 caller sites now go through one API instead of orchestrating three.
- Each new trigger type (retry, webhook, self-exec, fan-out) gets one path
  for capacity, not a choice between three primitives.
- Unblocks #429 (CLEANUP-COLLAPSE) and the actor-model destination by
  reducing the surface a single capacity store has to expose.

API:
  capacity.acquire(agent, exec_id, max_concurrent, *,
                   overflow_policy='reject'|'queue_in_memory'|'queue_persistent',
                   overflow_payload=PersistentTaskPayload(...))
  capacity.release(agent, exec_id)        # idempotent
  capacity.release_if_matches(agent, eid) # TOCTOU-safe (watchdog)
  capacity.get_status(agent, max_concurrent)
  capacity.reclaim_stale(agent_timeouts)  # called by cleanup_service
  capacity.force_release(agent)           # emergency
  capacity.cancel_all_overflow(agent, reason)  # agent deletion
  capacity.run_maintenance(max_age_hours)  # 60s tick from main.py

Wire format unchanged: same Redis keys (agent:slots:*, agent:queue:*),
same SQL columns (schedule_executions.queued_at, backlog_metadata).
In-flight executions unaffected; clean revert path.

Deviations from issue spec (user-approved):
- No feature flag — single runtime path. dev-soak + clean revert is the
  rollback mechanism, simpler than a per-agent DB column + flag check at
  every call site.
- ExecutionQueue deleted in this PR rather than separate cleanup PR.
  SlotService and BacklogService kept as private internals (well-factored,
  one job each).

Soak deviation: shipped after 5 days of #306 soak rather than the planned
14 days. Mitigated by additive-style refactor (no wire-format change).

Files:
- NEW services/capacity_manager.py (~480 LOC)
- DELETE services/execution_queue.py (~360 LOC)
- 7 caller migrations: routers/chat.py (4 sites), routers/agents.py (2),
  routers/agent_config.py (1), services/cleanup_service.py (4),
  services/task_execution_service.py (1), services/agent_service/queue.py (3),
  main.py (1, callback wiring is now internal).
- NEW tests/unit/test_capacity_manager.py — 21 tests covering acquire/release
  for all three overflow policies, drain wiring, status, force_release,
  reclaim_stale, cancel_all_overflow.
- UPDATE tests/test_watchdog_unit.py — 11 mock decorator pairs collapsed to
  single get_capacity_manager mock.

Tests: 21 new + 35 watchdog + 33 backlog = 89 green for affected surface.

Fixes #428

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(feature-flows): add capacity-management.md, deprecate predecessor flows (#428)

- NEW capacity-management.md — public surface, overflow policies, end-to-end
  /chat and /task flows, storage map, maintenance & recovery, what-replaced-what.
- DEPRECATE notes on the three predecessor flows with redirects:
  - execution-queue.md (ExecutionQueue deleted)
  - parallel-capacity.md (SlotService internalized)
  - persistent-task-backlog.md (BacklogService internalized)
- "Now uses CapacityManager" notes on four downstream flows:
  - task-execution-service.md, parallel-headless-execution.md,
    cleanup-service.md, execution-termination.md
- Index: Recent Updates row + Core Agent Features row for capacity-management.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(requirements): note BACKLOG-001 is now internal to CapacityManager (#428)

Section 10.8 (Persistent Task Backlog) — replace direct SlotService callback
reference with the unified CapacityManager facade. Status bumped with the
2026-04-26 internalization date and #428 cross-ref.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(agent): drain pipe before close to preserve final result line (#531) (#532)

* fix(agent): drain pipe before close to preserve final result line (#531)

drain_reader_threads previously called safe_close_pipes() immediately
after terminate_process_group(), discarding the kernel pipe buffer before
the reader thread could drain it. On long agentic tasks the final
{"type":"result"} JSON line (cost, duration, answer) was in that buffer
at the moment of close, causing the reader to raise ValueError and
metadata.cost_usd / duration_ms to remain None — triggering the HTTP 502
"Execution completed without a result message" classification from #521.

Fix: reorder so grandchildren are killed first, then the reader is given
post_kill_grace=30s to drain naturally (grandchildren dead → kernel
delivers EOF once the buffer is consumed → reader returns '' and exits
cleanly). safe_close_pipes() is now a true last resort — only called when
the reader is still alive after 30s, which indicates a genuine wedge, not
unfinished backlog drain.

Also extends _classify_empty_result to derive num_turns from raw_messages
when metadata.num_turns is None (result line lost), so the 502 detail
reports an honest turn count instead of always showing 0.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs(tests): update test catalog for #531 drain_reader_threads fix

- Add test_subprocess_pgroup.py and test_empty_result_classification.py
  to Test Categories (Operations & Observability, unit section)
- Add 2026-04-27 Recent Test Additions entry with description of the
  pipe-drain ordering regression tests and raw_messages fallback tests
- Update unit test count: 165 → 170; total: 2,257 → 2,262

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs(feature-flows): document drain_reader_threads pipe-ordering fix (#531)

Update parallel-headless-execution.md with the root cause fix for
the "Execution completed without a result message" HTTP 502: the old
drain_reader_threads sequence closed the pipe before the reader could
drain the kernel buffer (including the final result JSON line). New
sequence: kill grandchildren → natural drain (post_kill_grace=30s) →
force-close only as last resort.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(tests): update orphaned-recovery mocks to CapacityManager (#533) (#534)

Replace stale services.slot_service sys-mock with services.capacity_manager
so all four recovery scenario tests pass after the #428 consolidation.
Assertions updated from release_slot → release to match the new API.

Fixes #533

Co-authored-by: Claude <noreply@anthropic.com>

* docs(skills): align validate-pr with DEVELOPMENT_WORKFLOW.md

Add quick triage block, base branch check, PR size warning, type-docs
label, Base Branch/PR Size rows in report table, and review pipeline
matrix linking /review and /cso --diff with their complementary roles.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(validate-architecture): stale-citation filter + dedupe guard (#511) (#513)

* fix(validate-architecture): add stale-citation filter + issue dedupe guard (#511)

The /validate-architecture skill produced false-positive issue #479 by:
1. citing file paths the report's snapshot saw, but `main` no longer has
   (process engine deleted in #430 the same day);
2. running `gh issue create` with no check for existing open issues with
   the same finding fingerprint.

Two targeted edits to .claude/skills/validate-architecture/SKILL.md:

- New Step 2c "Filter Stale Citations" — `git ls-files --error-unmatch`
  every cited path before report. Drop ghosts. Downgrade FAIL → PASS
  when an invariant has zero remaining real citations.

- Modified Step 4 — fingerprint = sorted invariant numbers; query
  open `automated,priority-p1` issues with `--search "in:body
  validate-architecture fingerprint=<fp>"`; comment on existing
  issue instead of creating duplicate. Issue body now stamps the
  current commit SHA for evidence binding.

Closes #511.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(validate-architecture): clarify dedupe branching, distinct fingerprint marker, quote paths (#511)

Follow-up to review feedback on PR #513. Three small skill-prose
hardenings:

- I2 (LLM-driven flow control): the dedupe branch previously relied on
  `if [ -n "$EXISTING" ]; then ...; exit 0; fi` followed by a separate
  create block. `exit 0` halts a bash subshell, not an LLM walking the
  markdown — a future runner could execute both blocks. Replace with
  explicit "Path A — COMMENT, then STOP" / "Path B — CREATE" prose
  branching and an explicit DO-NOT note.

- I4 (fingerprint collision): replace free-text body search
  `validate-architecture fingerprint=$FP` with HTML-comment marker
  `<!-- validate-architecture::fingerprint=$FP -->` plus a quoted-phrase
  search. Self-evidently programmatic; won't collide with prose.

- I3 (path quoting): the Step 2c example now uses `"$path"` and a note
  about shell metachars, so implementers don't strip the quotes.

- Add concurrency caveat documenting that the dedupe is best-effort,
  not atomic (no GitHub primitive provides this).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(config): clean stale Auth0 / AUDIT_URL, document SMTP/SendGrid/FRONTEND_URL (#481) (#509)

- Remove dead Auth0 env vars + build args from docker-compose.prod.yml
  and docker/frontend/Dockerfile.prod (Auth0 removed 2026-01-01). The
  build-arg fallbacks were also leaking a real Auth0 domain + client ID
  into a public repo.
- Drop AUDIT_URL from .env.example (audit-logger service no longer exists;
  no Python references it).
- Add FRONTEND_URL to .env.example (required in prod for OAuth post-auth
  redirects in slack_service.py / public_links.py and SSH host
  auto-detection in ssh_service.py).
- Document SMTP_HOST/PORT/USER/PASSWORD and SENDGRID_API_KEY in
  .env.example so the advertised EMAIL_PROVIDER=smtp/sendgrid modes are
  actually configurable from the template.

Closes #481

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(auth): require auth on /api/docs endpoints (#452) (#507)

Add Depends(get_current_user) to the three handlers in
src/backend/routers/docs.py so the file no longer violates
Architectural Invariant #8.

Note: this router is not currently registered in main.py, so
the endpoints are not reachable on the running API. The fix is
applied to the file as written so the invariant validator stops
flagging it and so the file is correct if it is ever remounted.

Closes #452

* fix(chat): wrap long unbroken strings in chat bubbles (#457) (#502)

Long URLs, tokens, base64 blobs, and other unbroken strings in agent
chat responses were overflowing their 85% bubble and forcing horizontal
scroll on the entire Chat tab.

Root cause: ChatBubble.vue capped the bubble width but never told
inner content how to handle unbreakable strings. Inline <code> and
the user-text <p> had no overflow-wrap; <pre> defaulted to white-space:
pre with no overflow-x: auto override.

Fix (CSS-only, all 3 render branches — user / self-task / assistant):
- min-w-0 on outer wrapper, overflow-hidden on inner bubble
- break-words on user text and prose container
- prose-pre:overflow-x-auto + prose-pre:max-w-full so code blocks
  scroll inside the bubble instead of expanding it
- prose-code:break-words for long inline tokens
- prose-a:break-words for long URLs in markdown links

Verified visually: before/after static test page shows BEFORE leaks
content well past the bubble border; AFTER wraps cleanly with no
regression on normal markdown (headings, lists, links, short code).

* fix(schedules): add missing Request import for webhook endpoints (#493)

Regression from c630931 (WEBHOOK-001 / #291): `routers/schedules.py`
uses `Request` as a type annotation on the `generate_webhook` and
`trigger_webhook` handlers but never imports it, so the module fails
to load and the backend won't start with a NameError.

Minimal fix: add `Request` to the existing `from fastapi import …`
line (line 11). No behavioral change — the annotation was already
intended.

Surfaced while dev-testing FILES-001 (PR #491). uvicorn reload pulled
in the dev branch state and blew up.

Co-authored-by:  Pavlo <pash@pashs-MacBook-Pro.local>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(git): route orphan cleanup through db.delete_git_config (#451) (#501)

Replaces raw `DELETE FROM agent_git_config` SQL in routers/git.py with the
existing `db.delete_git_config()` method (already used at line 435 of the same
file for the init-failure rollback path). Restores Architectural Invariant #1
(Three-Layer Backend) for this router. No behavior change — identical SQL,
identical parameter binding.

Closes #451

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(subscription): auto-switch on first failure + auth-class triggers (#441) (#508)

Drop the 2-consecutive-429 gate in `subscription_auto_switch` so a single
subscription failure now triggers a switch — the 2h skip-list on
alternative selection (already pinned by #444 / #476 regression tests) is
sufficient as the lone thrash guard. Broaden the trigger surface to also
fire on auth-class failures (401/403/credit balance/expired OAuth token,
etc.), classified via a centralized `AUTH_INDICATORS` list, so a broken
subscription auto-recovers instead of failing every execution until
manual intervention. Flip the `auto_switch_subscriptions` default to
"true" — operators can still opt out, but the safe behavior is now the
default. Backward-compat shim `handle_rate_limit_error` preserved for
existing 429 callers.

- services/subscription_auto_switch.py: new `handle_subscription_failure`
  with `failure_kind` dispatch ("rate_limit" | "auth"); new
  `is_auth_failure` classifier; default flipped; notification + log
  wording adapts per kind; old shim retained.
- services/task_execution_service.py: 503 / auth-classified errors now
  also call the switch path alongside 429.
- routers/chat.py (sync): same broadening on the interactive chat
  surface; auth path returns 503+retry hint mirroring the 429 UX.
- routers/subscriptions.py: GET `/auto-switch` default also flipped to
  "true" so the UI toggle and runtime gate read the same value.
- scheduler/service.py: dedupe two inline `auth_indicators` copies into
  a single module-level constant; cross-reference the canonical list in
  backend (cross-container import not viable).
- tests/unit/test_subscription_auto_switch_pingpong.py: new
  TestIsAuthFailure + TestSingleEventThreshold classes (8 new tests, all
  pingpong + #476 aging tests still green).
- tests/test_subscription_auto_switch.py: flip default-off → default-on.
- docs: SUB-003 feature flow + requirements doc reflect the new
  threshold, broadened scope, and on-by-default behavior.

Closes #441

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(feature-flows): clean up #95 drift missed by #500 (#496) (#503)

* fix(backlog): repair drain spawn after #95 rename (#496)

`services/backlog_service.py:_spawn_drain` was lazy-importing
`_execute_task_background` from `routers.chat`, but #95 (PR #316) deleted
that function and replaced it with `_run_async_task_with_persistence`.
Every backlog drain raised `ImportError`, was caught at line 218-228, and
silently marked queued executions FAILED — leaving BACKLOG-001 (#260)
non-functional whenever an agent hit capacity.

Rewire the lazy import to the new helper and adjust the call shape:
- drop `task_activity_id` (not in new signature; chat router already
  passes None at enqueue)
- drop `release_slot=True` (the wrapper passes `slot_already_held=True`
  to TaskExecutionService, which manages release in its finally block)
- derive `is_self_task` from x_source_agent vs agent_name
- pass `self_task_activity_id=None` (queued items don't carry one;
  separate gap, not in scope here)

Add `tests/test_backlog_drain_unit.py` with five regression checks:
two AST-based contract tests that pin the function name and signature
in `routers/chat.py` (would have caught the original break), and three
runtime spy tests covering the kwarg shape `_spawn_drain` forwards. The
existing `tests/unit/test_backlog.py::test_drain_happy_path_spawns_background`
is updated to match the new contract.

Sync the BACKLOG-001 and TaskExecutionService feature-flow docs to
reference the renamed helper.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(feature-flows): sync index + parallel-capacity for #496

- Add #496 entry to feature-flows.md Recent Updates.
- Fix two more stale `release_slot=True` references in
  parallel-capacity.md left over from #95 — the param never existed
  on `_run_async_task_with_persistence` (slot release happens inside
  TaskExecutionService via slot_already_held=True).

Other stale `release_slot=True` references in
authenticated-chat-tab.md and parallel-headless-execution.md are
deeper drift (separate flows, not touched by #496) — leave for a
follow-up doc-cleanup pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(catalog): register test_backlog_drain_unit.py (#496)

Adds the new BACKLOG-001 regression test file to tests/registry.json
so it shows up in the catalog alongside test_event_bus.py and
unit/test_backlog.py.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test: drop redundant test_backlog_drain_unit.py

PR #500 (which superseded the original #496 fix scope) shipped
equivalent contract coverage with a more robust setup:
- `TestLazyImportTarget` (AST guard for the lazy-import target)
- `test_drain_threads_self_task_fields` (round-trip via real
  BacklogService against sqlite)

The local file used sys.modules stubs which were strictly weaker.
Keeping it would only add maintenance burden for duplicate coverage,
so drop the file and its registry entry. Net effect on PR #503 is
that it becomes a small, focused docs-cleanup PR (parallel-capacity.md
and task-execution-service.md drift from #95, plus the missing
Recent Updates entry for #496).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(deploy): align scripts, configs, and docs with production operating patterns (#504)

* docs(generate-user-docs): add hub-and-spokes deployment structure and ops-pattern import

Restructure skill to produce guides/deploying/ as a hub plus six spokes
(local-development, single-server, public-access, upgrading,
backup-and-restore, monitoring) instead of one flat deploy guide.

Add an "operational guide" template (When to Run → Pre-flight →
Procedure → Verify → Rollback) for procedural docs that don't fit the
feature-shaped dual-audience template, plus verbatim-reuse snippets for
the load-bearing rules: never down/up, rebuild platform services only,
six-probe verification, resource-thresholds table, alpine cp backup.

Add Step 2h to draw operational patterns from the private ops runbook
under ../trinity-ops/, with explicit safe/forbidden import lists and a
sshpass→localhost rewrite rule.

Strengthen Step 2e to cross-check .env.example keys against each
compose's environment block — docs must not promise behavior the
chosen compose can't deliver.

Add public-safety greps in Step 7 (sshpass, trinity-ops, tailnet, real
IPs, instance-dir refs) so leaked private detail blocks completion.

Tracks issue #504.

* fix(deploy): align scripts, configs, and docs with production operating patterns (#504)

Fixes the first-run blocker (agent creation fails silently without base
image) and removes references to the removed audit-logger service that
caused verify-platform.sh and validate.sh to always fail.

Scripts:
- start.sh: detect missing base image and auto-build on first run; use
  `docker compose stop` in help text (not `down`, which destroys agents)
- verify-platform.sh: full rewrite — remove trinity-audit-logger and port
  8001 audit checks; fix frontend from port 3000 → 80; check scheduler
  health at :8001; add MCP/Vector probes; fix login hint
- validate.sh: remove non-existent `deployment/` dir, `QUICK_START.md`,
  and `src/audit-logger/audit_logger.py` from required paths; fix port 3000

Config:
- docker-compose.yml: wire 5 missing env vars into backend (PUBLIC_CHAT_URL,
  FRONTEND_URL, EXTRA_CORS_ORIGINS, SLACK_SIGNING_SECRET, SSH_HOST)
- .env.example: remove stale AUDIT_URL; annotate prod-only / overlay-only
  vars (SLACK_SIGNING_SECRET, PUBLIC_CHAT_URL, FRONTEND_URL, SSH_HOST,
  TRINITY_GIT_BASE_URL) so users know scope before setting them

Docs:
- deploying-trinity.md: add explicit build-base-image.sh step; fix
  /trinity:connect to use MCP API key flow (not username/password); add
  Upgrading, Health Verification, Resource Thresholds, and Common Recovery
  Patterns sections from ops runbook; use `docker compose` (v2 syntax)
- setup.md: remove false claim that start.sh builds the base image; correct
  admin account creation (env var driven, not wizard); clarify wizard path
  (used only when ADMIN_PASSWORD is unset)

New file:
- quickstart.sh: interactive one-command setup (checks Docker, generates
  secrets, sets ADMIN_PASSWORD, builds base image, starts services, verifies)

Skill:
- generate-user-docs: add deployment config reading rules (read scripts
  literally; cross-check env vars vs compose; never claim "auto" unless code
  proves it); add operational guide template (pre-flight/steps/verify/rollback);
  resolve conflict preserving the hub+spokes guide structure from branch

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(generate-user-docs): remove private repo name from SKILL.md

Replace explicit `trinity-ops` repo references with generic path aliases
(`../ops-runbook/`) so the private repo name is not embedded in this
public repository.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(reliability): CAS guards on execution status writes + state machine doc (#524) (#541)

Closes the FAILED→SUCCESS and SUCCESS→FAILED races that were patched by
#378 re-verify logic without eliminating the root cause.

Changes:
- update_execution_status: SUCCESS writes are unconditional (agent wins);
  non-success terminal writes blocked when row already terminal
- mark_stale_executions_failed / mark_no_session_executions_failed: inner
  UPDATE gains AND status='running' to close the SELECT→UPDATE TOCTOU window
- _recover_execution: routes through mark_execution_failed_by_watchdog
  (already CAS-guarded) instead of bare update_execution_status
- TaskExecutionStatus: state machine, transitions, and authorized writers
  documented in docstring; PENDING_RETRY added to enum
- Remove now-dead _STALE_SLOT_ERROR_PATTERN constant

Full projector architecture (ExecutionStateProjector, agent event emission,
projected_status shadow column) deferred — agents have no Redis access and
the restart-recovery design needs more thought before those land.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(public-chat): build context before storing user message to prevent duplication (#539) (#540)

* fix(public-chat): build context before storing user message to prevent duplication (#539)

In the public chat endpoint, the user message was persisted to the database
before build_public_chat_context read from it, causing the current message
to appear twice in every agent prompt — once in "Previous conversation:"
and once in "Current message:". Reordering the calls so context is built
first (from prior history only) then the user message is stored eliminates
the duplicate on every turn.

Adds unit tests that document both the old broken order (two occurrences)
and the corrected order (one occurrence), guarding against regression.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs(feature-flows): update public-agent-links with #539 context ordering fix

- Correct PUB-005 data flow: build_public_chat_context before add_public_chat_message
- Update backend implementation step ordering to match fixed code
- Add revision history entry for the bug fix
- Add #539 entry to feature-flows.md index

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(agent-runtime): guard content_block isinstance in process_stream_line (#542) (#543)

Prevents AttributeError crash when Claude Code stream-json emits a
string element inside a message content array. Guards both
process_stream_line (real-time path) and parse_stream_json_output
(batch path) using the same isinstance(block, dict) pattern already
used by the error_content loop.

Fixes #542

Co-authored-by: Claude <noreply@anthropic.com>

* docs(#411): Phase 1 canary harness design + catalog Phase 1 subset additions (#544)

* docs(#411): Phase 1 canary harness design + catalog Phase 1 subset additions

- New design doc at docs/planning/CANARY_HARNESS_PHASE_1.md scoping the
  AC-required infrastructure (snapshot collector, canary_violations table,
  canary agent template, fleet, alerts) for the three required invariants
  (S-01, E-02, L-03).
- Catalog Phase 1 subset expanded 10 → 12: adds S-03 (slot TTL ≥ exec
  timeout, catches #226) and E-05 (dispatched rows have session, catches
  #106), since both bugs are cited in the catalog motivation but had no
  Phase 1 detector.

* docs(#411): scope fleet to strict minimum for AC's 3 invariants

* docs(#411): expand design doc to cover full Phase 1 (12 invariants, snapshot format)

* feat(settings): add Remove buttons for stored API keys + Slack (#459) (#483)

Settings page lets admins save/test Anthropic API Key, GitHub PAT, and
Slack OAuth credentials, but exposed no UI to clear them once stored.
Only workaround was calling DELETE endpoints directly or editing the DB.

Adds Remove buttons next to Save in each row, conditionally rendered
when the value lives in settings DB (source === 'settings'). Env-var
fallbacks stay uneditable from UI. Confirm dialog before deletion
(reuses ConfirmDialog component + pattern from ApiKeys.vue).

Backend DELETE endpoints already existed — no backend work:
- DELETE /api/settings/api-keys/anthropic
- DELETE /api/settings/api-keys/github
- DELETE /api/settings/slack

Audit of other Settings sections: Trinity Prompt has clearPrompt,
Skills Library blanks via deleteSetting, MCP URL has resetMcpUrl,
GitHub Templates/Email Whitelist have inline remove. Agent Quotas are
config values, not secrets.

Closes #459.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(migrations): swallow duplicate-column race on cold start (#456) (#537)

* fix(migrations): swallow duplicate-column race on cold start (#456)

`_migrate_sync_health` (#389) used a check-then-act PRAGMA → ALTER
pattern that is not atomic across uvicorn workers. On cold start with
`--workers 2`, both workers passed the PRAGMA before either committed
the ALTER, and the loser crashed its child process with
`sqlite3.OperationalError: duplicate column name: auto_sync_enabled`.

Fix:
- Add `_safe_add_column` helper that swallows the duplicate-column
  OperationalError (treats it as success — another worker won the race).
  Future migrations should route ALTER TABLE ADD COLUMN through it.
- Refactor `_migrate_sync_health` to use the helper for both column
  additions and switch the bare `CREATE TABLE` to `CREATE TABLE IF NOT
  EXISTS` (atomic in SQLite).

Tests:
- `test_safe_add_column_swallows_duplicate_column_race` — drives the
  exact production race via a PRAGMA-lying cursor proxy.
- `test_safe_add_column_propagates_other_errors` — non-duplicate errors
  still raise.
- `test_safe_add_column_returns_true_when_added` — happy path.
- `test_migrate_sync_health_idempotent_under_race` — `_migrate_sync_health`
  is now safe to re-run on already-migrated schemas, including under
  the simulated race.

The other ~50 ALTER ADD COLUMN sites are untouched: they're already
applied on production DBs (run_all_migrations short-circuits via the
schema_migrations tracking table). The race only bites new migrations
on first cold-start; the helper is available for them.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(migrations): route all ALTER ADD COLUMN through _safe_add_column (#456)

Mechanical sweep of every check-then-act `PRAGMA table_info` →
`ALTER TABLE ADD COLUMN` site through the `_safe_add_column` helper, so
new migrations on a fresh cold-start with `--workers N` are race-safe by
default — not just `_migrate_sync_health` (the originally reported case).

22 migrations refactored. Bare `try/except Exception` swallows in
`_migrate_chat_messages_source_column` and
`_migrate_agent_ownership_voice_prompt` are also replaced with the
helper, which catches only the duplicate-column error instead of every
exception class.

Verification:
- Schema dump (init_schema + run_all_migrations on fresh DB) is
  byte-identical before and after the sweep — every column type,
  default, FK, and index preserved.
- run_all_migrations is idempotent across runs and across fresh
  connections (2nd/3rd runs print no add/create lines).
- 6-worker concurrent stress test (`threading.Barrier`-coordinated)
  completes without any worker crashing; final schema is intact.
- tests/unit/test_migrations_concurrent.py +
  tests/unit/test_migrations.py + tests/unit/test_guardrails.py:
  72 pass, 0 fail.

`tests/unit/test_guardrails.py::test_migration_is_idempotent` updated
to also exec the `_safe_add_column` helper into its isolated
namespace, since the migration now delegates to it.

The two remaining bare `CREATE TABLE` calls in the file
(`_migrate_agent_sharing_table`, `_migrate_agent_skills_table`) are
one-time DROP+CREATE data-recreation migrations that already shipped
on every existing install; they are out of scope for this sweep.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(git): UI Push no longer commits runtime state (#462)

Expands the platform .gitignore deny-list to cover all runtime files
(.env, .mcp.json, .credentials.enc, instance dirs, content/, Claude
Code state, temp files). Adds idempotent migration that updates
existing agents on next Push and calls `git rm --cached` for files
that are now tracked but newly ignored.

Closes #462

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(frontend): semantic status color tokens (#67) (#553)

Introduce 5 semantic status tokens (`status-success/warning/danger/info/urgent`)
in tailwind.config.js as direct aliases of the green/yellow/red/blue/orange
palettes, then migrate 9 frontend files (4 components, 2 panel-local helpers,
1 composable, 1 utility) from raw color classes to the new tokens. Visual
output is byte-equivalent — tokens compile to identical RGB values.

Add CI safety net: `npm run check:tokens` script verifies token-palette
equivalence and catches typo'd token references in source. Wired into a new
frontend-build.yml workflow that runs `npm ci → check:tokens → build` on PRs
touching `src/frontend/**`.

Drive-by fix: rename postcss.config.js → postcss.config.mjs to fix Node
ESM/CJS interop for local `npm run build` (production Docker build was
masking the issue).

70+ raw-color files remain for follow-up sweep (per autoplan phasing).

Fixes #67

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(files): FILES-001 outbound file sharing — MVP + Phase 1 hardening (#491)

* feat(files): FILES-001 outbound file sharing MVP (Steps 1-6)

Implements outbound file sharing per docs/drafts/amazing-file-outbound.md:
- Schema + migration (agent_shared_files table with FK cascade)
- Per-agent opt-in toggle + Docker publish volume (agent-{name}-public)
- Internal share endpoint with path/MIME/size/quota validation
- Public download endpoint (/api/files/{id}?sig=...) with token auth
- share_file MCP tool (agent-scoped)
- SharingPanel UI: toggle, list, revoke, copy URL

Live-verified on Slack: agent→share_file→URL→download end-to-end.
Unit tests: 33 passed (migration, mixin, mount-match).
Known limitations + production readiness plan:
  docs/drafts/amazing-file-outbound-production-readiness.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(files): FILES-001 — requirements, architecture, feature-flow doc

- requirements.md §13.10 new entry marking FILES-001 Implemented (2026-04-24)
- architecture.md: add files.ts MCP module, agent_shared_files_service,
  routers/files.py, the 5 new API endpoints + dedicated section, and the
  agent_shared_files table schema + operational notes
- feature-flows.md: Recent Updates entry + Documented Flows index
- feature-flows/file-sharing-outbound.md: new full vertical-slice doc
  (UI → store → router → service → DB → download) matching the template

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* security(files): cap filename length at 255 chars (C2)

ShareFileRequest.filename and ShareFileMcpRequest.filename get
Field(max_length=255, min_length=1). display_name same cap.
Prevents 10KB+ filename edge cases from agent or attacker.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* security(files): disk-space pre-check before write (C3)

New check_disk_space() helper using shutil.disk_usage('/data').
Refuses writes when /data has less than size_bytes + 500MB free
(HTTP 507 Insufficient Storage). Called before persisting.

Protects shared /data mount — SQLite DB, Vector logs, and log
archives live there too; letting an agent fill the disk causes
platform-wide outage, not just a failed share.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(cleanup): purge expired and old-revoked shared files (C4 / Step 7)

Adds delete_expired_and_revoked(revoke_grace_hours=24) to the DB ops
class (returns stored_filename list for disk unlink) + facade forward
+ wired into cleanup_service.py's 5-min tick.

Per cycle:
- SELECT rows where expires_at < now OR revoked_at < now - 24h
- DELETE them from DB
- unlink each /data/agent-files/{stored_filename}
- bumps CleanupReport.shared_files_purged

The 24h grace on revoked rows keeps them queryable for incident
diagnosis right after revocation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* security(files): dedicated rate-limit bucket for downloads (C5)

/api/files/{id} now uses _check_file_download_rate_limit which keys
redis by file_downloads:{ip} instead of sharing the public_link_lookups
bucket used by /api/public/chat and friends. Limits unchanged (60/min
per IP).

Prevents heavy download traffic from starving the rate-limit quota for
public chat or other /api/public/* endpoints on the same IP.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(files): HEAD handler mirroring GET validation (C6)

Link previewers (Slackbot, Twitterbot, Discordbot, facebookexternalhit)
HEAD-probe URLs before GET. Our endpoint was 405-ing those.

Extracted _validate_download_request() helper from GET; new HEAD
handler reuses it and returns Response(200) with the same headers
(Content-Disposition, nosniff, no-store, Content-Length) but no body,
no download counter bump, no audit row. Follows RFC 7231 §4.3.2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* security(files): tighten list endpoint to owner+admin only (C7)

GET /api/agents/{name}/shared-files previously used can_user_access_agent
(owner/admin/shared). But the list response includes full download URLs
with signed tokens — so anyone able to see the list can reuse every
share. That's the same capability as share_file + revoke, both of which
already require can_user_share_agent.

Change to can_user_share_agent (owner + admin), 403 otherwise.
DELETE was already owner-only; no change needed there.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(prompt): agent nudge for share_file MCP tool (C8)

Add a new 'Sharing Files with Users' section to the system-wide
PLATFORM_INSTRUCTIONS between Collaboration and Operator Communication.
Tells every agent:
  - write files to /home/developer/public/
  - call share_file MCP tool with the relative filename
  - return the URL as-is

This means new agents discover the capability without the user
needing to name the tool explicitly. Applies immediately to every
agent via compose_system_prompt() — no image rebuild needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(pr-491): address validation findings — PII redaction + scope drift

PR #491 /validate-pr flagged two issues:

1. CRITICAL: pavshulin@gmail.com in two draft docs' Owner fields
   (amazing-file-outbound.md, amazing-file-outbound-production-readiness.md).
   Public repo + CLAUDE.md forbids real user emails.
   → Replaced with @pavshulin (GitHub handle).

2. WARNING: .claude/settings.json committed as new file — personal
   Claude Code permission allowlist unrelated to FILES-001 scope.
   → Merged 8 allowlist entries into .claude/settings.local.json
     (gitignored per .gitignore:67). Removed .claude/settings.json.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test: fix test-ordering contamination from FILES-001 mixin fixture

Two adjustments surfaced by running the full unit suite:

1. test_file_sharing_mixin.py registered `sys.modules['db']` as a plain
   module (no `__path__`), which poisoned `from db.X import Y` lookups in
   sibling tests (e.g. test_fleet_sync_audit did `from db.schedules ...`
   and hit `'db' is not a package`). Now we give our stub a `__path__`
   pointing at the real db directory, and restore `sys.modules['db']` on
   fixture teardown so no leakage remains.

2. test_start_agent_skip_inject.py didn't mock the new
   `check_public_folder_mount_matches` import added by FILES-001 in
   services/agent_service/lifecycle.py. The Mock container lacked
   iterable `attrs["Mounts"]`, blowing up with TypeError. Stubbed the
   whole `file_sharing` submodule and bound the check on `_mod`
   per-test to return True by default.

Full unit suite now matches dev baseline: 17 pre-existing failures,
701 passing (+33 over dev, all from FILES-001).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by:  Pavlo <pash@pashs-MacBook-Pro.local>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(channels): deliver images as vision content blocks via stream-json (#562) (#566)

Replaces the broken base64 data-URI-in-text approach (where Claude Code
received images as opaque markdown strings) with proper vision content
blocks fed via --input-format stream-json stdin. Images sent through
Telegram (and other channel adapters) are now visible to the agent.

- message_router: _handle_file_uploads returns 4-tuple (added image_data);
  image MIME files collected as {media_type, data} dicts instead of embedded
- task_execution_service: execute_task() accepts images param, forwards in payload
- agent_server models: ParallelTaskRequest.images field added
- agent_server chat router: passes images to runtime.execute_headless()
- claude_code: adds --input-format stream-json and builds JSON content-block
  stdin payload when images present; stdout/stderr threads start before stdin
  write to prevent pipe deadlock; write moved into executor (not event loop)
- runtime_adapter ABC + GeminiRuntime: images param added to prevent TypeError

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(frontend): add state, brand, accent token families (#555) (#561)

Extends the design-system token system from #67 with three additional families
for colors that don't fit the status taxonomy:

  state-*   agent operating modes (autonomous, locked)
  brand-*   third-party product identity (claude, gemini)
  accent-*  decorative highlights named after the literal color so future
            accents (accent-green, etc.) join cleanly

New tokens (all alias full Tailwind palettes, identical visual output):
  state-autonomous → amber   (AutonomyToggle AUTO mode)
  state-locked     → rose    (ReadOnlyToggle ON mode)
  brand-claude     → orange  (RuntimeBadge for Claude Code)
  brand-gemini     → blue    (RuntimeBadge for Gemini CLI)
  accent-purple    → purple  (DashboardPanel widget badges)

Also extends scripts/check-design-tokens.mjs to validate the new families
via a KNOWN_FAMILIES map; the reference scanner now flags typos within any
of the four families (status/state/brand/accent), not just status-*.

Migrates the 4 components blocked by #67's status-only scope:
  - RuntimeBadge.vue       → brand-claude, brand-gemini
  - AutonomyToggle.vue     → state-autonomous
  - ReadOnlyToggle.vue     → state-locked
  - DashboardPanel.vue     → accent-purple slot in getStatusColors

Fixes #555

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(scheduler): agent-owned pre-check hook (#454) (#455)

* feat(scheduler): agent-owned pre-check hook (#454)

New optional contract: agents implement POST /api/pre-check in their
container; scheduler calls it before firing a cron-triggered chat.
Endpoint absent or any error → fire as usual (fail-open). fire=false
records a skipped execution. fire=true with a message overrides the
schedule.message for that invocation.

- docker/base-image/agent_server/routers/pre_check.py: new router that
  dynamically loads /home/developer/.trinity/pre-check.py (template-
  supplied) and calls its check() function
- agent-server main.py: mount pre_check_router
- scheduler/agent_client.py: pre_check() method with fail-open semantics
  on 404/5xx/timeout/malformed-response
- scheduler/service.py: _run_pre_check + pre-check branch in
  _execute_schedule_with_lock (cron only; manual triggers bypass)
- tests/scheduler_tests/test_pre_check.py: 12 tests covering client-
  and service-level behavior; 161/161 scheduler suite passes

Zero schema change — reuses existing ExecutionStatus.SKIPPED and
create_skipped_execution. Closes the "wake agent on every cron tick"
cost gap noted in docs/planning/PR_REVIEWER_AGENT.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(#454): scheduler pre-check feature flow + arch + requirements

- feature-flows/scheduler-pre-check.md: new flow doc with contract,
  fail-open semantics, error table, testing summary
- architecture.md: add /api/pre-check to agent-server endpoint list
  and pre-check note to Scheduler Service row
- requirements.md: SCHED-COND-001 entry under §10 (Scheduling & Execution)
- feature-flows.md: index row
- docs/planning/PR_REVIEWER_AGENT.md: design doc from which this
  feature was extracted — committed for traceability

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* review: address PR #455 review feedback

- pre_check.py: asyncio.get_event_loop() → get_running_loop() (deprecated in 3.10+)
- pre_check.py: oversized message override no longer dropped silently —
  response now carries message_truncated="override dropped: N bytes exceeds
  32000 cap" so scheduler/operator can see what happened; log escalated to
  ERROR with size+limit details
- pre_check.py: module-level docstring expanded to note the security scope
  of check() (full Python interpreter access, same sandbox as chat tools —
  operators should review .trinity/pre-check.py like any executable template
  file) and the intentional no-cache behavior
- tests/unit/test_pre_check_router.py: 15 new router/unit tests covering
  oversized-message drop path and non-dict return → 500 (both previously
  only exercised by inspection). Uses importlib to load pre_check.py
  directly, avoiding python-multipart requirement from sibling routers
- feature-flows/scheduler-pre-check.md: document truncation behavior,
  security scope expectation, and updated test summary (12 scheduler +
  15 router = 176 total passing)

Lock-scope concern noted in review is not an issue: the skip path returns
from _execute_schedule_with_lock, and the outer _execute_schedule holds
the lock in a try/finally that covers the return. No leak.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(#454): docker exec instead of agent-server HTTP endpoint

Review feedback on #455 flagged that the HTTP-endpoint design introduced
a new system edge (scheduler → agent-server direct) and a novel code-
loading pattern (importlib in a router). Both broke with Trinity's
established convention that all "run something in an agent container"
flows go through `services/docker_service.execute_command_in_container`
— the same primitive used by:

- services/git_service.py (persistent-state allowlist, #384 S3)
- services/ssh_service.py (key provisioning)
- services/agent_service/terminal.py (web SSH)
- routers/system_agent.py (admin exec)
- adapters/message_router.py (Slack file ingest)
- routers/voice.py, monitoring_service.py

This commit swaps the design accordingly.

Changes:
- Delete docker/base-image/agent_server/routers/pre_check.py and its
  router registration. No new HTTP surface on agent-server.
- Delete tests/unit/test_pre_check_router.py (router is gone).
- Add src/backend/routers/internal.py →
  POST /api/internal/agents/{name}/pre-check. Runs the template-shipped
  `.trinity/pre-check.py` via execute_command_in_container. Two-step:
  `test -f` for existence, then `python3 .../pre-check.py`. Returns
  {hook_present, exit_code, stdout, stderr}. Gated by existing
  X-Internal-Secret header (C-003).
- Rewrite src/scheduler/service.py::_run_pre_check to call the backend
  endpoint (scheduler no longer opens a direct edge to agent-server).
  Translates …
vybe pushed a commit that referenced this pull request Jun 19, 2026
…kend resource (#1083) (#1273)

* feat(exec): apply_result extraction + inert hardened result-callback endpoint (#1083 PR1)

PR1 of fire-and-forget dispatch — a pure refactor plus a fully-hardened,
fail-closed result-callback endpoint that rejects all live traffic until PR2
sets the durable async marker. Zero behavior change to current execution paths.

- Extract TaskExecutionService.apply_result — the single terminal applier shared
  by the inline sync path and the future result-callback. Moves the SUCCESS and
  the httpx #678-salvage FAILED terminal writes (sanitize, cost rollup, context,
  CAS write, activity completion, breaker outcome, optional slot release) behind
  one normalized TerminalEnvelope. Every side effect is gated on the CAS bool
  (Codex #1/#12): a CAS-lost write does nothing — no double activity close,
  breaker churn, or slot drain. Producer-side classification (SUB-003, error-code,
  timeout-terminate) stays in execute_task.
- slot_service.release_slot: gate the BACKLOG-001 drain on the ZREM result so a
  replayed/no-op release can't admit a backlog row past max_parallel_tasks
  (Codex #12). Every legitimate drain trigger removes a present member.
- POST /api/agents/{name}/executions/{id}/result — agent's own MCP-key auth
  (mirrors heartbeat authorize_heartbeat) + ownership + a durable async-marker
  gate (RUNNING rows must carry claude_session_id='dispatched_async', else 409)
  + idempotent replay (terminal → {replayed:true}) + body-size 413 caps.
- db.get_open_activity_id_for_execution: filtered by related_execution_id AND
  chat/schedule_start AND state='started' so a shared-eid tool_call row can't be
  cross-closed (Codex #8). mark_execution_dispatched gains async_dispatch=True.
- config: DISPATCH_ASYNC flag + ASYNC_DISPATCH_ELIGIBLE_TRIGGERS={schedule,webhook}
  + dispatch_async_eligible(); compose/.env forwarding (backend-only, default off).

Tests: apply_result golden + CAS-gate matrix, callback auth/ownership/replay/
marker/size matrix, DB seams; existing CAS-gate + #678 auto-retry parity green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(exec): backend cutover — async dispatch-and-return + lease reaper (#1083 PR2 lane A)

Backend half of the fire-and-forget cutover (agent-server half follows). Inert
until DISPATCH_ASYNC=true AND a Claude-runtime agent on a new base image ACKs 202.

- execute_task: for an async-eligible trigger ({schedule,webhook}) under
  DISPATCH_ASYNC, send async_result=true, write the durable 'dispatched_async'
  marker, and on a 202 ACK return RUNNING/dispatched_async immediately, handing
  the slot lease to the result callback (skip the `finally` release). Any non-202
  response (200 / old image / non-Claude runtime) falls through to today's
  synchronous handling — the safe mixed-fleet fallback. The runtime gate is
  enforced agent-side (decision 5).
- cleanup_service lease reaper: after fail_stale_slot_execution, tag the FAILED
  message with the lease_expired code and close the open dispatch activity via
  the filtered get_open_activity_id_for_execution (the absent fire-and-forget
  coroutine `finally` no longer closes it). Adds TaskExecutionErrorCode.LEASE_EXPIRED.
- Finding 1: _sweep_stale_executions now uses each agent's timeout+SLOT_TTL_BUFFER
  window (mark_stale_executions_failed gains agent_timeouts+buffer_seconds) instead
  of the flat 120-min default, so a legitimately-running max-timeout async turn
  isn't failed ~5 min before the slot reaper / canary E-01. agent_timeouts=None
  reproduces the prior flat behaviour exactly.

Tests: dispatch-return matrix (202→RUNNING/no-release, non-202 fallback, trigger
scope), stale-sweep per-agent boundary REGRESSION (timeout+buffer±ε), lease
activity close; existing CAS-gate / cleanup / observability tests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(agent): async-accept (202) + result-callback report/persist/retry (#1083 PR2 lane B)

Agent-server half of the fire-and-forget cutover. When the backend sends
async_result=true AND this agent runs the Claude runtime, /api/task accepts with
202 and runs the turn in a detached task that reports the typed terminal to the
backend's result-callback endpoint. Inert for non-Claude runtimes / old behaviour
(async_result defaults false).

- services/result_callback.py: try_spawn_async gates on async_result + Claude
  runtime + execution_id + callback creds (else the caller runs synchronously —
  the non-202 fallback). _run_and_report runs the headless turn, builds a typed
  envelope (success → completed; HTTPException → status-mapped error_code/
  terminal_reason, with metadata salvaged from the structured 502 body), persists
  it atomically to ~/.trinity/pending-results/<eid>.json, and delivers it with
  capped backoff up to the slot-lease deadline (dispatch + timeout +
  SLOT_TTL_BUFFER), deleting on a 2xx or a permanent 4xx. A strong-ref _inflight
  set defeats the asyncio weak-ref GC footgun.
- main.py: startup sweep re-sends any envelope left on disk by a crash/restart,
  so completed work isn't lost to a phantom LEASE_EXPIRED (a late SUCCESS still
  overwrites it via the backend CAS).
- chat.py /api/task: 202 branch before the synchronous path; models.py:
  ParallelTaskRequest.async_result flag.

v1 limitation (T8 / #1201, P2 fast-follow): only the 502 empty-result failure
path carries cost/context metadata on the async callback; 504/503 write a
null-cost row until execute_headless_task exposes ctx.metadata on those paths.

Tests: envelope mapping, eligibility gating, persist/resend roundtrip, and the
retry-to-deadline delivery loop (2xx, permanent-4xx no-retry, transient-5xx
retry, deadline). agent-server regression subset green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(architecture): document fire-and-forget dispatch subsystem + result-callback endpoint (#1083)

architecture.md gains a "Fire-and-Forget Dispatch (#1083)" cross-cutting block
(apply_result CAS-gating, durable async marker, callback endpoint, agent-side
persist/retry/sweep, lease reaper + stale-sweep buffer fix, v1 boundaries), the
result-callback endpoint row, and a pointer from the task_execution_service
catalog entry. feature-flows index gains the #1083 Recent Updates row.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(exec): let a late SUCCESS callback correct a reaper LEASE_EXPIRED (#1083, Codex #2)

The result-callback's idempotent-replay short-circuit fired for ALL terminal
statuses, which would block a genuinely-late SUCCESS callback from reaching the
CAS after the lease reaper had already FAILed the row (LEASE_EXPIRED) — directly
contradicting the plan's accepted "SUCCESS overwrites a phantom FAILED" behavior.

Now only the authoritative terminals (SUCCESS/CANCELLED/SKIPPED) short-circuit as
a replay ACK; a FAILED row falls through to apply_result, whose CAS lets a late
SUCCESS overwrite it (a duplicate FAILED is harmlessly CAS-blocked → no
side-effect re-run). The async-marker gate still rejects a FAILED *sync* row
(marker != dispatched_async), so the cross-path guard holds for terminals too.

+2 callback tests (FAILED-async fall-through, FAILED-sync 409); architecture.md
clarified.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(agent): validate execution_id charset before async dispatch (#1083)

Defense-in-depth: result_callback builds a pending-results filesystem path
(~/.trinity/pending-results/{execution_id}.json) and the backend callback URL
from the backend-supplied execution_id. Add a strict allowlist guard
(_SAFE_EXECUTION_ID = ^[A-Za-z0-9_-]{1,128}$) enforced at try_spawn_async — a
value outside the token_urlsafe / UUID charset now falls back to synchronous
handling and never reaches the path build or the callback URL.

Adds parametrized unit tests and archives the CSO --diff audit report for the
branch.

Refs #1083

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(agent): containment barrier at the path-build sink (#1083, CodeQL)

CodeQL py/path-injection flagged _persist/_delete: a caller-side regex guard in
try_spawn_async is not recognized as a barrier for the callee sink. Move the
sanitizer into _pending_path itself — resolve() + is_relative_to() containment
against _PENDING_DIR (mirrors jsonl_recovery), raising ValueError on escape. The
regex stays as the belt; this is the suspenders on the path-build dataflow, so a
hostile execution_id can never traverse out of the pending dir.

Refs #1083

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(agent): use os.path.basename to sanitize the pending-result path (#1083, CodeQL)

CodeQL py/path-injection did not accept resolve()+is_relative_to() as a barrier
and still flagged the path build in _pending_path/_persist/_delete. Switch to the
canonical CWE-022 sanitizer: os.path.basename strips any directory components
from execution_id before the join, so the write is confined to _PENDING_DIR. The
resolve()+is_relative_to() containment stays as suspenders; the regex guard in
try_spawn_async remains the belt.

Refs #1083

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(agent): adopt the #950 normpath+startswith path-containment guard (#1083, CodeQL)

resolve()+is_relative_to() and os.path.basename were both ignored by CodeQL's
py/path-injection model (same dead end #950 hit before switching). Mirror the
guard that actually cleared the alert there: os.path.normpath collapses any
'..', an inline startswith prefix-check confirms containment under _PENDING_DIR,
and the normalized value is flowed downstream to write/replace/unlink. Raises on
escape; the try_spawn_async regex belt still rejects such ids upstream.

Refs #1083

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Eugene Vyborov <eugene@beingluminous.com>
vybe pushed a commit that referenced this pull request Sep 8, 2026
…preview and delete (#2582, Abilityai/trinity-enterprise#548) (#2608)

* docs(workspace): requirements + architecture + flows for the Files tab (#2582, Abilityai/trinity-enterprise#548)

Trinity Rule #1 — the docs land before the code.

- requirements/core-agent.md: new §5.30 (the six ACs, the permission matrix,
  the storage decision, the stated limits); §5.20 AC-2/AC-4 amended — the
  Files signal now covers uploads and the refresh triggers widen.
- requirements/content-files.md: §13.10's flat "Content-Disposition:
  attachment" bullet was stale against shipped ent#461; rewritten as the
  server-decided allowlist plus the one-way ?download=1.
- architecture/integrations.md: the ent#461 delivery-policy paragraph records
  the one-way flag, why it is applied in the handlers, why it is parsed
  tolerantly, and that sig is a stored bearer token rather than an HMAC over
  the URL — so appending the flag cannot invalidate it.
- architecture/api-endpoints.md: GET row updated, the missing HEAD row added,
  and the three new client-portal routes catalogued.
- architecture/workspace.md + database.md: the upload announcement, the
  session-type-dependent matrix, and portal_file_dismissals with the three
  reasons its shape is what it is.
- feature-flows: workspace-rail.md gains Slice 3; file-sharing-outbound.md
  corrected on three counts that were stale since ent#461 and #568;
  workspace-agents-at-the-centre.md and the index updated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq

* feat(workspace): portal_file_dismissals on both migration tracks (#2582, Abilityai/trinity-enterprise#548)

A Workspace viewer needs to remove an agent-shared file from THEIR list without
revoking the share. agent_shared_files has no audience column, so
portal_documents lists every active share of an agent to every rostered client;
and the one generic per-user preference store is FK'd to users.id, which a
portal principal has no row in. So: new storage.

- tables.py / schema.py (DDL + the sweeper's file_id index) / migrations.py
  (SQLite) / migrations/versions/0058 (PostgreSQL) — Invariant #9, both tracks,
  single head confirmed by scripts/ci/check_alembic_heads.py.
- agent_name is on the table FOR the AgentRef registration: agent_shared_files
  is a CASCADE ref, so deleting an agent hard-deletes its shares without going
  through the revoke sweeper — every dismissal keyed on those ids would be
  orphaned forever, and a table with no agent column would sidestep the parity
  guard that exists to catch exactly this.
- Both purge paths in db/agent_shared_files.py delete the matching dismissals
  in the same transaction.
- PK leads with client_email (the read is WHERE client_email = ?, once per
  participant per turn end); the file_id index serves the sweeper.

test_agent_cleanup_parity + test_schema_parity: 8 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq

* feat(files): the one-way ?download=1 flag, and the Workspace file verbs (#2582, Abilityai/trinity-enterprise#548)

routers/files.py
- GET and HEAD accept `download`, which may only ever force `attachment`.
  There is no ?disposition= and no way to force `inline` — that direction is
  ent#461's XSS allowlist. `_format_disposition`'s docstring now records the
  asymmetry, so the next reader does not resolve the apparent contradiction in
  the wrong direction.
- Typed Optional[str] with a truthy check, NOT bool: a bool query param 422s on
  ?download= or ?download=x, and this is the public link opened from Telegram /
  WhatsApp / iOS — a malformed query it ignores today must keep being ignored.
- Applied in the handlers, never in _validate_download_request, whose arg list
  is AST-pinned by test_file_download_no_session_gate.
- A ranged PREFIX read no longer bumps download_count. The Workspace preview
  reads from byte 0, so without this every preview would inflate the owner's
  numbers and bury the audit log. The audit row is kept and made separable
  instead: details.ranged_prefix.
- `# mcp: none` header (Invariant #13).

client_portal
- portal_owns_agent + `owned` on the roster row and the card: the SAME
  membership the card renders, so the UI's "Delete for everyone" and the
  service's gate cannot disagree.
- Three routes: read one of your own uploads back (attachment, nosniff,
  no-store), delete one (idempotent), and remove/revoke an agent share. Each is
  _require_roster -> rate_limiter -> service -> audit.
- TWO limiter tiers, env-tunable, per router.py's own stated rule — and the
  burst tier is 20, tighter than upload's, because this is the first time a
  rostered client can reach extract_from_agent (sync docker-py iteration on the
  global 4-worker pool, ~3x file size resident). Delete gets its own looser
  counter; an rm is not a get_archive.
- portal_revoke_shared_file is access-first (roster 404 -> owner 403 -> row
  404), never existence-then-access (Invariant #8), and 404s where its operator
  sibling 204s — enumeration-uniformity on an external surface, stated in the
  docstring so nobody aligns it.
- portal_dismiss_shared_file does NOT validate the file_id (an existence oracle
  over every share in the install) and caps rows instead — the same fork
  set_chat_star already resolved.
- _inbox_path_for: `_safe_filename(name) == name` or a uniform 404. Every shell
  use is shlex.quote plus `--`, because _safe_filename admits a leading hyphen.
- _read_inbox guesses mime_type, and PortalUploadItem declares it — the row's
  FileIcon has been rendering the generic icon since it shipped because the
  response model stripped the undeclared field.
- portal_documents drops dismissed ids and appends &download=1. Only this base
  URL gets the flag; the agent's chat link is untouched.

test_ent79_portal_exposure: the two download_url assertions updated (the AC
changes the URL) and the fixtures gained the table portal_documents now reads.
71 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq

* feat(workspace): the Files tab — uploads at once, save, preview, delete (#2582, Abilityai/trinity-enterprise#548)

stores/clientPortal.js
- uploadDocument queues the agent in a pending-agent SET. It is the ONE funnel
  all three upload surfaces already call, so notifying here makes "Files you
  sent" update before any agent reply WITHOUT touching PortalConversation.vue
  or PortalRoom.vue — the files the delivery sequence is serialized to protect.
- A set, not a scalar, because both real gestures defeat a scalar: a sequential
  multi-file batch (a consumer joining the in-flight read gets a listing taken
  before the later files landed) and a room's fan-out across three DIFFERENT
  agents in one Vue flush window (only the last value survives).
- fetchUploadBlob / deleteUpload / deleteDocument.

stores/portalRailFeeds.js
- noteUpload(agent): shares _fetchToken with refresh() (a refresh issued before
  the upload but resolving after it would otherwise silently clobber the fresh
  listing), coalesces leading AND trailing, and re-checks participation after
  the await. upload() now delegates to it rather than carrying a second read.

portalRail.js / usePortalRailFeeds.js
- filesSignalItems(documents, uploads) projects uploads onto the created_at key
  the Files dot already reads, and BOTH the signal and markSeen read it — one
  mechanism, or opening the tab would mark documents seen and leave the
  uploads' dot lit forever. The collections stay separate; only the signal
  merges them.
- The pending set is drained by the owner: clear-then-read, so a note arriving
  mid-drain is a new entry rather than one this drain already claimed.

portalFiles.js (new, pure — vitest pins environment: node)
- flattenFiles owns BOTH render order and preview index; two orderings drift and
  the lightbox opens the wrong file, silently. `groups` is a parameter so
  ent#484's shared folder becomes a third entry with no structural change.
- previewKind is extension-first for text: Python's mimetypes maps .ts to
  video/mp2t and .toml to nothing, and a shared .md arrives as text/plain.
- neighbour skips non-previewable rows and stops at the ends.
- errorDetail reads a Blob body first: with responseType 'blob' the usual
  err.response.data.detail idiom yields undefined, so the promised "server's
  named reason" would degrade to a generic line for exactly the new verbs.

PortalFilePreview.vue (new)
- Images ONLY through <img :src>, never inline <svg>, never v-html.
- Text capped at 256 KB, fetched whole and sliced: CORS allow_headers omits
  Range, so a ranged preview dies silently cross-origin — and slicing keeps
  preview off the transfer-start counter path. The cap is stated in the UI.
- Escape/arrows registered with { capture: true } + preventDefault, because the
  conversation's turn-cancel listener is on document in the BUBBLE phase.
- v-if not v-show (the column and the mobile sheet are siblings), z-40 below
  ConfirmDialog's z-50, focus on the safe action, a Tab trap.
- Never a blank modal: an unpreviewable type, an over-cap image AND a failed
  byte fetch all land on the same name/size/type + Download card.

PortalRailFiles.vue
- One v-for over the flat list; Download on both lists; the delete matrix
  mirrored off the roster card's `owned`; one ConfirmDialog with copy that
  restates the consequence per case; per-row InlineError with the server's
  reason. The dead single-file upload() (zero callers) is gone.
- AC-3's `download` attribute is deliberately dropped: the control is a button
  driving a blob save and cannot carry it, and it is inert on a cross-origin
  anchor anyway — which is why the server-side flag exists.

npm run test:unit: 101 files, 2238 passed. check:tokens OK. Both new files
scan 0 raw_nongray / 0 hardcoded; loading gates total 70, equal to baseline.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq

* test(files): the one-way flag, the upload verbs, and the delete matrix (#2582, Abilityai/trinity-enterprise#548)

Three new files, registered in tests/registry.json, plus the ent#461 guard
extended so the asymmetry is asserted where a future widening would be edited
past.

test_2582_download_flag.py (35)
- Proves the ASYMMETRY, not the feature: the flag forces attachment and NO
  input forces inline (?download=0 on text/html stays attachment).
- ?download= / ?download=x / a repeated pair never 422 — the regression a bool
  annotation would have shipped on the public link ent#461 exists to keep
  opening from a phone.
- HEAD agrees with GET; the sig survives the extra query pair; every other
  ent#461 header is carried through; the flag rides the 206 branch.
- A ranged prefix read is audited ranged_prefix:true and does NOT bump
  download_count; a full-file range and a plain GET do; a mid-file seek does
  neither. Mutation control: forcing is_ranged_prefix=False fails that test.

test_2582_portal_uploads.py (28)
- The traversal 404 asserted at the handler AND through the mounted route,
  with a positive control — %2F shapes never match the route while %2E%2E
  reaches the handler, and a status-only test cannot tell the two apart.
- Gate order (off-roster before any docker work), ent#308's collision on the
  read side, the translated extract_from_agent exceptions (its 404 echoes the
  container path), the deliberate 409/502 on a stopped/missing agent, rm -f --
  with quoting proven by a name that needs it, and both limiter tiers.

test_ent548_portal_share_delete.py (23)
- The matrix, including the two cases that read as bugs without the rule: a
  non-owner admin is a viewer, and so is an owner on a portal token.
- Access-first revoke proven with a call-recording monkeypatch (the row must
  not be read before the caller is authorized). Mutation control: reordering
  to existence-then-access fails it.
- The dismissal's non-validation and its row cap as a PAIR, both purge paths,
  the AgentRef registration, both migration tracks, the PK's leading column.
  Mutation control: dropping the delete_for_agent purge fails it.

218 passed across the plan's verification set; alembic heads still 1.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq

* test(workspace): the upload signal, the flat projection, and the preview rules (#2582, Abilityai/trinity-enterprise#548)

portalRailFiles.spec.js (19) — and it found a real defect.

The room fan-out test failed against my own first cut: `noteUpload` shared
`_fetchToken` with `refresh()`, so three concurrent per-agent reads each
invalidated the last and two of three listings were discarded. The two
questions are different — "has the chat moved on?" is global, "is this agent's
listing still the newest?" is per agent — so the store now carries a
`_scopeToken` (participant changes and clear only) and a per-agent
`_uploadEpoch` that `refresh()` snapshots before its awaits. That snapshot is
also what stops a refresh issued before an upload and resolving after it from
silently clobbering the fresh listing.

Three mutation controls, each failing exactly its own test:
- a shared counter instead of the per-agent epoch -> the fan-out test and the
  clobber test fail;
- dropping the trailing re-fire -> the two-file batch test fails;
- refresh ignoring the epoch snapshot -> the clobber test fails.

The spec mounts the composable inside an effectScope stopped after each test:
its watcher on the SHARED portal mock has no component to unmount it, so the
oldest surviving watcher drained the queue against a previous test's Pinia
store. Diagnosed from the symptom (uploads.scout undefined, a fan-out noting
one agent), not guessed.

Also pinned: the dot lights with Files CLOSED after one targeted read; opening
the tab clears it (markSeen must read the same projection or the upload's dot
stays lit forever); a non-participant bump does nothing; a hidden rail drops
the note rather than queueing it; and source guards that PortalConversation.vue
and PortalRoom.vue are untouched and know nothing of the rail.

portalFiles.spec.js (47) — previewKind's extension-first rule with the three
cases that motivate it (.md arriving as text/plain, .ts as video/mp2t, .toml as
nothing), neighbour skipping and stopping, flattenFiles order equalling render
order, the fileActions matrix failing closed, sameOriginPath on a RELATIVE url
(the default install), the Blob error path, and the component's source guards:
an <img> and no v-html, no Range, capture + preventDefault, revokeObjectURL,
z-40 below ConfirmDialog, v-if not v-show, never a blank modal including on a
failed fetch, zero raw palette classes and zero hex.

portalRailFeeds.spec.js: the portal mock is now reactive() with the new fields,
so the owner's watcher is not permanently inert there.

103 files, 2304 passed. check:tokens OK; loading gates 70 (= baseline); both
new files 0 raw_nongray / 0 hardcoded.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq

* docs(workspace): correct the upload-ordering mechanism, and record the third Escape owner (#2582, Abilityai/trinity-enterprise#548)

Two corrections and one addition from the tail steps.

The docs written before implementation said the drain "shares the feed store's
_fetchToken with refresh()". That is no longer true and was never sufficient:
the room fan-out test proved a shared counter makes three concurrent per-agent
reads invalidate each other. The mechanism is a _scopeToken (chat identity) plus
a per-agent _uploadEpoch that refresh() snapshots before its awaits. Corrected
in requirements/core-agent.md §5.30, architecture/workspace.md,
feature-flows/workspace-rail.md (Slice 3, with the finding recorded as the
reason) and the feature-flows.md changelog row.

/sync-feature-flows also surfaced one flow the docs commit missed:
chat-turn-cancellation.md owns the "what may take Escape" rule, and this PR adds
a THIRD way of owning it — a rail-mounted overlay with no ref the conversation
could name, taking Escape in the capture phase with preventDefault() rather than
joining the per-surface overlay list. The known residual (the voice-call branch
above shouldCancelOnEscape never consults defaultPrevented) is recorded there
too, with a revision-history row, and the index row now points at it.

The Slice 3 Testing block names the effectScope the spec needs and the three
mutation controls.

NOTE: /update-tests also updated .claude/agents/test-runner.md, which lives in
the private .claude submodule (detached HEAD in this worktree) — that edit is
left UNCOMMITTED there rather than creating a dangling commit and dirtying the
gitlink. It needs landing in trinity-dev separately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq

* fix(workspace): Download a share through the server flag, not a 50 MB blob (#2582)

Self-review finding. The first cut fetched every row as a blob and handed it
back through a synthetic <a download> — which made the one-way ?download=1 flag
decorative on the one surface it was added for, pulled up to 50 MB into the tab
to save a file the browser could stream itself, and used exactly the
programmatic-blob-save path the plan's own risk register flags as the classic
iOS Safari failure, on a surface whose primary form is a phone sheet.

An agent share now saves by an anchor click on its already-attachment URL. A
client upload keeps the blob path, because it has no URL at all — no DB row, no
token — which is the whole reason that path exists.

AC-3's `download` attribute rides on that anchor as belt-and-braces; a browser
ignores it cross-origin, which is precisely why the server-side flag is the
mechanism rather than the attribute.

Pinned by a source guard with a mutation control (collapsing the branch fails
it). 103 files, 2305 passed. Docs corrected in §5.30 and workspace-rail.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq

* fix(workspace): the delete confirm owns Escape too, or it cancels the turn (#2582, Abilityai/trinity-enterprise#548)

Review finding. The preview modal took Escape in the capture phase with
preventDefault() so it could not reach the conversation's bubble-phase
turn-cancel listener — but the ConfirmDialog it raises has no key handling of
its own, and nothing was added for it. So Escape on an open delete confirm
dismissed nothing and arrived at shouldCancelOnEscape with defaultPrevented
still false: the dialog stayed up and an in-flight turn was destroyed.

Same shape, same reason. And because two capture listeners on `document` fire
in registration order — the tab body mounts before the modal it opens — the
preview now returns early on event.defaultPrevented, so one keystroke closes
one overlay rather than both.

Also documents the three PORTAL_FILE_* limiters in .env.example, beside the
PORTAL_CHAT_*/PORTAL_UPLOAD_* pair they were modelled on. They are the only
bound on the newly-exposed extract_from_agent path, and an operator cannot
tune a knob they cannot discover.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq

* perf(workspace): one inbox read per upload, not two (#2582, Abilityai/trinity-enterprise#548)

Review finding. `portalRailFeeds.upload()` awaited `noteUpload()` itself while
`clientPortal.uploadDocument()` — which it calls one line earlier — already
queued the same agent for the rail owner's drain. The drain's note therefore
arrived while the store's own read was in flight, became its trailing re-fire,
and every drop-zone upload cost two container execs: eight for a four-file
batch, on the shared 4-worker executor.

The funnel is the mechanism, so let it be the only one. `upload()` now just
sends. The two specs that pinned "delegates to noteUpload / re-reads its own
agent" pinned the redundancy, so they now pin ZERO reads from `upload()` and
move the "skips a chat that moved on" property onto `noteUpload`, where it
actually lives.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq

* docs(workspace): name the four filed follow-ups (#2582, Abilityai/trinity-enterprise#548)

The plan listed four things to file and the docs said "filed as a follow-up"
while nothing existed on either tracker. Filed, and named where a reader lands:

  trinity#2598  Escape during a voice call ends the call even when an overlay
                already claimed the keystroke — PortalConversation.vue's voice
                branch sits above shouldCancelOnEscape and reads no
                defaultPrevented, so preventDefault() cannot reach it
  trinity#2599  the Workspace Files routes are uncatalogued in api-endpoints.md
  ent#549       shared files have no audience — every rostered client sees
                every share of an agent, ?sig= included; a dismissal is a
                preference, not authorization
  ent#550       a client upload has no DB row, which is why listing, download,
                delete, preview and MIME are five different mechanisms

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq

* docs(memory): the Escape-ownership class, from the #2582 review

An overlay that claims Escape must claim it for the dialogs it raises too —
the 4.14 incomplete-fix class in UI clothes, and destructive-and-silent here
because the wrong Escape kills billed work with no sign. Plus the two rules
that fall out: capture listeners on `document` fire in registration order, not
z-order; and a `preventDefault()` protocol is only as good as the consumer
branch that reads it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq

* test(files): prove the one-way flag where it is WIRED, not only where it is computed (#2582)

`_apply_download_flag` is a pure function, and the existing test pins its
asymmetry exactly. But a pure function cannot tell you the route handed it the
right `inline=`. The plausible regression — a handler deriving the disposition
from the query parameter instead of from `is_inline_safe(row["mime_type"])` —
passes every helper-level assertion in this file and ships stored XSS on a
public token-gated link.

So the route fixture is now parametrized by type, and a `text/html` row is
asserted `attachment` across every flag value, on GET and HEAD both. Verified
by mutation: deriving the disposition from the parameter turns 6 tests red,
5 of them these.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(workspace): forward the portal file limits from every compose (#2582, Abilityai/trinity-enterprise#548)

The three knobs were read by `client_portal/router.py` and documented in
`.env.example` while being forwarded by NONE of the three compose files, and
neither prod nor hosted uses `env_file:` — so the advertised `.env` lever was
inert on every install. That is worse than an undocumented knob: the operator
sets it, sees no effect, and has nothing to debug. The #1056 / trinity-enterprise#31
packaging-gap class, caught at the `/validate-pr` gate.

Proven by render rather than grep — `docker compose config` showed NONE before
and honours `PORTAL_FILE_HOURLY_LIMIT=7` over the 100 default after, on both
dev and prod.

Wiring prod also broke #2280's wholesale env parity against
`docker-compose.hosted.yml`, which is the guard doing its job: the gap was three
files wide, not two.

Guarded so it cannot recur: the knobs must be forwarded by all three composes,
with defaults agreeing across them AND with the module default. #2433's guard is
scoped to its own two vars by a hardcoded tuple, so extending it would have been
the wrong home. Mutation-verified — dropping one var from prod turns 2 tests red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(workspace): exclude file previews from download counts

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: sim <sim@example.com>
dolho added a commit that referenced this pull request Sep 9, 2026
Addresses the `/validate-pr` CHANGES_REQUESTED on #2487. Every item is about
a guard that is present and inert, which is why the refactor landed green.

**C1 — the 60/60 route-set proof never ran in CI.** It read the pre-split
module out of git, and every checkout in `backend-unit-test.yml` is
`fetch-depth: 1`, so `git show dd91056:…` failed on every run and the test
skipped — leaving "no route lost or invented" across a 3,529 → 10 module split
proven nowhere. The fork-point set is now a frozen 60-tuple literal (a fork
point is a historical fact, so freezing it costs no maintenance; a route added
since goes in `_ADDED_SINCE_SPLIT`, one reviewed line at a time). The git read
survives as a separate test that re-derives the literal wherever history is
deep enough, so the transcription cannot drift. Its temp module is written
under `tmp_path`, not `src/backend/` — an interrupted run there left a
top-level module `Dockerfile:131` would bake into the image.

**C2 — the integration suite broke at collection**, in FOUR files, not three:
`test_monitoring_service.py` too. All of them `spec_from_file_location` on
`services/agent_client.py`, which is now a package, so they raised
FileNotFoundError before any test ran; this only stayed green because
integration runs nightly rather than per-PR. Replaced with plain imports: the
loaders existed to bypass `services/__init__.py`, and that has not been true
since the module started importing `services.agent_auth` at import time (it is
on `dev` too). Privates come from the module that owns them
(`circuit._CIRCUIT_HASH_PREFIX`, `http_pool._client_pool`), per the package's
own no-mirrored-collaborators rule, and the caplog assertions key on the
parent logger name so they still capture from every submodule. Collection is
back to 83 = `dev`'s 83.

**Two invariant guards went blind on 3,529 lines.** `routers/settings/` is the
first subdirectory ever created under `routers/`, and both
`test_1310_auth_wiring.py` (Invariant #8) and `test_models_centralized.py`
(Invariant #14) globbed one level deep — all ten modules escaped, and it fails
open, so nothing showed. `rglob`, keyed by path relative to `routers/` so two
packages cannot share an allowlist key (a top-level file's relative path is its
bare name, so neither allowlist changes). 73 → 84 files scanned.

**A dead constant with a live test guarding it.** `routers/public.py` still
declared `MAX_CHAT_MESSAGES_PER_IP`/`_PER_TOKEN` while enforcement reads
`public_chat_service`'s copy, so `test_ip_rate_limit_fix.py` was asserting a
constant nothing enforces — equal values today, so only the guard had broken.
Now re-exported from the enforcing module.

**Split-detachment in `public.py`.** The two #311 gates were from-imported
under private aliases while the service called its own module-locals: one
function, two monkeypatch targets. Now called through the sibling module
object, which is the rule both package `__init__` docstrings state; the two
tests that patch or read it are repointed, and the `files.py` guard now bans
both spellings so the retired alias cannot let a re-import through.

Docs: `architecture.md` Invariant #1 gains the package paragraph (re-export
the public surface only; reach siblings through the module object; guards use
`rglob`), and the four stale `.py` references in the shards are corrected.

Also: nine modules carried a duplicated module-level `logger`; and
`fleet_ops_service` renames the fleet-restart log channel from `routers.ops`,
which is now stated in the code rather than left for an operator to discover.

Verified: unit suite 6269 passed, 1 failure — the pre-existing `::ffff:`
IPv4-mapped parsing case (local 3.12 vs the repo's 3.13 target), byte-identical
on clean `dev`.

Related to #1028

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP
dolho added a commit that referenced this pull request Sep 10, 2026
…on the session (ent#535)

**The critical.** The Redis session blob omitted `tool_manifest`, so the
cross-worker rebuild silently unlocked it: `get_session` passed nothing,
`_session_manifest` read `None` as "never resolved", and the reconstruction
handed the model the FULL platform default — in the `LiveConnectConfig` and in
the dispatcher, with no log line, because from that worker's view nothing had
ever been narrowed. Production runs `--workers 2` and the WebSocket routinely
lands on a worker other than the one `/voice/start` ran on, so this is the
normal path, not an edge case. The block comment directly above that dict says
"EVERY field a reconstructed session decides on must be here"; this PR added a
decision field and did not.

It is stored as `sorted(...)`/`null` — `json.dumps` cannot serialize a set, so
writing the frozenset raw would raise inside the try and lose the whole blob —
and read back by `_manifest_from_meta`, which maps three inputs onto two
answers: absent or `null` → unresolved (the pre-ent#535 default, and the
mid-deploy case); a list, INCLUDING `[]` → a decision; anything else → unresolved
rather than a crash in the audio loop. Collapsing `null` and `[]` would
reintroduce the exact inversion this PR fixed.

`test_create_session_writes_redis` asserted three named keys and never round-trip
completeness, which is why this shipped green. It is now a SUBSET relation over
the session's own dataclass fields, with the deliberately-unpersisted ones listed
by reason, so a field added tomorrow fails the guard instead of shipping
unpersisted.

**Also folded in, all from the review:**

* `_portal_turn` had 15 unreachable lines below its `return`, copy-pasted from
  `_execute_tool` and referencing names not in that scope. Removed, with an AST
  guard so nothing lands after that return again.
* The empty-prompt guard is back on the chat path. `portal_chat` calls
  `_persist_user_turn` unconditionally, so a blank `run_task` durably wrote an
  empty user row into the person's Workspace thread and dispatched a real,
  cost-tracked execution; `required=["prompt"]` makes that unlikely, not
  impossible. Stripped rather than falsy, and worded identically on both paths.
* `include_owned=True` is no longer a constant at the turn site. It travels on
  the session as `is_platform` (default False), written by
  `start_workspace_voice` — the function that refuses a non-platform caller, so
  the gate that authorizes the wider roster read is the one that records it.
  Sound today because that function is the only writer of `portal_session_id` +
  `client_email`; a future path setting both would otherwise widen
  `agent_on_roster` with no change at that line (Invariant #8).
  `canvas_audience` already travels for exactly this reason.
* `docs/memory/requirements/runtimes.md` §29.7 (VOICE-007) rewritten — it still
  claimed a 30s timeout and `_execute_and_respond()` → `POST /chat`, both false
  for the Workspace path since this PR. AC 7 said it had been; only the
  feature-flow had.

Removing the persisted field reds three of the new tests; verified by deleting
it and re-running. 848 passed across every voice / portal test in the suite.

Related to Abilityai/trinity-enterprise#535

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RSjVEjay9ztC1oDXXkh9uN
vybe pushed a commit that referenced this pull request Sep 10, 2026
…ent#535) (#2656)

* feat(voice): the call acts as the agent, over a locked tool surface (ent#535)

`run_task` from a Workspace voice call went to the agent container's task
endpoint: a stateless run with a 30s timeout, no thread, no memory of the
conversation the person was in. In the issue's words, that is "what makes
voice just chat today".

A Workspace call now runs the turn through `portal_chat` — the SAME
pipeline a typed message takes — into the thread the call is bound to, on
the thread's own `cached_claude_session_id`. The agent has its skills,
its files, its memory and its mid-work state, and the answer lands in
that chat as a turn (and on the canvas if it drew). A call with no thread
(VoIP, the legacy Agent Detail session) keeps the container path, because
there is nothing to run it in. The split is `_is_workspace_bound`, which
requires BOTH `portal_session_id` and `client_email`: a thread with no
email cannot be attributed, an email with no thread has nowhere to land,
and either alone would silently fall back to the container.

**The spoken budget is not a cancellation.** Past `_SPOKEN_BUDGET_SECONDS`
(20s) the model is told the work is still running and keeps the floor,
while the turn CONTINUES and its reply lands in the chat. The old 30s
`wait_for` cancelled it — throwing away work already done and paid for.
The detached turn is strongly referenced so it cannot be collected
mid-flight (the #1083 footgun), and it deliberately outlives the call: a
turn the person asked for is worth landing whether or not they are still
on the line. `_on_tool_result` fires when it lands, so a badge clears on
the real event rather than on a timer.

**The manifest is locked.** `services/voice_tools.py` owns the policy:
resolved once at session start, `_build_live_config` builds the config
FROM it, and the dispatcher refuses any name outside it before reading an
argument. A per-agent declaration may only NARROW — `template.yaml` is
agent-writable, so a declaration that could ADD would let an agent grant
itself a capability by editing itself. Fleet tools are absent by
construction: `PLATFORM_VOICE_TOOLS` is the only door a name enters
through.

Two defects found while building it, both fixed here:

* the manifest was read as `session.tool_manifest or default`, so an
  agent declaring `voice.tools: []` — the strongest narrowing — fell
  through to the FULL platform set. The field is tri-state now: `None` is
  "never resolved" (→ the safe default), `frozenset()` is a decision.
* `_execute_and_respond` read a nameless tool call as `run_task`
  (`getattr(fc, 'name', 'run_task')`), sending the model's arguments to
  the agent under a name nobody chose. It is refused now, and the refusal
  ANSWERS the call — a model that never receives a response for a call it
  made stops speaking.

Scoped out, as decisions rather than omissions:

* **the template-declared narrowing is mechanism-only.** `resolve_manifest`
  takes and honours a declaration, and `create_session` threads it — but
  nothing reads one yet, because `/api/template/info` does not expose a
  `voice` block. Adding it means an agent-server field plus a base-image
  rebuild, and a reader against a field no deployed agent returns would
  be a feature that reads as working and does nothing.
* **the orb badge is backend-only.** The session counts in-flight turns
  and the landing fires `_on_tool_result`; rendering it is frontend work
  this PR does not do.

`_execute_tool` keeps its original `(agent_name, …)` contract — the
routing moved to the dispatcher, which already holds the session — so the
existing container-path tests still exercise the real thing. Two panel
tests move to `workspace_mode=True`: the canvas tools only exist in a
workspace session, which is the combination the model could ever produce.

23 new tests, mutation-checked (a cancelling budget, a widening union,
and a dropped refusal each turn the suite red). 1001 passed across the
voice, canvas, portal and VoIP families.

Related to Abilityai/trinity-enterprise#535

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(voice): persist the locked manifest; the roster widening travels on the session (ent#535)

**The critical.** The Redis session blob omitted `tool_manifest`, so the
cross-worker rebuild silently unlocked it: `get_session` passed nothing,
`_session_manifest` read `None` as "never resolved", and the reconstruction
handed the model the FULL platform default — in the `LiveConnectConfig` and in
the dispatcher, with no log line, because from that worker's view nothing had
ever been narrowed. Production runs `--workers 2` and the WebSocket routinely
lands on a worker other than the one `/voice/start` ran on, so this is the
normal path, not an edge case. The block comment directly above that dict says
"EVERY field a reconstructed session decides on must be here"; this PR added a
decision field and did not.

It is stored as `sorted(...)`/`null` — `json.dumps` cannot serialize a set, so
writing the frozenset raw would raise inside the try and lose the whole blob —
and read back by `_manifest_from_meta`, which maps three inputs onto two
answers: absent or `null` → unresolved (the pre-ent#535 default, and the
mid-deploy case); a list, INCLUDING `[]` → a decision; anything else → unresolved
rather than a crash in the audio loop. Collapsing `null` and `[]` would
reintroduce the exact inversion this PR fixed.

`test_create_session_writes_redis` asserted three named keys and never round-trip
completeness, which is why this shipped green. It is now a SUBSET relation over
the session's own dataclass fields, with the deliberately-unpersisted ones listed
by reason, so a field added tomorrow fails the guard instead of shipping
unpersisted.

**Also folded in, all from the review:**

* `_portal_turn` had 15 unreachable lines below its `return`, copy-pasted from
  `_execute_tool` and referencing names not in that scope. Removed, with an AST
  guard so nothing lands after that return again.
* The empty-prompt guard is back on the chat path. `portal_chat` calls
  `_persist_user_turn` unconditionally, so a blank `run_task` durably wrote an
  empty user row into the person's Workspace thread and dispatched a real,
  cost-tracked execution; `required=["prompt"]` makes that unlikely, not
  impossible. Stripped rather than falsy, and worded identically on both paths.
* `include_owned=True` is no longer a constant at the turn site. It travels on
  the session as `is_platform` (default False), written by
  `start_workspace_voice` — the function that refuses a non-platform caller, so
  the gate that authorizes the wider roster read is the one that records it.
  Sound today because that function is the only writer of `portal_session_id` +
  `client_email`; a future path setting both would otherwise widen
  `agent_on_roster` with no change at that line (Invariant #8).
  `canvas_audience` already travels for exactly this reason.
* `docs/memory/requirements/runtimes.md` §29.7 (VOICE-007) rewritten — it still
  claimed a 30s timeout and `_execute_and_respond()` → `POST /chat`, both false
  for the Workspace path since this PR. AC 7 said it had been; only the
  feature-flow had.

Removing the persisted field reds three of the new tests; verified by deleting
it and re-running. 848 passed across every voice / portal test in the suite.

Related to Abilityai/trinity-enterprise#535

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RSjVEjay9ztC1oDXXkh9uN

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: sim <sim@example.com>
dolho added a commit that referenced this pull request Sep 14, 2026
…nt#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
vybe pushed a commit that referenced this pull request Sep 14, 2026
…ile (ent#553) (#2619)

* 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

* 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): 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

* 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

---------

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>
vybe pushed a commit that referenced this pull request Sep 16, 2026
…the gate a mechanism (Abilityai/trinity-enterprise#628) (#2826)

* fix(mcp): gate the loop tools on the agent permission edge, and make the gate a mechanism (Abilityai/trinity-enterprise#628)

`run_agent_loop` resolved its target from the caller's parameter and called no
gate, and the backend behind it resolves an agent-scoped key to its owner
(Invariant #8) — so an agent key could start a loop on any same-owner sibling
with no `agent_permissions` edge. Verified live: the loop ran on the sibling.
The per-tool gate had ten spellings across nine modules and the tool added
last called none of them, so this closes the class, not the tool.

- src/mcp-server/src/access.ts (new): ONE implementation of the agent-scope
  edge (`checkAgentEdge` — system bypasses; an agent key reaches itself and its
  permitted targets, the permitted list read fail-closed; a user key passes
  through because the backend already scopes it by role and per-user grant;
  any other scope is denied — an allowlist, #2323); `TOOL_ACCESS_POLICY`, one
  row per registered tool (enforce / in-tool / baselined:<owner> / none:<why>);
  `policyFor` (no row, an enforce on an undeclared parameter, or a none on a
  tool whose parameters name an agent throws at registration); and
  `withAgentAccess`, the enforce wrapper.
- server.ts: every tool passes `policyFor`; enforce rows are wrapped before
  `withAudit`; dynamic tools declare their policy as an argument.
- tools/loops.ts: `run_agent_loop` is an enforce row; `get_loop_status` /
  `stop_loop` resolve the loop's agent, gate, then act — a denial withholds the
  payload behind a compound uniform reason (the id was the caller's only
  input), and a failed resolve sends no stop and names the escape hatch.
- tools/chat.ts: the agent/system branch delegates to the shared gate; unknown
  scopes are denied before the user branch (closes a fall-through that
  promoted an unnamed agent key to the same-owner rule); `resolveClient` moves
  to access.ts. types.ts / client.ts: `LoopStatus`, `getLoopStatus` typed.
- .github/workflows/mcp-server-test.yml: an offline boot smoke of
  dist/server.js — bundler module resolution hides a missing `.js` until the
  container dies.
- tests: access.test.ts (createServer boots against the real table, every row
  names a tool and every tool has a row; policyFor refusals; the wrapper never
  reaches execute on a denial; the read fails closed), tools/loops.test.ts
  (all three tools through the real row and wrapper with a fake client; a
  denial means the side-effecting call never happened; the reason is
  byte-identical to chat_with_agent's), J10: the strict xfail comes off, and a
  loop-id read or stop after the edge is removed is refused without naming
  the loop's agent while the owner can still stop it.
- docs: the flow, the requirement (§38.1 said cross-agent loops were out of
  scope, the flow said "backend enforces", the code did neither), the
  architecture area file and the P-02 catalog entry now say one thing: a
  tool-surface gate at the MCP layer; the REST routes stay owner-equivalent,
  which is Abilityai/trinity-enterprise#629's ruling. The 53 `baselined` rows
  are that issue's work list. Learnings ledger +1; diff-scoped CSO report.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* test(mcp): execute the line that wires the gate — run_agent_loop over a real transport, an agent key, and a counted backend (Abilityai/trinity-enterprise#628)

Review C1 on #2826: `server.ts::addToolWithAudit` composes `withAgentAccess`
around an `enforce` row's `execute` and hands the result to `withAudit`. Both
existing files proved the wrapper — `access.test.ts` calls it directly,
`tools/loops.test.ts` builds the composition by hand — and neither executed the
composition site. Build the wrapper there and discard it (`withAudit(tool.name,
tool.execute, …)`) and the suite stayed at 406 green with `run_agent_loop`
ungated: the #2811 class, the reaction tested and the wiring that calls it not.

`access-wiring.test.ts` drives the tool the way an agent does: a real
`createServer` in key mode, a real MCP client presenting an agent-scoped key
over the streamable-HTTP transport, and a stub backend that answers
`/api/mcp/validate` and the permission-edge read and COUNTS every
`POST /api/agents/<target>/loops`. Without an edge the count stays at zero and
the caller reads the denial; with an edge the loop starts; a self loop starts
without a permission read. Under the reviewer's mutation the first case is the
one red (`loopPosts` = ["bravo"], `success: true`); restored byte-identical,
409/409. Pattern: `inline-auth-transport.test.ts` (#2035).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
vybe pushed a commit that referenced this pull request Sep 16, 2026
…2487)

* refactor(settings): routers/settings.py becomes a ten-module package (#1028)

3,529 lines — the largest file in the backend, 4.4x the 800-line critical
threshold — split into ten domain modules composed onto ONE router, so the
mounted API is byte-identical and `from routers.settings import router` is
unchanged. Largest resulting module: credentials.py at 778 lines.

Inclusion order is load-bearing (Invariant #4): `generic` owns the
GET/PUT/DELETE /{key} catch-alls, which match any single segment, so it is
included LAST — before its siblings it would swallow /ops/config,
/brain-orb, /api-keys/anthropic and answer 'setting not found' for routes
that exist. test_1028_settings_package.py pins:

- the mounted route SET equals the pre-split module's, compared against the
  real blob out of git (60/60, none lost, none invented)
- no route is shadowed by an earlier registration (the property that
  actually matters — literal order is deliberately NOT pinned, since
  regrouping specific routes relative to each other is inert)
- the catch-all include stays last, named at the include line a human edits
- every module stays under the 800-line threshold
- the import surface callers depend on still resolves (resolve_mcp_url,
  the key sets, _REPO_PATTERN)

Collaborators (db, platform_audit_service, settings_service) are
deliberately NOT re-exported on the package __init__: ~20 tests patch them
as module attributes, and after a move such a patch would apply cleanly to
a module nobody reads — a test asserting nothing while hitting the real
accessor. Absent attributes make every stale patch raise AttributeError
instead, which is exactly how the 14 affected test files were found and
repointed to the modules that own their handlers.

GET '' (the root listing) is registered on the parent router because a
prefix-less sub-router cannot carry an empty path (FastAPI refuses).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NLfHNPtB5UCMk4LonZiJux

* refactor(git): services/git_service.py becomes a six-module package (#1028)

2,322 lines split by responsibility — conflicts, gitignore, remotes,
trinity_files, sync, provisioning — with the full public surface re-exported
from the package __init__, so `from services.git_service import
sync_to_github` and `git_service.<name>` callers are unchanged. Largest
resulting module: gitignore.py at 649 lines.

Cross-module calls go THROUGH the sibling module object
(`gitignore._detect_git_dir(...)`), never a from-import of the function: a
from-import freezes the binding, so a test patching the owning module would
silently stop reaching the caller. Pinned structurally by
test_1028_git_service_package.py, alongside the size threshold and the
import surface.

Private names are re-exported ONLY where another backend module imports them
or a test reads them as data. A private function mirrored on both the
package and its owning module can be monkeypatched on the wrong one and
silently detach — which is exactly what happened to test_2069's readiness
probes mid-split (the multiline setattr sites patched the package's
re-exported copies while merge_gitignore_after_clone read the module's own),
so the collaborator-shaped names are deliberately not mirrored: a stale
patch raises AttributeError instead of testing nothing.

~15 test files repointed to the modules that own their handlers, including
the three sys.modules-isolated file loaders and test_github_init_push, whose
exec fake must now land on every module binding the driven function awaits
through (provisioning + gitignore + remotes — patched via the loaded package
instance, since its harness purges and reloads the package). The #2069
merge-caller guard now walks the whole package and matches qualified calls,
so a caller cannot fall out of its census by moving between modules.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NLfHNPtB5UCMk4LonZiJux

* refactor(client): services/agent_client.py becomes a three-module package (#1028)

1,294 lines split by responsibility — circuit (the #631 transport breaker:
constants, Lua, CircuitState, dormant alerting, admin read/reset), http_pool
(the per-agent httpx pool + drop-grace stamps), client (AgentClient, typed
errors, get_agent_client) — public surface re-exported from the package
__init__, so every existing import is unchanged. Largest module: client.py
at 698 lines.

Same discipline as the git_service split, pinned by
test_1028_agent_client_package.py: cross-module calls go through the sibling
module object, collaborators are not mirrored on the package, and no module
may from-import a sibling's function (a frozen binding silently detaches
monkeypatches on the owning module).

test_circuit_breaker.py's direct file-load gains package plumbing
(submodule_search_locations + a sys.modules registration before exec — the
__init__'s relative imports cannot resolve their parent otherwise), and its
patches land on the owning modules. The #1677 caller-parity allowlist entry
for _emit_dormant_alert follows the file to
services/agent_client/circuit.py — that guard firing on the move is exactly
what it is for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NLfHNPtB5UCMk4LonZiJux

* refactor(ops,public): the heavyweight handlers move behind their routes (#1028)

The last two ACs. `public_chat` — 289 lines of session identity, access
gating, rate accounting, upload decoding, memory injection and dispatch,
inside routers/public.py — moves to services/public_chat_service.py in the
#1483 shape (service raises PublicChatError, the thin route maps it 1:1);
client-IP extraction, the per-IP limit and token resolution stay router-side
because they are HTTP concerns. agent_requires_email /
agent_allows_open_access move with it and the router re-imports them — one
definition, not a copy. public.py: 1,239 → 901 lines.

routers/ops.py's five heavyweights — fleet health, the #1860 locked fleet
restart, fleet stop, emergency stop, the cost rollup — move to
services/fleet_ops_service.py (704) and services/ops_costs_service.py (208).
The auth gates stay IN the router deliberately: the #2389 fence-vs-gate
scans read live handler source there, and a gate that moved with the body
would satisfy auth while blinding the scan. ops.py: 1,304 → 506 lines.

test_1028_extracted_services.py pins the thinness, the gates' location, the
size class — and an unresolved-module-scope-name walk, added because the
move surfaced exactly that class twice: PublicChatResponse was unresolved in
the chat service while 623 tests passed (nothing drives the sync-success
return), and utc_now_iso the same in the costs service. py_compile cannot
see this; the walk can.

test_1860 / test_1917 fixtures now hand back the SERVICE module with the
route entry points attached, so collaborator patches land on the bindings
the moved bodies actually read while the gate patch stays on the router.
The #894 override-wiring census follows public_chat's two execute_task call
sites to their new file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NLfHNPtB5UCMk4LonZiJux

* test(1028): repoint the settings source-guards at the package; satisfy the sys.modules lint

Four guards read routers/settings.py as SOURCE TEXT (the ent#12 consent
AuditEventType pin + generic-PUT block, the ent#434 catch-all window)
and went FileNotFoundError when the module became a package — repointed
at the package glob (or generic.py where the guard scopes a specific
handler window). The new test files' own sys.modules registrations move
onto monkeypatch.setitem / the _restore_sys_modules precedent, and the
lint baseline is regenerated DOWNWARD (140 across 49 files — the patch
migrations in the split commits removed ~66 stale entries).

Related to #1028

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NLfHNPtB5UCMk4LonZiJux

* test(1028): the size guard counts blank lines out, comments in

Self-review finding, and the more serious of the two: my previous commit
changed the guard's METRIC so that my own edit would pass. That is the
re-baselining this file exists to make hard, wearing a docstring as cover.

Measured rather than argued. `credentials.py`:

  at 6d8c93a  raw 778   non-blank 687
  after        raw 806   non-blank 687   (blank-line restoration only)
                         non-blank-non-comment 643

Excluding blanks is exactly invariant under the change that prompted it.
Excluding comments as well moves the calibration: these files carry 110-208
comment lines each, so a comment-blind count hands `credentials.py` ~160 lines
of headroom the 800 ceiling never gave it, and `generic.py` 208.

So the metric now excludes blank separators and nothing else. Restoring a
PEP-8 blank line between two defs still does not read as a module growing,
and the ceiling still means what it meant when these files were authored
against it.

Related to #1028

* test(1028): name the size metric for what it counts

Re-review of my own fix. The metric was corrected to exclude blank lines only,
and left named `_logical_lines` — which in Python means the opposite, since a
logical line excludes comments.

That is not a cosmetic mismatch. Reading the phrase "logical lines" in this
test's docstring is precisely what talked the previous pass into excluding
comments and re-baselining the guard by 160 lines. Leaving the name in place
leaves the same trap armed, one identifier along, for the next reader who
"corrects" the body to match it.

`_non_blank_lines`, and the AC's docstring says "800 non-blank lines" in its
own words rather than delegating the definition to a helper name.

Related to #1028

* fix(1028): make the split's own safety nets actually run

Addresses the `/validate-pr` CHANGES_REQUESTED on #2487. Every item is about
a guard that is present and inert, which is why the refactor landed green.

**C1 — the 60/60 route-set proof never ran in CI.** It read the pre-split
module out of git, and every checkout in `backend-unit-test.yml` is
`fetch-depth: 1`, so `git show dd91056:…` failed on every run and the test
skipped — leaving "no route lost or invented" across a 3,529 → 10 module split
proven nowhere. The fork-point set is now a frozen 60-tuple literal (a fork
point is a historical fact, so freezing it costs no maintenance; a route added
since goes in `_ADDED_SINCE_SPLIT`, one reviewed line at a time). The git read
survives as a separate test that re-derives the literal wherever history is
deep enough, so the transcription cannot drift. Its temp module is written
under `tmp_path`, not `src/backend/` — an interrupted run there left a
top-level module `Dockerfile:131` would bake into the image.

**C2 — the integration suite broke at collection**, in FOUR files, not three:
`test_monitoring_service.py` too. All of them `spec_from_file_location` on
`services/agent_client.py`, which is now a package, so they raised
FileNotFoundError before any test ran; this only stayed green because
integration runs nightly rather than per-PR. Replaced with plain imports: the
loaders existed to bypass `services/__init__.py`, and that has not been true
since the module started importing `services.agent_auth` at import time (it is
on `dev` too). Privates come from the module that owns them
(`circuit._CIRCUIT_HASH_PREFIX`, `http_pool._client_pool`), per the package's
own no-mirrored-collaborators rule, and the caplog assertions key on the
parent logger name so they still capture from every submodule. Collection is
back to 83 = `dev`'s 83.

**Two invariant guards went blind on 3,529 lines.** `routers/settings/` is the
first subdirectory ever created under `routers/`, and both
`test_1310_auth_wiring.py` (Invariant #8) and `test_models_centralized.py`
(Invariant #14) globbed one level deep — all ten modules escaped, and it fails
open, so nothing showed. `rglob`, keyed by path relative to `routers/` so two
packages cannot share an allowlist key (a top-level file's relative path is its
bare name, so neither allowlist changes). 73 → 84 files scanned.

**A dead constant with a live test guarding it.** `routers/public.py` still
declared `MAX_CHAT_MESSAGES_PER_IP`/`_PER_TOKEN` while enforcement reads
`public_chat_service`'s copy, so `test_ip_rate_limit_fix.py` was asserting a
constant nothing enforces — equal values today, so only the guard had broken.
Now re-exported from the enforcing module.

**Split-detachment in `public.py`.** The two #311 gates were from-imported
under private aliases while the service called its own module-locals: one
function, two monkeypatch targets. Now called through the sibling module
object, which is the rule both package `__init__` docstrings state; the two
tests that patch or read it are repointed, and the `files.py` guard now bans
both spellings so the retired alias cannot let a re-import through.

Docs: `architecture.md` Invariant #1 gains the package paragraph (re-export
the public surface only; reach siblings through the module object; guards use
`rglob`), and the four stale `.py` references in the shards are corrected.

Also: nine modules carried a duplicated module-level `logger`; and
`fleet_ops_service` renames the fleet-restart log channel from `routers.ops`,
which is now stated in the code rather than left for an operator to discover.

Verified: unit suite 6269 passed, 1 failure — the pre-existing `::ffff:`
IPv4-mapped parsing case (local 3.12 vs the repo's 3.13 target), byte-identical
on clean `dev`.

Related to #1028

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP

* refactor(1028): split gitignore.py three ways after the dev merge

The dev merge brought #2529's ~715 lines into
`services/git_service/gitignore.py`, taking it to 1305 raw lines — past
the 800-line threshold this PR exists to enforce, and its own guard
(`test_1028_git_service_package::test_every_module_is_under_the_critical_threshold`)
said so. Re-baselining the guard under cover of a fix is precisely what
the settings-test docstring in this PR warns against, so the module is
split instead:

  gitignore.py        686  the patterns, the regions, the command builders
  gitignore_sweep.py  418  what a sweep DID — tags, parse, alert, report
  gitignore_clone.py  273  the once-per-agent merge after clone

The seam is "what the file CONTAINS" vs "what did that just do" vs "the
one-shot at creation". Cross-module references go through the module
object (`gitignore.<name>`, `gitignore_sweep.<name>`), never a
from-import: a from-import freezes the binding and a monkeypatch then
lands on a detached copy — which is how test_2069's readiness probes
went dark mid-split.

Three real defects surfaced while wiring it and are fixed here, not
carried:

- `_gitignore_merge_semaphore` and `_inflight_gitignore_merge_tasks`
  were referenced bare in `gitignore_clone` with no such globals —
  a NameError on the live clone-time merge path. The five merge
  constants + the semaphore + the in-flight set now live in
  `gitignore_clone`, their sole consumer.
- `_shadowed_negations` read `_GITIGNORE_MANAGED_LINES` bare after the
  move. It now reads it off `gitignore` through a deliberately
  function-local import — `gitignore` imports this module at its top
  level, so a module-level one would close the cycle at import time.
- `datetime` was left behind by `_augment_commit_message`.

The sibling suites are repointed at the module that OWNS each name, so
every symbol still has exactly one monkeypatch target: test_2529's sweep
names to `gitignore_sweep` (new `_sweep()` accessor beside `_gs()`), and
test_2069's readiness/merge collaborators to `gitignore_clone`.

741 passed, 3 skipped across the git/gitignore/1028/1310/models families.

Related to #1028

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP

* test: dev's post-fork tests reach the split modules the way every earlier one does (#1028)

Five files landed on dev after the fork with patch targets and source reads
on the monoliths. Re-pointed the same way the split re-pointed the rest:

- test_ent582_platform_keys: `is_claude_auth_configured` /
  `connect_agents_to_first_credential` on `settings.credentials`,
  `platform_keys_service.check_resend_key` on `settings.provider_keys`
- test_2695_stt_capability_probe: `_elevenlabs_settings_state_with_capability`
  on `settings.integrations`
- test_2691_public_url_reachability: the save-path source read on
  `settings/generic.py`, the flag-surface read on `settings/flags.py`,
  `update_setting`/`db`/`platform_audit_service` on `generic`
- test_github_init_push: the exec recorder also installed on
  `git_service.token_scrub`, which the ent#615 seed now runs through
- routers/settings/generic.py: the #2572 hook reaches `credentials` through
  an absolute function-local import — `test_2216_backup_observability` and
  `test_2572` load this module in isolation via `spec_from_file_location`,
  where a module-level relative import raises at collection

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf

* test: dev's two new settings tests reach the split modules (#1028)

`test_workspace_flag_retired` patches `settings_service`, `telemetry_sharing_service`
and `db` on the module it calls `get_public_feature_flags` from, and
`test_2696_stt_provider_errors` calls `_elevenlabs_settings_state_with_capability`.
Both imported the flat `routers.settings`. Now they import the `flags` and
`integrations` submodules, like the earlier re-points in 9c34662, so the
patches land on the globals the handlers actually read.

Full unit suite on this tree: 16468 passed, 32 skipped, 0 failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test: patch the modules that own the moved globals, and let #1917 follow the moved ops code (#1028)

merge-train validation findings. Collection was fixed earlier; these are the
runtime half.

- `tests/integration/test_circuit_breaker.py`: 19 `monkeypatch.setattr(agent_client, ...)`
  calls and 34 `agent_client.CIRCUIT_*` reads now target
  `services.agent_client.circuit`, which reads its own globals. Patching the
  package re-export changed nothing, and `_get_circuit_redis` is not
  re-exported, so it raised AttributeError. Against a fakeredis server:
  8 failed / 26 passed before, 34 passed after (dev: 34 passed).
- `tests/git_sync/test_s5_conflict_classifier.py` loads `git_service/conflicts.py`,
  because the flat `git_service.py` no longer exists.
- `tests/git_sync/test_s7_reserve_instance_id.py` patches
  `check_remote_branch_exists` and `db` on `git_service.provisioning`, where
  `reserve_and_generate_instance_id` looks them up. Both git_sync files:
  33 passed.
- `tests/unit/test_1917_stack_trace_exposure.py`: the raw `str(e)` ban now
  also scans `services/fleet_ops_service.py` and `services/ops_costs_service.py`,
  where the ops handler bodies moved. Neither file has any hits today.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: sim <sim@example.com>
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>
vybe pushed a commit that referenced this pull request Sep 23, 2026
…heir own included (ent#596) (#2990)

* feat(skills): only designated agents may change an agent's skills — their own included (ent#596)

An agent-scoped key resolves to its owner carrying the owner's role
(Invariant #8), so the owner fence on the four skill write routes let any
agent rewrite the skills of every sibling its owner holds — and since
#2703 an assignment also writes the skill's executable files into the
target in the same call.

Ruling (operator 2026-09-17, restated 09-18): changing an agent's skills —
another agent's OR ITS OWN — is its own permission. An instance admin
grants it to named agents; every other agent key is refused on both.
Neither this nor permission to call an agent implies the other. People and
the system agent are unchanged. OSS-core (operator, 2026-09-23).

The fence. `dependencies.get_skill_managed_agent_by_name`, a composed
dependency on PUT /skills, POST /skills/inject, POST and DELETE
/skills/{skill}. It checks the capability BEFORE the owner fence, so a
non-holder gets one uniform named 403 (`skill_management_not_permitted`)
for an existing, nonexistent or foreign target alike — never an existence
signal (#186) — and a holder gains no reach beyond its owner's agents. An
ALLOWLIST over `mcp_scope` (#2323): JWT / user key / system pass; `agent`
passes only with a live grant; connector, ops, portal_delegate and any
scope invented later are refused; a principal with no scope fails closed.
Every refusal is audited with the agent and key id.

The side door. Both independent plan reviewers found it: PUT/DELETE
/api/agents/{n}/files and POST /files/mkdir checked only ACCESS, and
`.claude/skills/**` was not in the write deny-list, so an agent key with
no grant wrote the same SKILL.md + scripts/ into a sibling through a
route whose name says nothing about skills. Those routes now take the
capability for that subtree; a delete of an ancestor (.claude, the home
dir) counts, since it removes every skill. The shared deny-list — mirrored
in the agent image and the guardrails baseline — is untouched, so people
still edit skills in the Files tab.

The grant. `agent_capability_grants(agent_name, capability, granted_by,
granted_at)`, capability `skills.manage` — a row, not a column, so who
granted it and when is answerable and later capabilities (ent#590,
ent#341) share the seam. Both tracks (SQLite + Alembic 0072 ← 0071),
AgentRef CASCADE. Reads join agent_ownership and filter deleted_at, so a
soft-deleted agent holds nothing and recovery restores the grant.
GET /api/skills/managers (admin) and PUT /api/agents/{n}/skill-manager
(admin AND interactive — a user-scoped key never grants). Refuses a
nonexistent/soft-deleted agent (uniform 404) and the system and ephemeral
agents (named 422). Settings → Agents → Skill managers. The route is its
own noun: /skills/manager would be captured by the {skill_name} catch-all.

Attribution (Tandem R29). `agent_skills.assigned_by_agent` records the
agent that made an assignment — the system agent's writes are attributed
to it, since the backend otherwise strips its name — NULL for a person.
The bulk replace is delete-all + reinsert, so it now carries who/when for
names it keeps; otherwise an orchestrator's next replace would make a
skill a person assigned last month read as the orchestrator's.

MCP: the three write tools move from `baselined: ENT629` to the backend
fence, and their descriptions say the permission exists.

Default on upgrade: nobody holds it (the ruling). Orchestrators running
/reconcile-skill-map get the named 403 until granted — grant trinity-pm /
corbin at deploy.

Tests: tests/unit/test_ent596_skill_manager.py (65) — the scope matrix,
capability-before-owner ordering, the route table read off FastAPI's real
dependant graph, a refusal changing nothing on all four routes through a
real app, the file-route side door, grants and attribution over a real
SQLite file, both migration tracks executed. Eight mutations each red:
{self} ∪ permitted, a route losing the fence, the sentinel as
getattr(..., None), every agent holding the grant, owner fence first, the
file bypass reopened, delete ignoring ancestors, the replace re-stamping.
Frontend skillManagersPanel.spec.js (7, mounted); access.test.ts (+1).
Full backend unit suite: the same 43 failing node ids as the dev
merge-base, zero new.

Fixes Abilityai/trinity-enterprise#596

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix(skills): a refused capability is audited as the AGENT, not its owner (ent#596)

Found by the live demo, not by review or the suite: every
`capability_refused` row on the dev instance read `actor_type=user,
actor_id=1` — the owner — with the agent visible only through the key's
name. `platform_audit_service._resolve_actor` ranks `actor_user` above
`actor_agent_name`, so passing the principal filed a prompt-injected
agent's refused attempt as the owner's own act. That is the exact "the
agent did it" vs "the person did it" line (Tandem R29) this capability
exists to keep.

For an agent principal the gate now passes the agent as the actor, the
owner as `actor_email`, and the key id / name / scope explicitly (the
service derives them from `actor_user` otherwise). Humans are unchanged —
they are never refused by this gate anyway.

The unit test asserted only that `actor_agent_name` was passed, which was
true and irrelevant: the resolver ignores it whenever `actor_user` is also
present. It now asserts `actor_user` is absent and pins the real resolver's
precedence. Negative control: the old call shape turns it red.

Related to Abilityai/trinity-enterprise#596

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fix(skills): the Skill managers picker shows its placeholder after a grant, not a blank select (ent#596)

Found by the live screenshot. After a successful grant the granted agent
leaves the candidate list, so its <option> is removed while still
SELECTED: the browser reports value '' with selectedIndex -1 and renders
nothing. The handler then reset the model to '' — a no-op for Vue, whose
value patch compares against the DOM's current '' and skips it. The
placeholder was never re-selected.

The choice is now cleared BEFORE the request, with the pending agent held
in its own ref for the "Granting…" label and restored beside the named
refusal if the grant fails.

The new mounted spec asserts selectedIndex === 0 after a grant — value ''
alone would pass in exactly the broken state. Red on the old code in
jsdom, green on the fix. Learnings entry: every BaseSelect consumer whose
selected option can disappear has this hazard.

Related to Abilityai/trinity-enterprise#596

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* merge-train: capability_grants docstring names the real gate (#2990) — mechanical, per the merge-train note on the PR

The module docstring pointed at `dependencies.assert_agent_capability`, which
does not exist; the gate is `capability_refusal`, applied by
`enforce_agent_capability`.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: trinity-ability <309458136+trinity-ability@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants