Skip to content

Feature/process engine - #6

Merged
vybe merged 34 commits into
mainfrom
feature/process-engine
Jan 17, 2026
Merged

Feature/process engine#6
vybe merged 34 commits into
mainfrom
feature/process-engine

Conversation

@oleksandr-korin

@oleksandr-korin oleksandr-korin commented Jan 16, 2026

Copy link
Copy Markdown
Contributor

Note

Introduces example workflows and foundational docs for the process-driven platform.

  • Adds templates: content-review, customer-support, data-analysis with definition.yaml and template.yaml under config/process-templates/
  • Documents the platform: backlogs (MVP/Core/Advanced), index, development process, and architecture iterations (IT1IT4) in docs/PROCESS_DRIVEN_PLATFORM/
  • Updates docker-compose.yml to set backend TRINITY_DB_PATH=/data/trinity.db
  • Adjusts .gitignore to include src/backend/services/process_engine/repositories/

Written by Cursor Bugbot for commit aa3d56e. Configure here.

- Move PROCESS_DRIVEN_AGENTS.md to PROCESS_DRIVEN_PLATFORM/ folder
- Add IT1-IT4 thinking documents (analysis, architecture, DDD, UI/UX)
- Add phase-based backlog files (MVP, Core, Advanced)
- Add BACKLOG_INDEX.md with conventions and traceability
- Add DEVELOPMENT_PROCESS.md with workflow and testing strategy

This establishes the foundation for implementing the Process Engine feature.
- ProcessId: UUID wrapper with validation and generation
- ExecutionId: UUID wrapper for execution instances
- StepId: Validated string identifier (alphanumeric, hyphens, underscores)
- Version: Major.minor versioning with parsing and comparison
- Duration: Time duration with parsing ("30s", "5m", "2h", "1d")
- Money: Decimal-based currency representation

All value objects are frozen dataclasses for immutability.
Includes 67 unit tests covering all validation and operations.

Refs: IT3 Section 4.3 (Value Objects)
Domain Model:
- ProcessDefinition: Aggregate root with validation, publish/archive lifecycle
- StepDefinition: Entity for individual steps (agent_task, human_approval, gateway)
- StepExecution: Runtime state tracking for step instances
- OutputConfig: Process output configuration

Step Configurations:
- AgentTaskConfig: agent, message, timeout, model, temperature
- HumanApprovalConfig: title, description, assignees, timeout (stub)
- GatewayConfig: gateway_type, routes, default_route (stub)
- TimerConfig, NotificationConfig: stubs for Advanced phase

Validation:
- Duplicate step ID detection
- Invalid dependency reference checking
- Circular dependency detection via DFS
- Empty name/steps validation

Schema & Documentation:
- JSON Schema for editor validation (process-definition.schema.json)
- Example YAML files (content-pipeline.yaml, approval-workflow.yaml)

Includes 27 new tests (94 total process engine tests passing)

Refs: IT3 Section 4 (Aggregates)
Repository Interfaces:
- ProcessDefinitionRepository: Full CRUD + versioning + filtering
- ProcessExecutionRepository: Interface for execution state (impl later)

SQLite Implementation:
- SqliteProcessDefinitionRepository with JSON storage
- Indexed columns for name, status, and name+version uniqueness
- In-memory support for testing

CRUD Operations:
- save: Create or update (upsert by ID)
- get_by_id, get_by_name, get_latest_version
- list_all, list_by_name with filtering and pagination
- delete, exists, count

Version Tracking:
- Multiple versions per process name
- Version major.minor stored separately for efficient queries
- get_latest_version returns latest published

Also: Updated .gitignore to allow process engine repositories folder

Includes 22 new tests (116 total process engine tests passing)

Refs: IT3 Section 7 (Repositories)
ProcessValidator Service:
- validate_yaml(): Full validation pipeline from raw YAML
- validate_definition(): Validate existing ProcessDefinition

Validation Levels:
1. YAML syntax validation with line numbers
2. Schema validation (required fields, types)
3. Semantic validation (from domain layer):
   - No duplicate step IDs
   - All depends_on references exist
   - No circular dependencies
4. Agent existence checking (warnings only)

ValidationResult:
- Separates errors (blocking) from warnings (advisory)
- is_valid: True if no errors
- to_dict(): API-friendly response format

Error/Warning Details:
- message: Human-readable description
- level: error | warning
- path: JSON path (e.g., "steps[0].agent")
- line: YAML line number (when available)
- suggestion: Fix suggestion

Includes 26 new tests (142 total process engine tests passing)

Refs: IT3 Section 6 (Domain Services)
Domain Events:
- DomainEvent: Base class with kw_only timestamp for inheritance
- Process events: ProcessStarted, ProcessCompleted, ProcessFailed, ProcessCancelled
- Step events: StepStarted, StepCompleted, StepFailed, StepSkipped
- Approval events: ApprovalRequested, ApprovalDecided
- All events immutable (frozen dataclasses) with to_dict() serialization

Event Bus Infrastructure:
- EventBus: Abstract interface for publish/subscribe
- InMemoryEventBus: MVP implementation with async dispatch
- subscribe(): Handler for specific event types
- subscribe_all(): Global handler for all events
- unsubscribe(), clear(): Handler management
- Async non-blocking dispatch via asyncio.create_task()
- Error isolation: handler errors logged but don't stop other handlers
- wait_for_pending(): For testing and graceful shutdown

Includes 23 new tests (165 total process engine tests passing)

Refs: IT3 Section 5 (Domain Events)
Added comprehensive tests for execution-side domain model:
- ProcessExecution creation and initialization
- State transitions (start, complete, fail, cancel, pause, resume)
- Step operations (start_step, complete_step, fail_step)
- Query methods (get_completed_step_ids, all_steps_completed)
- Serialization roundtrip (to_dict, from_dict)
- StepExecution entity tests

Fixes gap identified during Sprint 1 review.
31 new tests (196 total process engine tests passing)
Implements REST API for process definitions:
- POST /api/processes - Create new process definition
- GET /api/processes - List all (with filters, pagination)
- GET /api/processes/{id} - Get single definition
- PUT /api/processes/{id} - Update draft definition
- DELETE /api/processes/{id} - Delete draft/archived
- POST /api/processes/{id}/validate - Validate existing
- POST /api/processes/validate - Validate YAML
- POST /api/processes/{id}/publish - Publish draft
- POST /api/processes/{id}/archive - Archive process
- POST /api/processes/{id}/new-version - Create new version

Also marks E2-01 (Execution State Model) as done - was
implemented in Sprint 1 with ProcessExecution aggregate.

21 new API tests (217 total process engine tests passing)
Implements SQLite repository for process executions:
- SqliteProcessExecutionRepository with save/get/delete/list operations
- Tables: process_executions, step_executions
- Proper serialization of Money (as cents), timestamps, JSON data
- Query methods: list_by_process, list_active, list_all with filters

Also adds input/cost fields to StepExecution entity for tracking
step-level inputs and costs (prepares for E2-06 output storage).

21 new repository tests (238 total process engine tests passing)
Implements centralized output storage management:
- OutputStorage service for store/retrieve/delete operations
- OutputPath value object with path pattern /executions/{id}/steps/{step}/output
- Unified API for accessing step outputs regardless of backend
- get_all_outputs() for bulk retrieval
- clear_execution_outputs() for cleanup
- Handles empty dict vs None distinction properly

23 new output storage tests (261 total process engine tests passing)

Sprint 2 complete:
- E1-04: Process Definition API ✓
- E2-01: Execution State Model ✓
- E2-02: Execution Repository ✓
- E2-06: Step Output Storage ✓
Implements the core execution engine for process orchestration:

ExecutionEngine:
- start() - Start new execution from definition
- resume() - Resume paused/partial execution
- cancel() - Cancel running execution
- Timeout handling per step with asyncio.wait_for
- Domain event emission (ProcessStarted, StepCompleted, etc.)

DependencyResolver:
- get_ready_steps() - Find steps with satisfied dependencies
- get_next_step() - Get next step for sequential execution
- get_execution_order() - Topological sort of all steps
- is_complete() / has_failed_steps() - Status checks

StepHandler Interface:
- Abstract base for step type handlers (AgentTask, etc.)
- StepHandlerRegistry for handler lookup
- StepContext for passing execution state
- StepResult for success/failure responses

16 new tests (277 total process engine tests passing)
Implements handler for agent_task step type:

AgentTaskHandler:
- Executes agent_task steps by sending messages to Trinity agents
- Variable substitution for {{input.X}} and {{steps.X.output}}
- Timeout handling from step config
- Cost extraction from agent response

AgentGateway (Anti-Corruption Layer):
- Wraps Trinity's AgentClient for clean process engine integration
- Agent availability checking via Docker container status
- Message sending with context metadata
- Error handling and wrapping

10 new tests (287 total process engine tests passing)
Implements template expression evaluation for process messages:

ExpressionEvaluator:
- Evaluates {{expression}} placeholders in strings
- Supports input.X, input.X.Y for nested input data
- Supports steps.X.output, steps.X.output.Y for step outputs
- Supports execution.id, process.name context
- Strict mode raises ExpressionError for undefined expressions
- Expression extraction and validation utilities

EvaluationContext:
- Path-based access to input_data, step_outputs, metadata
- Handles nested dict navigation

AgentTaskHandler Integration:
- Now uses ExpressionEvaluator for variable substitution
- Supports all expression types in message and agent name

30 new tests (317 total process engine tests passing)
Implements REST API for managing process executions:

Endpoints:
- POST /api/processes/{id}/execute - Start new execution
- GET /api/executions - List executions with filters
- GET /api/executions/{id} - Get execution detail
- POST /api/executions/{id}/cancel - Cancel running execution
- POST /api/executions/{id}/retry - Retry failed execution
- GET /api/executions/{id}/steps/{step_id}/output - Get step output

Features:
- Background task execution via BackgroundTasks
- Status filtering (pending, running, completed, failed)
- Process ID filtering
- Pagination support (limit/offset)
- Full step execution details in response
- Authentication required for all endpoints
- Auto-generated OpenAPI docs

13 new tests (330 total process engine tests passing)
Sprint 4: Definition UI (Frontend)

E3-01: Process List View
- Card grid showing all processes with status badges
- Sorting: newest, oldest, name, status
- Filtering by status (draft/published/archived)
- Actions: Execute, Edit, Delete
- Empty state with Create CTA

E3-02: YAML Editor Component
- Monaco editor integration with dynamic import
- YAML syntax highlighting
- Line numbers and word wrap
- Dark mode theme support
- Inline validation error markers
- Copy to clipboard, Cmd+S to save

E3-04: Process Editor Page
- Full editor with validation panel
- Save/Publish/Execute buttons
- Unsaved changes warning on navigation
- Default YAML template for new processes
- Keyboard shortcut (Cmd+S)

Also:
- Added processes Pinia store
- Added routes for /processes, /processes/new, /processes/:id
- Added Processes link to NavBar
- Added monaco-editor dependency
Sprint 5: Monitoring UI (Frontend)

E4-01: Execution List View
- Table with status icons (✅ ❌ 🔄 ⏳)
- Columns: Process Name, Status, Started, Duration, Cost
- Filters by status and process
- Auto-refresh (30s polling) with pause/resume
- Pagination support
- Cancel/Retry actions

E4-02: Execution Timeline View
- Step-by-step progress display
- Progress bar with completion percentage
- Duration bars relative to longest step
- Status indicators (running animation)
- Click to expand step details

E4-03: Step Detail Panel
- Expandable within timeline
- Timing info: started, completed, duration, retries
- Error display with copy button
- Output loading on demand
- Copy output to clipboard

E4-05: Process Dashboard
- Overall stats: processes, executions, success rate, cost
- Recent executions list
- Published processes with quick execute
- Quick action buttons

Also:
- Added executions Pinia store
- Added routes for /executions, /executions/:id, /process-dashboard
- Added Executions link to NavBar
Fixes:
- Updated deploy scripts to use `docker compose` (V2) instead of legacy `docker-compose`
- Fixed frontend build error by adding `js-yaml` dependency
- Fixed backend crash in `executions.py` due to double `Depends` injection
- Fixed `sqlite3.OperationalError` in `processes.py` by ensuring DB directory exists
- Fixed 404 execution error by aligning DB paths between processes and executions routers
- Fixed `AttributeError` in `processes.py` by using `.major` for version logging
- Fixed UI "Delete" and "Unsaved Changes" modals by passing `:visible="true"` to ConfirmDialog
- Added `TRINITY_DB_PATH` env var to backend service for correct persistence

Features:
- Implemented `EventLogger` service (E15-04) to persist domain events
- Added `SqliteEventRepository` implementation
- Added `Event History` tab to Execution Detail UI
- Completed Process Dashboard implementation (E4-05)

Refs: BACKLOG_MVP.md
…or debugging UI

Sprint 7 implements comprehensive error handling for process execution:

- Add RetryPolicy value object with configurable max_attempts, initial_delay, backoff_multiplier
- Add ErrorPolicy value object with on_error actions (fail_process, skip_step)
- Implement retry logic in ExecutionEngine with exponential backoff
- Fix error code propagation through fail_step chain
- Update API schema to return full error objects (code, message, retry_count)
- Enhance ExecutionTimeline UI to display error details, retry attempts
- Fix WebSocket handler to properly construct error objects from events
- Update dependency resolver to handle skipped steps correctly
- Add comprehensive unit tests for error handling scenarios

Stories completed: E13-01, E13-02, E13-04
Sprint 8 implements parallel step execution for faster process completion:

- Add ParallelGroup and ParallelStructure classes for parallel analysis
- DependencyResolver.get_parallel_structure() identifies parallelizable steps
- ExecutionEngine runs independent steps concurrently with asyncio.gather()
- Add ExecutionConfig.parallel_execution and max_concurrent_steps options
- API returns parallel_level for each step and has_parallel_steps flag
- UI shows parallel indicator (⫘) for steps at same execution level
- Add comprehensive unit tests for parallel detection and execution

Stories completed: E5-01, E5-02, E5-03
- ProcessFlowPreview: Vertical orientation by default with swimlane layout
- ProcessFlowPreview: Parallel steps grouped horizontally with dashed border
- ProcessFlowPreview: Compact design to fit without scrolling
- ExecutionTimeline: Parallel indicator (⫘) and level-based sorting
…ox UI

Sprint 9 - Human Approval:
- Add human_approval step type that pauses execution for review
- Add ApprovalRequest entity and ApprovalStatus enum
- Add approval API endpoints (list, approve, reject)
- Add Approval Inbox UI page with filtering and stats
- Add inline approve/reject buttons in execution timeline
- Fix WebSocket event serialization for StepId objects
- Add non-retryable error handling for APPROVAL_REJECTED
- Add execution resume after approval decision
Sprint 10: Gateways & Triggers implementation

Gateway Step (E7-01, E7-02, E7-03):
- Add gateway step type with exclusive/parallel/inclusive modes
- Implement ConditionEvaluator for boolean expressions (==, !=, >, <, and, or)
- GatewayHandler evaluates conditions and selects routes
- Gateway UI shows route taken and evaluated conditions
- Fix _build_step_outputs for proper condition evaluation

Webhook Triggers (E8-01, E8-02):
- Add TriggerConfig schema (WebhookTriggerConfig, ScheduleTriggerConfig stub)
- Implement /api/triggers endpoints (list, invoke, info)
- Triggers stored in ProcessDefinition and serialized correctly
- Optional secret authentication via X-Webhook-Secret header

UI Improvements:
- Execute input dialog for providing JSON input data
- ProcessList shows Eye icon for published (view) vs Pencil for draft (edit)
- ProcessEditor guards against editing published processes
- "New Version" button for creating drafts from published processes
- Add notification step type (slack, email stub, generic webhook)
- Add webhook event publisher for process_completed/failed/approval_requested
- Add compensation handlers for step rollback on process failure
- Add CompensationConfig value object for type-safe compensation
- Add compensation domain events (Started, Completed, Failed)
- Add trigger management UI with webhook URL copy functionality
- Add execute dialog for JSON input when starting processes

Reference: BACKLOG_MVP.md - E14-01, E14-02, E15-03, E13-03, E8-03
Bugs discovered during manual testing:

1. Notification YAML parsing: Added missing NOTIFICATION branch in
   StepDefinition.from_dict() - inline fields (channel, message, url)
   were not being extracted, causing all notifications to default to
   slack channel.

2. Compensation handler registry: Fixed get_handler() → get() method
   call on StepHandlerRegistry.

3. Compensation webhook URL: Added 'url' field mapping for generic
   webhook channel (was only setting webhook_url for Slack).

Test: compensation-test.yaml triggers intentional failure to verify
compensation handlers execute correctly.
UI/UX improvements for Sprint 11:

1. Retry Execution Tracking:
   - Add retry_of field to ProcessExecution aggregate
   - Link retry executions to original failed execution
   - Display amber "Retry of: <id>" badge with link to original

2. Real-time Compensation Events via WebSocket:
   - Add CompensationStarted/Completed/Failed to WebSocket publisher
   - Add compensation event handlers in useProcessWebSocket composable
   - Display compensation events in Event History without refresh

3. Event Display Improvements:
   - Format both snake_case and PascalCase event types
   - Clear events when navigating to different execution
   - Add 500ms delay before reload to ensure events are persisted
   - Style compensation events with appropriate colors
Sprint 12 Implementation (E9 - Timer & Scheduling):
- Add cron presets (hourly, daily, weekly, monthly, weekdays)
- Add cron expression validation to ProcessValidator
- Extend scheduler service with process schedule support
- Hook schedule registration into publish/archive lifecycle
- Add schedule trigger list/info API endpoints
- Create TimerHandler for timer step type
- Add schedule trigger UI in ProcessEditor
- Display next run time in ProcessList

Test Debt Fix (Sprints 9-11):
- Add test_approval_handler.py (24 tests) - S9
- Add test_gateway_handler.py (18 tests) - S10
- Add test_webhook_triggers.py (14 tests) - S10
- Add test_notification_handler.py (20 tests) - S11
- Add test_compensation.py (22 tests) - S11
- Add test_schedule_triggers.py (23 tests) - S12
- Add test_timer_handler.py (11 tests) - S12

Bug fixes:
- Fix timer.py: context.step -> context.step_definition
- Remove unused import in timer.py

Total: 466 tests passing (was 374)
Sprint 14 Implementation:
- E11-02: Process Analytics Dashboard with metrics, trends, step performance
- E11-03: Cost Alerts system with thresholds (per-execution, daily, weekly)
- E12-01: Process Template Library with bundled templates
- E12-02: Template creation from published processes

UI Improvements:
- Add Process sub-navigation (Processes, Dashboard, Executions, Approvals)
- Add Agent sub-navigation (Agents, Files, Templates)
- Declutter main nav from 8 to 6 items
- Template selector in process creation flow

Backend:
- Analytics service with ProcessMetrics, TrendData, StepPerformance
- CostAlertService with threshold management and alert generation
- ProcessTemplateService for bundled and user templates
- New API endpoints for analytics, alerts, and templates

Frontend:
- TrendChart component for execution/cost visualization
- TemplateSelector component for process templates
- ProcessSubNav and AgentSubNav components
- Alerts page and NavBar badge integration
- Enhanced ProcessDashboard with analytics

Tests:
- 59 new unit tests for analytics, alerts, and templates
Sprint 15 - Agent Roles (EMI Pattern):
- Add AgentRole enum (Executor/Monitor/Informed)
- Add StepRoles entity with validation
- Create InformedAgentNotifier service for async notifications
- Add RoleMatrix.vue component for interactive role management
- Integrate roles tab in ProcessEditor with UI-YAML sync
- Extend ProcessValidator for role validation across all step types

Sprint 12-14 Completion:
- E9-01: Cron validation already implemented (verified)
- E10-02: Add breadcrumb navigation for nested sub-process executions
- E11-01: Cost tracking already implemented (verified)
- E11-03: Integrate CostAlertService with ExecutionEngine
- E12-01: Add customer-support bundled process template

Tests:
- Add test_roles.py for EMI pattern validation
- Add test_informed_notifier.py for notification service
- Add ExecutionEngine cost alert integration tests

Process engine adds ~27,400 LOC (~41% growth to Trinity codebase)
- Move PROCESS_DRIVEN_PLATFORM from docs/drafts/ to docs/
- Add IT5 thinking iteration (scale, reliability, enterprise)
- Add Process Engine roadmap with 14 test processes across 5 phases
- Create feature flow documentation for Process Engine:
  - README overview, process-definition, process-execution
  - process-monitoring, human-approval, process-scheduling
  - process-analytics, sub-processes, agent-roles-emi, process-templates
- Update requirements.md: Add Section 18 (Process Engine) with all features
- Update roadmap.md: Mark Phase 14 complete, add decision log entry
- Update architecture.md: Add Process Engine section, API endpoints, DB schema
- Update feature-flows.md index with Process Engine section
vybe pushed a commit that referenced this pull request Aug 13, 2026
… client can read (#2128) (#2143)

* fix(workspace): carry a rooms capability on the roster the client can read (#2128)

A chat with two or more agents is a room, and `/api/rooms` is served only by a
module a community build does not have. `PortalAgentPicker` offered multi-select
regardless, so picking two agents dead-ended in a generic "Could not start that
chat." — an affordance always offered that can never work.

The signal has to reach a PORTAL principal: an external client on an email-OTP
session with no platform account. That principal cannot read
`/api/settings/feature-flags` (it is `get_current_user`-gated), and the frontend
entitlement store behind it returns `[]` for any caller without a platform JWT —
so gating on it there would hide multi-select from EVERY external client,
including on an instance where rooms are present.

`GET /my-agents` is the payload that reaches both principal kinds, is awaited
first by the shell's bootstrap, and already carries a per-agent capability
boolean (`voice_available`). One field on it: no new route, no new auth surface,
no extra round-trip.

`_multi_agent_chat_available()` reads the entitlement registry rather than
probing the route table — a module that claims its id and then fails to mount
has that claim withdrawn (ent#196), so the registry already answers "mounted AND
serving". It is fail-CLOSED: an unreadable registry reports the capability
absent, because promising an affordance that cannot work is the bug being fixed.

Two details are load-bearing rather than stylistic:

* the import is function-local. `_set_for_testing` rebinds a module global, so a
  module-scope `from ... import entitlement_service` would freeze the boot-time
  instance and silently bypass the seam. Same form as `dependencies.py` and
  `routers/settings.py`, pinned by the static guard in
  `test_926_version_endpoint.py`.
* `bool(...)` is not decoration. A leaked MagicMock for the registry makes
  `is_entitled` return a mock, and pydantic COERCES that to True on a bool field
  rather than raising — the field would silently report *available*.

The field is named for the capability, never the module or the edition: this
payload is served to an operator's customer, who can neither buy a missing
module nor act on knowing it exists.

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

* test(workspace): pin the rooms capability bit and its fail-closed direction (#2128)

Nine cases (11 with parametrisation) over `get_roster`, on the throwaway-sqlite
harness from `test_ent357_workspace_owned_roster.py`.

What each guards, in order of how badly it would fail:

* B3 — the bit is derived from the ROOMS feature id specifically. A different
  registered module must not turn it on; copying the frontend's `hasAnyEnterprise`
  shape would advertise rooms on any instance carrying any other paid module.
* B5a/B5b — an unreadable registry (raises, or has no such method) reports the
  capability ABSENT and still returns the roster. Fail-open here would
  reintroduce the exact bug.
* B8 — the helper reads the LIVE singleton: it swaps the registry twice against
  one already-imported module, so a cached module-scope binding cannot satisfy
  both halves. This is what makes the function-local import load-bearing.
* B4 — `TRINITY_OSS_ONLY=1` wins over a registered module, with the service
  constructed AFTER `setenv`, because `_oss_only` is read once in `__init__` and
  the obvious spelling passes without proving anything.
* B9 — asserts `sys.modules["services.entitlement_service"]` is the real module,
  not a leaked stub, before trusting any assertion above it.

Mutation-checked rather than asserted: flipping the except arm to `return True`
fails B5a/B5b; swapping `is_entitled(...)` for a truthiness test on
`list_entitled_features()` fails B3; hoisting the import to module scope fails B8.

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

* fix(workspace): make the agent picker single-select without a rooms engine (#2128)

The selection decision moves into two pure functions in `portalUtils.js`, because
there is no component-mount harness in this project (no @vue/test-utils) — a
decision that lives inside the component is a decision that cannot be tested.

`applyAgentSelection` gives single-select replace-on-click and click-to-clear;
multi-select behaviour is byte-identical to what shipped. `collapseSelection`
handles the capability flipping while the dialog is open (a late roster, or the
store's self-heal firing), so the user is never left holding a two-agent
selection that Start can no longer confirm. Both return a new array.

The `multi` prop defaults to FALSE, not true: a caller that forgets to bind it
gets the surface that works on every edition rather than reintroducing this bug.

Accessibility: single mode drops `role="checkbox"`/`aria-checked` and uses
`aria-pressed` on what is already a native `<button>`. Not `role="radio"` —
`radiogroup` carries a roving-focus/arrow-key contract this does not implement,
and a radio cannot be un-checked by activating it, which contradicts
click-to-clear. Multi mode keeps its checkbox semantics untouched.

No "not available" or upgrade copy anywhere in the picker. The viewer here is
the operator's customer; a disabled control naming a licensing reason would put
the operator's billing tier in front of their client to no purpose. The
unreachable "they will share this conversation" clause is simply not shown.

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

* fix(workspace): refuse room calls at the store, and self-heal on a real 404 (#2128)

Invariant #6 puts API calls behind stores, so the refusal chokepoint belongs
here rather than in the view. `_requireRooms()` throws a typed
`code: 'rooms_unavailable'` (never message-sniffing) so the view can tell this
apart from a transport failure, which needs different copy.

Applied to all FIVE room actions, not the two reachable today. Three have
exactly one caller each, inside a component the render gate stops mounting — so
gating them is redundant *for the current call graph*, which is precisely the
claim `learnings.md` 2026-07-01 records failing: a kill-switch is only as
airtight as its least-gated entry point, and the construction is one `v-if` in a
file a sibling issue proposes to rewrite. Cost: three lines.

`fetchRooms` returns `[]` rather than throwing — its caller already treats "no
rooms" as normal, and the early return removes a guaranteed-4xx round-trip on
every sidebar refresh.

Flag lifecycle, three deliberate choices:

* RAISED only by a successful roster, strict `=== true`, so an older backend
  omitting the field (or a proxy returning the string "false") reads as absent.
* NOT pre-reset at the top of `fetchRoster`. The sibling `unavailable` reset
  points the opposite way — it resets to the OPTIMISTIC value so a stale 404
  cannot paint over a roster that loaded fine — and copying its shape here
  inverts its meaning: every background refetch would unmount a live room and
  flash a refusal at an entitled client before taking it back.
* LOWERED by a definitive 404/403 from the room endpoint itself. Entitlement can
  vanish between roster load and confirm, and without this the picker still
  dead-ends mid-session on the generic copy. A 5xx or a network error is "could
  not ask", not "is absent", and must not lower it.

`rosterLoaded` is assigned in the terminal arms and CONDITIONALLY on still being
signed in. The 401 branch signs the session out inside the catch, so a bare
`= true` in the catch — or in `finally`, which runs later still — re-sets what
the sign-out just cleared, and the field stops meaning "a verdict was reached
for this session".

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

* fix(workspace): refuse a room URL honestly, and let the user leave it (#2128)

`<PortalRoom>` gains the capability as a term in its own `v-if`. Gating the
RENDER rather than a watcher is what stops `PortalRoom::onMounted` issuing
`GET /api/rooms/:id` at all — on a community build that 404 fires before the
roster await can possibly resolve.

The new branch's POSITION is load-bearing. ANDing the capability onto the
existing `v-if` alone produces two distinct bad outcomes: if `activeAgent`
resolves, the user lands in a DIFFERENT agent's conversation under a room URL
(it defaults to the first roster entry); if it is null, the chain falls to the
`!activeRoomIdFromRoute` block whose guard is false here, rendering a completely
blank `<main>`. So the branch sits between the two components and catches every
remaining room-URL case.

Four sub-states, not one refusal. Fail-closed is right for the affordance and
wrong for the copy explaining it: during a transient 5xx on an instance where
rooms ARE present, a single message would tell the client "chats with more than
one agent aren't enabled here" — a false statement about the operator's build,
on the surface whose whole bar is honest status. Only a roster that loaded
CLEANLY and reported the capability absent may say that. The
`!rosterLoaded || loading` arm is what prevents a refusal flash on a hard-loaded
room URL.

`onPickerConfirm` gets ONE new arm mapping the typed `rooms_unavailable` code to
its message. The generic path is deliberately left intact — a true flag does not
guarantee success (the client may lack access to one selected agent) and that
reason still has to surface. The one-agent branch is untouched.

`startBlankChat`, `newChatWithAgent` and `onSignOut` now test `route.params.roomId`
alongside `sessionId`. Each navigated only on `sessionId`, so from a room URL
none of them changed the route — invisible while the room always rendered, but
the moment a room URL can resolve to a refusal it makes that refusal a state no
control except its own button can leave. A dead end created by the fix meant to
remove one.

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

* test(workspace): pin the rooms gate — pure decisions, store contract, structure (#2128)

28 cases in three layers, matching what this project can actually test.

Pure decisions (F1-F9) — single vs multi selection and the collapse case. These
exist as pure functions precisely so they are reachable without a component
harness.

Store contract (F14-F22) — the flag is raised only by a success and strict
`=== true`; a transport failure does NOT lower it; a 404/403 from the room
endpoint does and a 500 does not; `signOut` and a 401-during-load leave both
flags cleared; all five room actions refuse with NO axios call; and with the flag
raised the same actions issue their requests exactly as before, so the gate is
not a one-way ratchet.

Structure guards (F10-F12, F18, F23-F24) — for the parts no unit test can reach:
a prop default, a template branch ORDER, a `v-if` term, the honest-copy split,
and the three navigation guards. Each strips comments first, because a comment
explaining what not to write necessarily contains the offending string.

Three of these were vacuous as first drafted and are anchored deliberately:

* F10 matches the `multi` KEY, not a bare `/default:\s*false/` — the picker
  already carries `busy: { type: Boolean, default: false }`, so a loose pattern
  passes with the prop deleted entirely.
* F12 matches the exact `v-else-if="activeRoomIdFromRoute"` string, since
  `indexOf('activeRoomIdFromRoute')` hits the `PortalRoom` `v-if` first and
  measures nothing, and asserts both indices `> -1` BEFORE comparing — `indexOf`
  returns -1 on absence and `expect(-1).toBeLessThan(x)` passes, so a deleted
  branch would go green.
* F18 exists because nothing else implements AC #4; someone simplifying the
  chain could drop the capability from `PortalRoom`'s `v-if` with every other
  test still passing.

Every guard was meta-tested by planting its violation: default flipped to true,
`multi` prop deleted, store guard moved after the POST, branch moved after
`<PortalConversation>`, branch deleted outright, capability dropped from the
`v-if`, pre-reset restored at the top of `fetchRoster`, self-heal removed,
`roomId` dropped from the three navigation guards, and the copy split collapsed
to one message. Each fails the case that claims to cover it.

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

* docs(workspace): name the roster as the portal capability channel (#2128)

`architecture.md` — three scoped edits to the Workspace section:

* Declares the roster payload as *the* capability channel for this surface, with
  the reason: a portal principal cannot read `/api/settings/feature-flags`, and
  the frontend store behind it returns `[]` for any caller without a platform
  JWT. Without this sentence the next author reaches for the entitlement store
  and hits that trap again — which is why it is a stated rule and not a
  description of one field.
* Documents `multi_agent_chat_available`: resolved once per roster load,
  fail-closed, named for the capability rather than the module or edition, and
  what false actually does (single-select picker, five refusing store actions,
  an honest room-route refusal, self-heal on a definitive refusal). States
  plainly that the frontend gate is UX, not containment — a portal token
  legitimately reaches those endpoints where they exist.
* Corrects the stale "Chats are strictly single-agent" clause, falsified by
  ent#361 and made conditional by this change.

Describes the mechanism only — the module id is deliberately not printed here
(standing rule); the id in `service.py` is fine and is already public in the
user docs and the MCP tool.

`docs/user-docs/collaboration/rooms.md` — "and the Sessions view is hidden" has
been false since ent#381 (the view is gone; nothing is hidden). Replaced with
the Workspace behaviour this change creates.

No `requirements/` entry: this is a bug fix restoring a gate that #2120 dropped,
it adds no capability, and the requirements entry for multi-agent Workspace chat
is an explicit acceptance criterion of the separate documentation issue.

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

* docs(workspace): correct the bool()-cast rationale to what was measured (#2128)

The plan justified the cast as a defence against a leaked `MagicMock` for the
entitlement registry in a polluted test order. Measured, that is false in both
halves:

* a MagicMock is truthy, and pydantic coerces one to True on a bool field, so
  the field reads *available* with the cast or without it — the cast defends
  nothing there;
* the two files named as polluters both RESTORE the key (`patch.dict(...).stop()`
  in a fixture `finally`, plus `monkeypatch.delitem`), so there is no live leak.
  Proven on the full unit tier with the cast dropped: the new tests still pass.

What does catch a leak is the module-identity assertion in the test file — with a
real unrestored `MagicMock` planted ahead of it, seven cases fail loudly rather
than one going green on a lie.

The cast is kept for the reason that survives: it makes the return match the
annotation for any registry implementation, so a non-bool falsy value (None from
a partial stub) fails CLOSED instead of raising a ValidationError on the response
model — which would be a 500 raised outside this function's try.

A comment that states a protection the code does not provide is worse than no
comment: it is what stops the next reader looking for the real one.

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

* fix(workspace): a room refusal the module authored is not an absent engine (#2128)

`_noteRoomsRefusal` lowered the capability on ANY 403/404 from the rooms
endpoints. On a fully entitled instance that is wrong, and reachable by
ordinary use: the serving module answers "you do not have access to agent
'x'" with a 403 (`agent_not_accessible`) and "you are not in that room"
with a uniform 404 (`room_not_found`).

So a client who picked an agent that had just been un-shared got the whole
workspace switched to single-select for the rest of the session, every open
room unmounted, `/workspace/r/:id` claiming "this conversation isn't
available on this instance" — and the one message that actually told them
what to do ("You do not have access to agent 'x'") overwritten with a false
statement about the operator's build. The flag only returns on the next
`fetchRoster()`, so the damage lasted the session.

The plan named this exact case ("a true flag does not guarantee success —
the client may lack access to one selected agent, and that reason must
still surface") and the implementation defeated it.

Absence and denial are separable by the BODY, because different layers
author them: a module that is serving answers with its own structured
`detail: {code, message}`; absence is a plain string — FastAPI's "Not
Found" for a route that was never mounted, and the entitlement gate's
one-sentence 403 for mounted-but-unlicensed. A coded detail proves the
substrate is present, so it is passed through untouched and the caller's
existing generic path surfaces the server's own words.

F20c/F20d/F20e pin all three directions; removing the two-line
discriminator fails exactly F20c and F20d.

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

* fix(workspace): converge an OPEN room when the rooms engine goes away (#2128)

The self-heal was wired to `createRoom` and `fetchRooms` only. That covers
the picker, and leaves the case AC #4 is actually about: a client already
sitting in `/workspace/r/:id` when the entitlement lapses.

Nothing else converges it. `refreshThreads()` is event-driven, not
periodic, so the sidebar refresh that would have lowered the flag may never
run; `PortalRoom`'s 3s poll reports nothing unless the load is `full`; and
`send()` reads only `err.response.data.detail`, so the user gets "That
message was not delivered." on every attempt with no path forward. The gate
was correct at load and the room stayed a dead end for the rest of the
session — AC #1's failure mode, reached through AC #4's own surface.

Wiring `_noteRoomsRefusal` into the remaining three closes it: the poll's
own 404 lowers the flag, `<PortalRoom>` unmounts on the next tick, and the
room route's honest refusal (with its way out) takes the stage.

This is only safe because of the preceding commit. `/api/rooms/:id` answers
a uniform CODED 404 for a room the caller is not a member of, so on the
status alone one stale room link would have switched an entitled workspace
to single-select — F20g pins that it does not.

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

* refactor(workspace): write the stage ink once, so Portal.vue's raw-color count shrinks (#2128)

CLAUDE.md §9 and the design-system contract ratchet raw palette usage:
per-file counts may only shrink. Measured, not assumed:

  committed baseline (2026-07-31)  raw_gray 20
  origin/dev tree                  raw_gray 40   (ent#361 doubled it, unenforced)
  this branch before               raw_gray 56   (+16 from the new refusal block)
  this branch after                raw_gray 24

"The baseline was already stale" is not a defence — that is how a ratchet
dies — so the growth is removed rather than excused.

Converting the additions to semantic tokens, the usual remedy, is not
available here and it is worth saying why once: `gray` has NO semantic
token. It is the design system's residual family — the contract's colour
section ends "Everything else is gray" and then prescribes these exact
shades as the light/dark ink ladder — so there is nothing for
`text-gray-500 dark:text-gray-400` to become. Repainting body copy in
`status-*` or `action-*` would be a defect wearing a compliance badge, and
the scanner itself classes raw_gray as "partially sanctioned", reserving
"hard violation" for raw_nongray (0 here, before and after).

What is actually fixable is the number of PLACES a raw shade is written.
This file renders nine empty/refusal stages — a room URL this instance
can't open ×4, an unreachable agent, an empty roster ×4 — and each carried
its own copy of the same class strings. Hoisting them to five named
constants collapses twenty-odd sites to five, which is the migration
surface a future gray token would have to cross, and it makes design-system
principle 4 (loading/loaded/empty/failed share one footprint) structural
instead of a copy-paste convention that every new state re-negotiates.

Rendered output is byte-identical; verified the hoisted classes survive
Tailwind's content scan in the built bundle (it reads whole literals from
the script block). No template structure changed, so every source guard
still bites — re-proved by re-planting the branch-order, branch-deleted,
PortalRoom-v-if, nav-guard and honest-copy-split mutations.

Honest residual: 24 is still above the stale baseline's 20. The remaining
16 are the sign-in shell and root container, untouched by this PR and not a
duplication problem; regenerating the baseline is deliberately NOT done
(it would launder 141 unrelated files' drift).

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

* test(workspace): pin the room branch's other neighbour too (#2128)

F12 asserted the refusal branch precedes <PortalConversation — the
fall-through that opens a different agent's chat under a room link. The
opposite edge was left to inference: put the refusal FIRST and it shadows
<PortalRoom> outright, so an entitled instance never opens a room at all.

That is currently prevented by Vue refusing to compile a `v-else-if` with
nothing before it, which is a guarantee about the framework rather than
about this file, and it evaporates the moment anyone reshapes the chain.
Assert it directly instead.

Verified by moving <PortalRoom> below the branch (keeping `v-else-if`
intact, so the file still compiles): F12 fails.

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

* docs(architecture): the self-heal reads the body, not just the status (#2128)

The paragraph said "a definitive 404/403 from the room endpoint self-heals
the flag", which is the imprecision that produced the bug two commits ago:
on an entitled instance the serving module answers "you cannot reach that
agent" with a 403 and "you are not in that room" with a uniform 404, and
reading either as absence turns one denied request into a session-long
false claim about the operator's build.

State the discriminator instead — a serving module authors its own refusals
as a structured coded `detail`, absence is a plain string — and record that
the heal covers all five calls because the sidebar refresh is event-driven,
so nothing else converges a room that is already open.

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

---------

Co-authored-by: trinity-ability <309458136+trinity-ability@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
vybe pushed a commit that referenced this pull request Aug 13, 2026
…gent (#2160) (#2165)

Profiled before fixing, as AC1 asks; findings recorded on the issue.

`build_page` is NOT the bottleneck. Its six sub-reads total **32.7 ms** against a
1600-execution agent (health 4.0, stats 12.5, asks 3.1, recent work 2.1,
last-active 4.9, reports 6.0). Parallelising them would buy ~20 ms and is not
worth the concurrency, so this PR does not.

The endpoint took ~150 ms, and the gap was one line I wrote in ent#360:

    roster = await service.get_roster(email, include_owned=...)
    card = next((a for a in roster.agents if a.name == agent_name), None)

Opening ONE agent's page built the entire roster — and `get_roster` fans
`_agent_briefing` across every agent (a Docker lookup plus up to two agent HTTP
calls each, 5s timeout, awaited with `gather`). So the page paid for N briefings
to use one, and inherited the roster's floor (#2163): its load time was bounded
by the SLOWEST agent in the fleet, not by the agent being opened. One wedged
agent meant a five-second page for an unrelated one.

`get_agent_card` does exactly one. Measured on the live instance: **12 briefings
→ 1**, and the endpoint 150ms → ~110ms on a fleet where only 3 agents are
running (the local delta understates it — the point is that the cost no longer
grows with the fleet, and a test pins that at 200 agents).

The original intent is preserved: ent#360 projected the roster's card so the page
and the sidebar could not disagree about an agent's capabilities. Both paths now
go through the same `_row_to_card` / `_roster_rows` helpers, so they still
cannot, and membership resolves by the same ent#357 rule (owned agents only for
a platform session) rather than a second one that could drift.

Also: window-keyed `${name}:${window}` cache (AC5) with stale-while-revalidate,
so flipping 7d→30d→7d stops refetching an identical payload while never showing
stale numbers; and section-shaped skeletons instead of one "Loading…" line (AC4).

Two corrections to the issue's technical notes, verified: `_health` does NOT
reach the live container — it reads the last persisted `agent_health_checks` row
(4 ms), precisely so the page renders for a stopped agent (ent#360 AC #6). And
there are no unbounded scans: recent work and reports are LIMIT 20, last-active
LIMIT 1, stats the #1107 windowed accessor. AC3 had nothing to fix.

The test pins the COUNT, not a duration: a timing assertion passes on a
two-agent fixture no matter how the code is written, which is exactly how this
shipped.

Closes #2160

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: trinity-ability <309458136+trinity-ability@users.noreply.github.com>
AndriiPasternak31 added a commit that referenced this pull request Aug 16, 2026
Prevents the #1895 trap from recurring. New stdlib-AST guard
tests/lint_root_test_placement.py (never imports/execs a test module), wired into the
existing unconditional lint-sys-modules job (push + PR, never path-filtered):

  Part 1 — root placement. Every root-level tests/test_*.py must EITHER request a live
    fixture (api_client / created_agent / ws_ticket / …) it does not locally redefine,
    OR carry `# allow-root-live-test: <reason>`, OR be grandfathered in
    lint_root_test_placement_baseline.txt (ratcheted — never grows). A new
    self-contained root test fails → move it to tests/unit/.
  Part 2 — async markers. Every `async def test_` under tests/unit/ must have a
    resolvable pytest.mark.asyncio (own decorator, class decorator, class pytestmark,
    or module pytestmark). tests/unit/ runs pytest-asyncio strict, so an unmarked async
    test silently does not run; CI's pytest step is `|| true` and the diff gates on
    failing IDs, so this guard is how the strict-mode footgun becomes a hard failure.

Initial baseline = the 11 live-in-root files at adoption (raw-httpx/ws live tests,
trinity_cli-dependent CLI tests, and the currently red-and-hidden files that stay put —
see the batch commit messages). Verified 0 pre-existing unmarked async under tests/unit/
(the detector handles the class-decorator form many unit files use, e.g. test_backlog).

Durable rule (AC #6): tests/README.md gains a "Where does a new test go?" section;
tests/unit/conftest.py gains a placement-rule header; tests/conftest.py gains a stay-put
pointer near collect_ignore (the live-in-root set is enforced by the guard + baseline,
not a hand-maintained list) and its now-stale routers-preload docstring is refreshed (the
plain-stub installer left the root in #1895).

No architecture.md / requirements.md section governs test-file layout — nothing to update
there (Trinity Rule #1 satisfied). #1895 delivers collection + per-PR visibility; making
the job a *required* gate is #1958 (separate PR, deliberately deferred — a still-
stabilizing suite must not be made required, the #1228 class).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vybe pushed a commit that referenced this pull request Aug 16, 2026
…1895) (#2227)

* test(ci): relocate test_canary_invariants into tests/unit so per-PR CI collects it (#1895)

Batch 0 (canary, solo). The canary-invariant suite (~174 tests, the only file
exercising the canary alert composer incl. the G-04 credential-leak check) sat in
tests/ root, which no per-PR CI job collects (backend-unit-test.yml runs
'pytest unit/'; tests/unit/pytest.ini seals the island with 'norecursedirs = ..').
This is the exact gap that let #1880 ship. Moving it makes it a per-PR gate.

- git mv tests/test_canary_invariants.py -> tests/unit/
- drop the dead root-conftest neuter overrides (api_client()->None,
  cleanup_after_test) — inert in the unit island; letting the unit conftest's
  autouse cleanup run is strictly safer (more sys.modules restoration).
- drop the STALE 'test_canary_invariants.py' sys.modules baseline line: the file
  uses the _STUBBED_MODULE_NAMES/_restore_sys_modules escape hatch, so its current
  finding count is 0 at either path (verified) — relocating the stale 5 would just
  move an over-count.

Case A (sys.path.insert only) — no path re-anchor needed. Verified: pre-move
--co count (root config) 174 == post-move --co (unit) 174; 172 passed / 2 skipped
under seeds 12345/67890/99999; RuntimeWarning-clean; lint_sys_modules exit 0.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(ci): relocate clean self-contained unit tests into tests/unit (#1895)

Batch 1. Case-A movers (sys.path.insert only — no path re-anchor); baseline lines
relocated by path. Each verified: pre==post --co nodeid count, green under CI seeds
12345/67890/99999.

  watchdog_unit(43) proactive_audit_unit(4) self_execute(22) telegram_login_gate(5)
  whatsapp_adapter(83) access_grant_notification_unit(4, escape-hatch helper -> no
  baseline entry)

Two Attempt-3-driven deviations from the plan's static Batch-1 list (the trial is
the only reliable classifier, per AC #1):

- test_platform_default_model.py is MIXED, not a clean mover: 9 of 12 tests make a
  real httpx.post(BASE_URL) (Connection refused, no live backend). SPLIT it — the 3
  self-contained TestGetPlatformDefaultModelUnit tests (which hold all 6 sys.modules
  findings) -> tests/unit/; the 9 live feature-flags/settings-API tests stay in
  tests/ root with an allow-root-live-test marker.

- test_cb_probe_execution_close.py STAYS PUT (reverted): it is red-and-hidden, not
  passing coverage. Standalone it fails on two product paths — the #1804
  close_execution_activity await (its mock is a plain MagicMock, never updated) and a
  seed-dependent scrub_secret_and_urls ImportError. Moving it would redden the per-PR
  gate (AC #5). Fixing its drift is a separate bug; its sys.modules baseline line is
  left at the root path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(ci): relocate Case-B path-depth unit tests into tests/unit (#1895)

Batch 2. Files whose backend import base is a __file__-relative
spec_from_file_location/exec_module path: after the move one extra '..' hop
re-anchors the single base definition (all exec_module sites derive from it).
Verified: pre==post --co count, green under seeds 12345/67890/99999,
FileNotFoundError gone, lint_sys_modules exit 0.

  test_event_bus(23, baseline 1)  test_password_validation(25, no baseline)
  test_platform_prompt_unit(41, parametrized -> --co count, baseline 4)
  test_sharing_null_email_unit(5, no baseline)

Deviation (Attempt-3): test_github_pat_propagation_unit.py STAYS PUT (reverted).
Confirmed red-and-hidden — it fails the same 4/11 in root config on origin/dev:
its _fake_database stub predates the service's runtime
'from database import AgentGitConfig', so those 4 tests error regardless of
location. Moving it would redden the per-PR gate; its stale stub is a separate bug.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(ci): relocate test_idempotency into tests/unit with explicit async markers (#1895)

Batch 3a (special, solo). The #1084 effect-guard suite — 55 tests incl. 26 async,
ZERO of them carrying an asyncio marker. Under tests/unit/pytest.ini (strict mode,
no asyncio_mode) an unmarked async test does not run — empirically it FAILS
("async def not natively supported") on the current CI toolchain (pytest 9.1 /
pytest-asyncio 1.4). So the move needs explicit markers or the 26 async silently
stop testing (a false-green hole reintroduced by the fix itself).

- git mv -> tests/unit/; re-anchor _backend_path with one extra ".." (Case B: 6
  exec_module sites derive from it).
- class-level "pytestmark = pytest.mark.asyncio" on the 6 all-async classes (NOT
  module-level, which would decorate the 5 sync classes and warn "not async").
- drop the dead root-conftest neuter overrides (api_client/cleanup_after_test).
- TestProactiveSendMessageGuard (4 tests) SKIPPED (documented): red-and-hidden on
  origin/dev too — its exec_module of proactive_message_service.py fails the
  module-level "from services.settings_service import get_proactive_rate_limit"
  (#1609) against the fixture's services stub. Skipped not fixed (stale stub is a
  separate bug), so the file gates the other 51 without reddening the per-PR gate.

Verified across seeds 12345/67890/99999: --co 55 (== pre-move), 51 passed / 4
skipped, 22 async RAN (passed, not no-op'd), coroutine-never-awaited -W error clean,
lint_sys_modules exit 0 (0 bare findings — uses monkeypatch.setitem).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(ci): relocate test_inter_agent_timeout_unit + migrate routers preload (#1895)

Batch 3b (special, solo). The routers-stub file: at collection it installs a PLAIN
`routers` module (types.ModuleType, no __path__). In the unit island that breaks a
later `import routers.public` (test_ip_rate_limit_fix unit half) with "routers is
not a package". The root conftest defused this via
_preload_backend_routers_namespace(), but the unit island (norecursedirs = ..)
never loaded the root conftest.

- Migrate _preload_backend_routers_namespace() into tests/unit/conftest.py, called
  at module level before collection, so the file's `if "routers" not in sys.modules`
  guard fires and the plain stub is never installed (and any plain module is
  upgraded to a namespace package).
- git mv -> tests/unit/; re-anchor _backend_path with one extra ".." (Case B: two
  exec_module sites derive from it). Relocate baseline line (9).

Verified across seeds 12345/67890/99999: --co 7 (== pre-move), 7 passed, no "routers
is not a package", lint_sys_modules exit 0.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(ci): split test_validation — pure-logic classes into tests/unit (#1895)

Batch 3c. test_validation.py is mixed: 5 pure-logic classes
(TestValidationPromptBuilding / ...ResponseParsing / BusinessStatusMapping /
TestDatabaseOperations / TestScheduleValidationConfig) that stub the backend at
module import, plus TestValidationIntegration whose methods take api_client /
created_agent.

- Move the 5 pure-logic classes -> tests/unit/test_validation.py (they carry all 4
  module-level sys.modules stubs; baseline line relocated to the unit path).
  sys.path.insert base (Case A) — no re-anchor. Verified: --co 19, 16 passed / 3
  skipped under seeds 12345/67890/99999.
- TestValidationIntegration stays in tests/ root (auto-exempt: it requests the
  live api_client/created_agent fixtures). 0 sys.modules findings there.

Deviation (Attempt-3): test_ip_rate_limit_fix.py is NOT split — the whole file
(unit tests included) is red-and-hidden. _load_public_router imports routers.public,
which now does `from dependencies import ..., assert_owns` (#1310); the fixture's
MagicMock `dependencies` stub raises on an assert_*-named attribute (mock's
assertion-name guard), so all 14 tests error identically on origin/dev. It stays put
and is baselined by the new placement guard.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(ci): relocate 5 more self-contained unit tests found by trialing (#1895)

Batch 4. The plan flagged several ambiguous root files to trial; these five are
genuinely self-contained and green in the unit island, so they move (Attempt-3):

  test_agent_health_signal(7)  test_timeout_termination(8, async 8/8 marked - RAN
  verified, coroutine -W error clean)  test_archive_security(24)  test_lint_sys_modules(25)
  test_team_share_gate_unit(8)

- test_archive_security was collect_ignore'd in tests/conftest.py, so it ran in NO
  job at all — the literal #1895 symptom. It's self-contained (24 pass in unit);
  moved and the stale collect_ignore cleared.
- test_team_share_gate_unit asserts its computed backend path exists, so it needs
  the one-extra-".." re-anchor even though the base is only sys.path.insert
  (a plain Case-A insert would fail its assertion under pytest-randomly).
- The other three are Case A (sys.path.insert only, conftest already on-path).

All 0 sys.modules findings (no baseline change). Verified --co == pre-move and green
under seeds 12345/67890/99999; lint_sys_modules exit 0.

Trialed-and-kept-in-root (red-and-hidden or live, baselined by the new guard):
test_audit_log_unit (2 fail), test_log_archive (19 errors/live), test_cli_profiles
(trinity_cli not installed).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(ci): stop three relocated files leaking sys.modules stubs at collection (#1895)

test_platform_prompt_unit, test_inter_agent_timeout_unit and test_validation install
bare MODULE-LEVEL sys.modules stubs to load a backend module in isolation:
  - sys.modules["database"] = <SimpleNamespace> (platform_prompt)
  - sys.modules["services.task_execution_service"] = <fake> (inter_agent, validation)
In the tests/ root these never shared a collection with the files that import the
real module at collection time; the #1895 relocation puts them in the same island,
and pytest aborts the WHOLE session on a collection error, so these two leaks
reddened all ~10k unit tests:
  test_retention_floor  -> routers/settings.py `from database import SystemSetting`
  test_runtime_resolution_backend -> `from services.task_execution_service import
                                       _resolve_agent_runtime`
The conftest's per-test autouse restore runs only once collection has finished — too
late for a cross-file *collection*-time leak.

Fix: each file now declares the sanctioned _STUBBED_MODULE_NAMES/_restore_sys_modules
pair (the lint-recognised escape hatch, precedent
tests/unit/test_telegram_webhook_backfill.py) — it snapshots the modules it stubs and
restores them once its collection-time module loads have bound what they need. This is
safe for the files' own tests (they run under the conftest's per-test restore, so the
real modules are what execute) and undoes the leak before the next file imports. Being
lint-exempt via that pair, their three lint_sys_modules baseline entries are removed
(the baseline shrinks).

(An earlier attempt used a broad pytest_collectstart hook restoring the whole baseline
before every module import — reverted: popping passlib/dependencies/services.* before
each of ~550 modules churned import identity (768 failures) and was pathologically slow
(2 min for 4 files). The per-file restore is surgical and cheap.)

Verified: full unit collection 10980 (== 10395 base + 585 relocated), no abort, all 3
CI seeds; the three files stay green (19/41/7, counts unchanged); lint_sys_modules exit 0.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(ci): add root-test-placement guard + durable placement rule (#1895)

Prevents the #1895 trap from recurring. New stdlib-AST guard
tests/lint_root_test_placement.py (never imports/execs a test module), wired into the
existing unconditional lint-sys-modules job (push + PR, never path-filtered):

  Part 1 — root placement. Every root-level tests/test_*.py must EITHER request a live
    fixture (api_client / created_agent / ws_ticket / …) it does not locally redefine,
    OR carry `# allow-root-live-test: <reason>`, OR be grandfathered in
    lint_root_test_placement_baseline.txt (ratcheted — never grows). A new
    self-contained root test fails → move it to tests/unit/.
  Part 2 — async markers. Every `async def test_` under tests/unit/ must have a
    resolvable pytest.mark.asyncio (own decorator, class decorator, class pytestmark,
    or module pytestmark). tests/unit/ runs pytest-asyncio strict, so an unmarked async
    test silently does not run; CI's pytest step is `|| true` and the diff gates on
    failing IDs, so this guard is how the strict-mode footgun becomes a hard failure.

Initial baseline = the 11 live-in-root files at adoption (raw-httpx/ws live tests,
trinity_cli-dependent CLI tests, and the currently red-and-hidden files that stay put —
see the batch commit messages). Verified 0 pre-existing unmarked async under tests/unit/
(the detector handles the class-decorator form many unit files use, e.g. test_backlog).

Durable rule (AC #6): tests/README.md gains a "Where does a new test go?" section;
tests/unit/conftest.py gains a placement-rule header; tests/conftest.py gains a stay-put
pointer near collect_ignore (the live-in-root set is enforced by the guard + baseline,
not a hand-maintained list) and its now-stale routers-preload docstring is refreshed (the
plain-stub installer left the root in #1895).

No architecture.md / requirements.md section governs test-file layout — nothing to update
there (Trinity Rule #1 satisfied). #1895 delivers collection + per-PR visibility; making
the job a *required* gate is #1958 (separate PR, deliberately deferred — a still-
stabilizing suite must not be made required, the #1228 class).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(ci): unit test for the root-test-placement guard (#1895)

tests/unit/test_lint_root_test_placement.py drives the guard's pure AST helpers with
synthetic source (no filesystem/backend), mirroring tests/unit/test_lint_sys_modules.py:
  Part 1 — self-contained root file is flagged; a live-fixture param (in a test OR a
    fixture) passes; a locally-redefined fixture is not a live signal; the
    allow-root-live-test comment exempts; a baselined path passes.
  Part 2 — unmarked async is flagged; function decorator, class decorator, class
    pytestmark (incl. the [asyncio, skip] list form), and module pytestmark all pass;
    a sync test is never flagged.
Plus a live-tree assertion that both guard parts are green on the committed tree.
13 tests, green under CI seeds 12345/67890/99999.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(ci): patch service-module deps in place, not by del+reimport (#1895)

Three relocated fixtures injected a stub by evicting a service module from
sys.modules and re-importing it under the stub (importlib.reload of
services.settings_service in test_platform_default_model; del + reimport of
services.proactive_message_service in test_proactive_audit_unit and
test_access_grant_notification_unit). In the shared unit collection island this
leaves a fresh module object in sys.modules; every later test that captured the
ORIGINAL (an instance whose class is the old module's, or a
`from services.X import fn` consumer) then diverges from the sys.modules entry a
sibling test patches — the sibling's monkeypatch.setattr lands on the new module,
the victim runs against the old (zombie) one, and the patch silently no-ops.
Confirmed by object-id instrumentation to break test_117 (no_voice_id), test_1609
(rate limit) and test_1081 (506 clamp) under pytest-randomly.

The code under test reads its deps as module globals, so patch them IN PLACE
(monkeypatch.setattr on the already-imported module) instead of reimporting —
keeping module identity stable (island-safe) and dropping the sys.modules writes
to zero. Snapshot/restore around a reimport is NOT enough: instances created in
the fixture window keep referencing the fresh module.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(ci): self-restore import-time utils stubs in two relocated files (#1895)

test_self_execute stubs `utils`/`utils.helpers` and test_watchdog_unit stubs
`utils.helpers`/`utils.credential_sanitizer`/`database` at import to load models /
cleanup_service without the tests/utils shadow. On dev these lived in tests/ root
(uncollected), so the stubs never touched the collection island; relocating them
into tests/unit/ let the stubs leak into a LATER file's collection-time import.
Confine each with the lint-recognised _STUBBED_MODULE_NAMES + _restore_sys_modules
pair so the stub is reverted the moment this module's own collection-time imports
have bound what they need.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(ci): restore stateless shared modules at collection in the unit island (#1895)

Relocating files reshuffles the whole pytest-randomly island, so a file that
stubs a shared module at COLLECTION (test_voice_tools installs a `config` stub
with a fixed SECRET_KEY) can leak into a LATER file's collection-time capture.
test_voice_auth binds `from config import SECRET_KEY` at import and signs JWTs
with it while routers.voice re-reads config.SECRET_KEY at request time — a
collection-time config divergence makes every signed token decode as Invalid
token (WS close 4001) order-dependently.

Add a pytest_collectstart hook restoring ONLY the STATELESS shared modules
(config, models, database, utils*) before each Module import. It deliberately
EXCLUDES services.settings_service and other singleton-holding modules — those are
handled in place at the source (see the reimport fix), because restoring a fresh
singleton to a stale baseline is the divergence documented for agent_server.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
vybe pushed a commit that referenced this pull request Aug 16, 2026
…) (#2231)

* refactor(auth): retire slack.py inv8 carve-out — migrate 11 gates to #1310 helpers (#1710)

Migrate all 11 inline auth gates in routers/slack.py onto the existing #1310
imperative guard helpers and delete all 11 `# noqa: inv8` markers, closing the
expiry-less waiver #1310 left behind.

- 3 read gates → assert_agent_access; 8 owner gates → assert_agent_owner.
- Every detail= string preserved verbatim (byte-identical 403 body) — this is a
  wiring consolidation, not a policy change (AC #6). All sites are access-first,
  so they stay 403/self-uniform per Invariant #8 (imperative helpers, not the
  path-dependencies that would flip denial to 404).
- Site #10 (set_slack_channel_proactive) keeps its ent#223
  reject_agent_principal(current_user) human-only guard; site #11 stays
  agent-callable (deliberate grant-vs-send asymmetry).

The helpers add _enforce_connector_scope first, a no-op for the REST flow
(connector keys are already fenced at the auth entry point).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(architecture): drop the retired slack.py inv8 deferral clause (#1710)

Invariant #8's permanent-exceptions list no longer names slack.py as a
`# noqa: inv8`-marked deferred follow-up — the carve-out is retired. No new
prose: the invariant already carries "Access-first inline handlers ... stay
403", which documents why the migrated slack sites keep imperative-403.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(auth): stop calling the noqa hatch "the deferred slack.py sites" (#1710)

The slack.py sites are migrated onto the shared helpers, so no `# noqa: inv8`
marker remains in the tree. Update both docstrings (module + the live-tree
guard) to describe the marker as a general, individually-reviewed escape hatch
rather than a slack-specific deferral. The mechanism (`_line_has_noqa` +
`test_noqa_inv8_suppresses_line`) stays dormant for a future exception.

Guard verified green: 12 passed, zero slack offenders.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(auth): behavioral proof for the 11 migrated slack gates (#1710)

Add section A9 to the #1310 consolidation suite — the behavioral half the AST
wiring guard is blind to. Over all 11 migrated sites, driving the real
db.can_user_* predicates against a temp schema (db_harness):

- stranger → 403 + the exact per-site detail (catches a forgotten `detail=`);
- a shared (non-owner) reader is ADMITTED on the 3 read gates and DENIED on all
  8 owner gates (catches a mis-classified assert_agent_access/owner);
- owner and admin are never denied by the migrated gate;
- site #10 keeps its ent#223 human-only reject (agent principal → 403 with the
  human-only detail) while site #11 stays agent-callable (grant/send asymmetry).

Verified adversarial: read→owner swap, detail drift, and dropping the site-#10
reject each break the matching test. 46 passed, order-independent across seeds.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(auth): runtime human-only proof for the site-#10 consent toggle (#1710)

The existing ent#223 guard is source-grep only (`"reject_agent_principal" in
src`) — it catches outright deletion but not a behavioral/ordering regression,
and after #1710 the surrounding gate is a plain assert_agent_owner call the AST
guard can't see. Add a runtime case: an agent-scoped principal calling
set_slack_channel_proactive gets 403 + the human-only detail, with every
post-gate db call (owner check + mutation) made loud to prove the reject fires
first. Keep the source-grep as a cheap backstop.

Verified adversarial: dropping the reject line fails both. 10 passed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(feature-flows): re-sync slack Authorization Checks table with #1710

The migration moved slack.py's auth gates off inline `db.can_user_*` onto the
shared #1310 helpers, so a reader following the doc to slack.py no longer finds
the predicate inline. Update the Authorization Checks Method column to name the
helper (and the predicate it wraps), refresh the stale File:Line values to the
current gate lines, and add a Revision History row. Behavior-preserving — the
403 + detail is unchanged. Index untouched (existing flow, name/desc unchanged).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs: re-sync auth.md + feature-flows index with the retired slack inv8 carve-out (#1710)

The #1310 follow-up updated architecture.md and slack-integration.md but two
live docs still described the carve-out as active:

- requirements/auth.md still said 'slack.py deferred (10 sites) ... each carries
  a # noqa: inv8 marker' — false after the migration, and it reintroduces the
  exact 'considered exception vs abandoned TODO' ambiguity #1710 exists to kill.
- feature-flows.md index had no #1710 row (the flow doc's own changelog was
  updated, the index was not).

Docs-only; no code change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(auth): fix test_1649 slack router fixtures for the migrated owner gate (#1710)

The 3 TestRoutersPersist cases call send_agent_slack_channel_message, now
gated by assert_agent_owner → _enforce_connector_scope, which reads
current_user.connector_agent. The SimpleNamespace fakes lacked that field,
raising AttributeError before can_user_share_agent. Add connector_agent=None
(a real non-connector User carries it); owner admission stays byte-preserving.
Proven load-bearing: the 3 cases fail on the pre-fix fixtures, pass with it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
dolho added a commit that referenced this pull request Aug 25, 2026
One click on the thing being judged: thumbs on an agent message, Useful / Not
what I needed on a deliverable card — the affordance ent#365 left that card as
the surface for — and a comment box on the negative.

A rating is a PLATFORM PRIMITIVE, not a skill, and the reason is the eval
epic's: a capture-feedback skill runs inside the agent, so it can summarise
charitably, omit, or fail silently, and a user rating is the one score that must
not pass through the thing being scored. So this AMENDS ent#206's write fence to
admit a `workspace:<email>` evaluator rather than working around it. The rated
agent still has no write path.

Targets are checked against the READER, because message and report ids are both
global: a message must belong to this agent AND this client and be the agent's
message (rating your own is refused — it would put a self-rating in the agent's
tally), and a deliverable reuses ent#365's audience gate so "can rate" and "was
addressed to you" are one question answered in one place. A target that fails
either check returns the same 404 a missing one does, so this is not an
existence oracle.

`UNIQUE(evaluator, target_kind, target_id) WHERE target_id IS NOT NULL` makes a
second thumb a CORRECTION rather than a second vote, which is what makes the
agent-page tally count people rather than clicks. That tally is raw counts and
never a percentage: one thumbs-down out of one rating renders as "100%
negative", a number that looks like evidence and is not.

The issue's open grooming question is decided and implemented: the rated agent
reads its own tallies and never the words. A score an agent can read is a loop
it may start optimising for, and the comment is untrusted text written by an
annoyed stranger — handing it verbatim to the agent being criticised is a
prompt-injection path into it. `comment_withheld` lets a reader tell "no
comment" from "not yours to read"; operator surfaces are unchanged.

The words reach the agent only through `capture-feedback`, fenced as data with
the framing routers/webhooks.py already uses, dispatched as its own background
execution — never as a message in the client's thread, which would be a second
unasked-for reply in the conversation someone just complained about. Absent the
skill the rating and comment are still durable and the UI says "recorded"
instead of promising a follow-up (AC #6).

Verified live end to end: rate, change your mind (one row, updated), a target
that isn't yours 404s, a bad kind 422s, the tally reads 0/1, an operator sees
the comment and the RATED AGENT reads `quality: 0.0` with `comment: None,
comment_withheld: true`. In the browser: 15 thumb pairs, the existing rating
restored from history, the box opening on the negative, and the honest
acknowledgement.

One thing this deliberately does NOT do: stamp the feedback turn with
`source_channel=portal`. `test_2157_portal_narration` pins that stamp to exactly
the two portal turn-creation sites, and it is right — the stamp means "an
exchange on the Workspace surface", and this turn has no surface: nobody sees
its output and it must not be answering anyone.

Stacked on feature/ent365-workspace-deliverables: the deliverable card this
attaches to exists only there.

Related to Abilityai/trinity-enterprise#366

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dolho added a commit that referenced this pull request Aug 25, 2026
One click on the thing being judged: thumbs on an agent message, Useful / Not
what I needed on a deliverable card — the affordance ent#365 left that card as
the surface for — and a comment box on the negative.

A rating is a PLATFORM PRIMITIVE, not a skill, and the reason is the eval
epic's: a capture-feedback skill runs inside the agent, so it can summarise
charitably, omit, or fail silently, and a user rating is the one score that must
not pass through the thing being scored. So this AMENDS ent#206's write fence to
admit a `workspace:<email>` evaluator rather than working around it. The rated
agent still has no write path.

Targets are checked against the READER, because message and report ids are both
global: a message must belong to this agent AND this client and be the agent's
message (rating your own is refused — it would put a self-rating in the agent's
tally), and a deliverable reuses ent#365's audience gate so "can rate" and "was
addressed to you" are one question answered in one place. A target that fails
either check returns the same 404 a missing one does, so this is not an
existence oracle.

`UNIQUE(evaluator, target_kind, target_id) WHERE target_id IS NOT NULL` makes a
second thumb a CORRECTION rather than a second vote, which is what makes the
agent-page tally count people rather than clicks. That tally is raw counts and
never a percentage: one thumbs-down out of one rating renders as "100%
negative", a number that looks like evidence and is not.

The issue's open grooming question is decided and implemented: the rated agent
reads its own tallies and never the words. A score an agent can read is a loop
it may start optimising for, and the comment is untrusted text written by an
annoyed stranger — handing it verbatim to the agent being criticised is a
prompt-injection path into it. `comment_withheld` lets a reader tell "no
comment" from "not yours to read"; operator surfaces are unchanged.

The words reach the agent only through `capture-feedback`, fenced as data with
the framing routers/webhooks.py already uses, dispatched as its own background
execution — never as a message in the client's thread, which would be a second
unasked-for reply in the conversation someone just complained about. Absent the
skill the rating and comment are still durable and the UI says "recorded"
instead of promising a follow-up (AC #6).

Verified live end to end: rate, change your mind (one row, updated), a target
that isn't yours 404s, a bad kind 422s, the tally reads 0/1, an operator sees
the comment and the RATED AGENT reads `quality: 0.0` with `comment: None,
comment_withheld: true`. In the browser: 15 thumb pairs, the existing rating
restored from history, the box opening on the negative, and the honest
acknowledgement.

One thing this deliberately does NOT do: stamp the feedback turn with
`source_channel=portal`. `test_2157_portal_narration` pins that stamp to exactly
the two portal turn-creation sites, and it is right — the stamp means "an
exchange on the Workspace surface", and this turn has no surface: nobody sees
its output and it must not be answering anyone.

Stacked on feature/ent365-workspace-deliverables: the deliverable card this
attaches to exists only there.

Related to Abilityai/trinity-enterprise#366

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dolho added a commit that referenced this pull request Aug 26, 2026
One click on the thing being judged: thumbs on an agent message, Useful / Not
what I needed on a deliverable card — the affordance ent#365 left that card as
the surface for — and a comment box on the negative.

A rating is a PLATFORM PRIMITIVE, not a skill, and the reason is the eval
epic's: a capture-feedback skill runs inside the agent, so it can summarise
charitably, omit, or fail silently, and a user rating is the one score that must
not pass through the thing being scored. So this AMENDS ent#206's write fence to
admit a `workspace:<email>` evaluator rather than working around it. The rated
agent still has no write path.

Targets are checked against the READER, because message and report ids are both
global: a message must belong to this agent AND this client and be the agent's
message (rating your own is refused — it would put a self-rating in the agent's
tally), and a deliverable reuses ent#365's audience gate so "can rate" and "was
addressed to you" are one question answered in one place. A target that fails
either check returns the same 404 a missing one does, so this is not an
existence oracle.

`UNIQUE(evaluator, target_kind, target_id) WHERE target_id IS NOT NULL` makes a
second thumb a CORRECTION rather than a second vote, which is what makes the
agent-page tally count people rather than clicks. That tally is raw counts and
never a percentage: one thumbs-down out of one rating renders as "100%
negative", a number that looks like evidence and is not.

The issue's open grooming question is decided and implemented: the rated agent
reads its own tallies and never the words. A score an agent can read is a loop
it may start optimising for, and the comment is untrusted text written by an
annoyed stranger — handing it verbatim to the agent being criticised is a
prompt-injection path into it. `comment_withheld` lets a reader tell "no
comment" from "not yours to read"; operator surfaces are unchanged.

The words reach the agent only through `capture-feedback`, fenced as data with
the framing routers/webhooks.py already uses, dispatched as its own background
execution — never as a message in the client's thread, which would be a second
unasked-for reply in the conversation someone just complained about. Absent the
skill the rating and comment are still durable and the UI says "recorded"
instead of promising a follow-up (AC #6).

Verified live end to end: rate, change your mind (one row, updated), a target
that isn't yours 404s, a bad kind 422s, the tally reads 0/1, an operator sees
the comment and the RATED AGENT reads `quality: 0.0` with `comment: None,
comment_withheld: true`. In the browser: 15 thumb pairs, the existing rating
restored from history, the box opening on the negative, and the honest
acknowledgement.

One thing this deliberately does NOT do: stamp the feedback turn with
`source_channel=portal`. `test_2157_portal_narration` pins that stamp to exactly
the two portal turn-creation sites, and it is right — the stamp means "an
exchange on the Workspace surface", and this turn has no surface: nobody sees
its output and it must not be answering anyone.

Stacked on feature/ent365-workspace-deliverables: the deliverable card this
attaches to exists only there.

Related to Abilityai/trinity-enterprise#366

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dolho added a commit that referenced this pull request Aug 27, 2026
…egistry link could never match (#2338)

## 1. `_STATUS_KEY_RE` matched ordinary words

One unanchored alternation, `.search()`ed against every key at every depth —
and `red` is a substring of **`credential`** and of **`required`**. J06 is
literally *"I can give an agent a credential and it can use the tools that
credential unlocks"*, so a `credentials:` key was coming, and the moment it
arrived this rule would have failed accusing the author of committing run
status. `green` had the same shape (`evergreen`, `greenfield`), `result` matched
`results`/`resultant`. Measured on the shipped regex: `credential`,
`credentials`, `required`, `evergreen`, `greenfield`, `resultant` and even
`hundred` all matched. A rule whose entire value is that it fails CLEARLY cannot
afford a false positive landing on someone who did nothing wrong.

Word boundaries alone were not the fix: `_` is a word character, so `\bstatus\b`
does not match `journey_status` — that spelling would have gone quiet on exactly
the keys the rule exists to catch. The key is now split into tokens and matched
against a fixed vocabulary, plus an exact-match set for multi-token names whose
parts are individually innocent (`last_run`, `pass_rate`). Both directions are
now pinned by parametrized cases.

## 2. The registry join could not match (AC #6 was inert)

`tests/registry.json` stores `test_files` as a list of **dicts**, so `str(f)`
was a dict repr and `Path(str(f)).name` was nonsense. Compounding it, registry
paths are tests-relative (`unit/test_x.py`) while catalog harnesses are
repo-relative (`tests/journeys/test_x.py`), so even the substring arm could not
hit. The generated doc would have carried "Harnesses not present in
tests/registry.json" for every journey, permanently and wrongly — invisible
today only because all ten harnesses are still `null`.

Now reads `test_files[].file` and compares through one shared
`_norm_test_path`.

## 3 + 4. `green_harnesses` had the same prefix bug, and called all-skipped green

JUnit `file` attributes are rootdir-relative, so they read `journeys/test_x.py`
against a catalog holding `tests/journeys/test_x.py` — every journey would have
reported `no evidence` forever. Same normalisation fixes it.

And `skipped` is neither `failure` nor `error`, so a file whose every case
skipped was recorded green. J03's harness is precisely the credential-bound one
that skips on every PR-triggered run, so the first journey to carry a path would
also have been the first misreported — while the docstring above it said
"absence of evidence is not green". It was right about the missing-artifact case
and silent about this one; it now states both, and green requires at least one
real pass.

## Why these were invisible

The review named it exactly: the generator was verified against a real
downloaded artifact set, and that set contains no journey harness at all — so
the run exercised the PARSE path and never the MATCH path. The verification was
real, it just could not reach the code the findings were in. Three tests now
drive the match path directly, including the registry join against a genuine
`registry.json` entry. Each was mutation-checked: reverting `f.get("file")` and
reverting the all-skipped rule each turn the corresponding test red.

The committed `docs/testing/JOURNEYS.md` is byte-identical after these changes
(every harness is still `null`), and `--check` passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vybe pushed a commit that referenced this pull request Sep 2, 2026
…tecture.md counts (#2238)

A full /validate-architecture run on 9b0ed63 found Invariant #13 unenforceable — 44 of the
non-excluded routers had neither a same-named MCP tool module nor a `# mcp: none` marker —
and seven architecture.md counts more than 25% stale.

- Every router outside the by-design exclusions (internal/setup/auth/public/paid) now opens
  with a `# mcp:` header: 30 x `none — <reason>` (admin, grant-vs-use human-only, UI-only)
  and 14 x a pointer to the covering tool module (`agents.ts (rename_agent)`, ...), so the
  validator can tell "unexposed on purpose" from "forgotten". Comments only; each module
  docstring stays the first statement (AST-checked).
- architecture.md: router / service / tool-module / tool counts, agents.py size, AGENT_REFS
  cascade width, compatibility check count, four endpoint-group counts; the MCP tools table
  gains its three missing modules (a2a, connector, rooms) and the chat/skills rows catch up;
  the dead "DEPRECATED Redis credential keys" bullet (zero readers or writers) is removed;
  Invariant #13 records the header convention.
- CLAUDE.md: MCP tool count and endpoint/router count.

Related to #2238 — the remaining #6/#7/#15/#18 gaps stay tracked there.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AQHbGS2GV78AmBFnZX78j6
vybe pushed a commit that referenced this pull request Sep 2, 2026
…tecture.md counts (#2238) (#2482)

A full /validate-architecture run on 9b0ed63 found Invariant #13 unenforceable — 44 of the
non-excluded routers had neither a same-named MCP tool module nor a `# mcp: none` marker —
and seven architecture.md counts more than 25% stale.

- Every router outside the by-design exclusions (internal/setup/auth/public/paid) now opens
  with a `# mcp:` header: 30 x `none — <reason>` (admin, grant-vs-use human-only, UI-only)
  and 14 x a pointer to the covering tool module (`agents.ts (rename_agent)`, ...), so the
  validator can tell "unexposed on purpose" from "forgotten". Comments only; each module
  docstring stays the first statement (AST-checked).
- architecture.md: router / service / tool-module / tool counts, agents.py size, AGENT_REFS
  cascade width, compatibility check count, four endpoint-group counts; the MCP tools table
  gains its three missing modules (a2a, connector, rooms) and the chat/skills rows catch up;
  the dead "DEPRECATED Redis credential keys" bullet (zero readers or writers) is removed;
  Invariant #13 records the header convention.
- CLAUDE.md: MCP tool count and endpoint/router count.

Related to #2238 — the remaining #6/#7/#15/#18 gaps stay tracked there.


Claude-Session: https://claude.ai/code/session_01AQHbGS2GV78AmBFnZX78j6

Co-authored-by: trinity-ability <noreply@anthropic.com>
vybe pushed a commit that referenced this pull request Sep 4, 2026
…belled agent stops hiding its slug (#2519)

* docs(#2358): one column sizing context + label-leads-slug in the List and Grid flows

Requirements-first per CLAUDE.md Rule 1: FR-3/FR-4 in §1.3.1 already mandate
the behaviour this bug fix restores, so no new FR is added — only the §9.9
Key Features line is corrected (the badge set moves, and the lg header/rows
become one grid).

- feature-flows/dashboard-list-view.md: lg row anatomy in the Components
  block; D12 (one sizing context via subgrid — insets as item margins, not
  padding on a subgrid item; definite placement in BOTH axes for the meter and
  the secondary line, since sparse auto-placement opens an implicit third
  row), D13 (label leads, slug follows on the line that already exists — plus
  the intended md density change now the md tags row is always rendered), D14
  (badge policy + the fixed secondary-line order that keeps it from becoming a
  junk drawer). Testing section rewritten: the "No frontend unit-test infra
  exists" line was stale — vitest runs 73 spec files today.
- feature-flows/dashboard-grid-view.md: an Identity zone section — why the
  slug rides .t-repo rather than a third line (fixed 384x216 cell,
  space-between rhythm), and why .t-slug needs its own ellipsis, select-all
  and nodrag.
- feature-flows.md: dated index row.
- architecture.md: one sentence on each of the List and Grid blocks.

Refs #2358

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

* feat(#2358): agentNameParts + the non-default-runtime rule, with the module's first specs

Two pure helpers, written test-first, so the two decidable rules behind the
List/Grid fix live where a spec can reach them — vitest pins
`environment: 'node'` with no mount harness, so a rule inline in an SFC is a
rule no test can see.

- `agentName.js::agentNameParts(agent)` → `{primary, secondary}`, composed from
  the existing `agentDisplayName` / `hasDistinctLabel` rather than re-deriving
  the rule. `secondary` is either null or EXACTLY `agent.name` — never the
  label (it is the string a reader pastes into a URL or an MCP key lookup, so a
  presentation name appearing there would be worse than showing nothing), and
  null when the primary already is the slug, so a surface reserves no phantom
  second line. This is what lets call sites stop writing `display_label ||`.
- `agentRuntime.js` — `isDefaultRuntime` / `showsRuntimeBadgeInList`. The id
  set mirrors `RuntimeBadge.vue`'s own `isClaude` (`claude-code` AND `claude`):
  narrower, and a `runtime: "claude"` row would wear a Claude pill on a
  homogeneous Claude fleet, which is the noise the rule removes. Anchored to
  the platform default, not fleet majority — a majority rule flips badges as
  agents come and go.
- `agentNameTooltip`'s docstring no longer claims to be the slug's only home on
  list rows and tiles: a `title` is invisible on touch, unreachable by keyboard
  and impossible to copy from. It stays as a belt for a truncated label.

`tests/unit/agentName.spec.js` is the FIRST test over that module — it had
none, on all four exports, despite resolving every agent name the product
renders.

Refs #2358

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

* fix(#2358): the List header and every row become ONE column sizing context

The columns were ragged because the header and each row were INDEPENDENT CSS
grids that happened to share a copy of the same template string. Each resolved
its own `auto` tracks against its own content and the `1fr` name track absorbed
the difference, so a row reading `14/16` put Controls / Success / Exec-Sched at
a different x than a row reading `0`, and neither lined up with the header. Two
more drifts rode along: the header spacers were not the widths of the cells
they sat above (`w-3`/`w-6` vs `w-2.5`/`w-4`), and the row grid was 10px
narrower than the header because the CapacityMeter sat OUTSIDE it as a flex
sibling — different right edges, therefore a different `1fr`, therefore
misalignment even with identical content.

The template is now declared once on the list container; the header and every
row are `lg:grid-cols-subgrid lg:col-span-full` items of it, and the meter
joins as track 10 so the two share a right edge. Fixing the spacer widths alone
could not have worked: they are `auto` tracks, so with two grids the content
decides.

Three properties keep it true, each pinned by a source-level guard in
`agentName.spec.js` (vitest is node-env, so this is not reachable as
behaviour, and the Playwright specs that would reach it are `ui`-gated):

- **No horizontal padding on any subgrid item.** A subgrid item's own padding
  is laid out INSIDE its first and last tracks (CSS Grid L2 §7.1), so insets
  are ordinary item margins instead — `lg:ml-8` on the checkbox, `lg:mr-4` on
  the meter, identical on the header and every row. Alignment then rests on
  plain L1 margin sizing, not on a spec corner.
- **Definite placement in BOTH axes** for the two items that are not on row 1.
  With sparse auto-placement the secondary line's definite column bumps the
  cursor to row 2 and the meter's 2-row span lands in rows 2–3 — an implicit
  third row hanging below every line. `lg:row-start-1` after `lg:row-span-2`
  is what yields `grid-row: 1 / span 2` (Tailwind emits gridRow before
  gridRowStart; verified against the installed 3.4.19).
- **One `contents` wrapper** for the eleven lg cells, so a single breakpoint
  switch stands in for eleven `hidden lg:…` classes and no extra box sits
  between the row and its tracks. The row div is still the visual box, so
  background, radius, hover, the system `border-l` and the half-out avatar's
  positioning parent are all unchanged.

Hooks for the alignment e2e: `data-testid="list-header"` and
`data-col="name|status|controls|success|stats"` on both the header cells and
the row cells.

Refs #2358

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

* fix(#2358): the label leads and the slug follows — at all three List breakpoints

A labelled row rendered the label ALONE with the slug on a `title`. That shows
one agent under two naming conventions in one list, with nothing connecting
"Delivery Operations Manager" to the `delivery-ops` that every URL, MCP key,
container and volume is keyed on — and a `title` is invisible on touch,
unreachable by keyboard and impossible to copy from (§1.3.1 FR-4 says the slug
stays visible AND copyable).

The slug now renders as real `<code>` text on the line each layout ALREADY
always renders — the lg secondary line, the md tags row, the base meta line —
so a labelled row is exactly as tall as an unlabelled one and nothing reserves
a phantom second line. It is `select-all`, because a double-click under plain
text selection takes one hyphenated segment rather than the slug, and it is
never inside the router-link: it is a copy target, not a nav target. All three
sites resolve through `agentNameParts()`; the markup no longer resolves a name
any other way (`agentDisplayName` stays imported for the name filter and the
toasts, which are prose).

Badge policy, so the name cell is worth reading:

- The name cell keeps EXCEPTION markers only — SYSTEM, GHOST, Shared.
- The #471 pressure badge (on essentially every row of a shared-subscription
  fleet) moves to the lg secondary line, first in it: a problem signal
  escalates to the front, as on the grid tile. Predicate untouched.
- The runtime badge renders only for a NON-DEFAULT runtime, and never at base.
  Relocating a pill that is identical on every row is not a reduction: on a
  homogeneous fleet it now disappears entirely, and on a mixed fleet it marks
  the exceptions.
- The secondary line has a fixed order (slug · pressure · runtime · tags · +N)
  and a shrink policy — the slug is the only shrinkable item, so the identity
  ellipsizes instead of the chips being crushed. That order is what keeps the
  line from becoming the drawer every future badge lands in.

md/base row-to-row parity — there is no header there, so parity between rows is
all a reader has: a toggle a row may not use is now RESERVED (`invisible` keeps
the box and takes no clicks) instead of dropped, which had been shifting the
success bar, meter and task counts on exactly the system and shared rows; the
md CapacityMeter is always rendered for the same reason; and the md tags row is
always rendered, which both hosts the slug and removes a latent jitter where a
tagged md row was ~30px taller than an untagged one. Tagless md rows therefore
grow by that line — an intended density trade, release-noted in the flow doc.
Reservation widths come from the toggles themselves, not hand-written rems, so
they cannot drift.

Raw-color ratchet (measured): AgentListPanel raw_nongray 28 → 28,
hardcoded_colors 3 → 3 (both flat — the hard bar), raw_gray 155 → 159, which is
the two meta-ink pairs the lg and md secondary-line containers need so the slug
`<code>` inherits ladder-correct ink rather than carrying its own.

Refs #2358

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

* fix(#2358): the Grid tile shows the slug the label hides

Same defect as the List row, same surface class (§1.3.1 FR-3 names dashboard
cards and grid tiles together): a labelled tile rendered the label alone with
the slug on a `title`, so switching Dashboard modes showed the same agent under
two naming conventions.

The slug leads `.t-repo` — the meta line the tile ALREADY always renders —
rather than taking a line of its own. `.gtile` is a fixed 384x216 cell laid out
with `space-between`, so a third identity line present only on labelled tiles
would compress their zone rhythm and squeeze the charts on exactly those tiles,
making the fleet look inconsistent tile to tile.

Three details are load-bearing rather than decorative:

- `.t-slug` carries its OWN `text-overflow: ellipsis` — `.t-repo span` targets
  `span`, so a `code` inherits none of it — plus `flex: none; max-width: 55%`,
  so a long slug ellipsizes instead of painting over the repo text.
- Its own `color: var(--gv-muted)` rather than inheriting: `.t-repo.local`
  ghosts the whole line for an agent with no repo, and the identity is not
  decoration. No new `--gv-*` var — this is the ink the repo text beside it
  already uses.
- `user-select: all` against `.gtile`'s `user-select: none`, and `nodrag`
  because `FleetGrid.onTilePointerDown` starts a drag unless the target is
  inside `.nodrag`. Without both the slug could be neither selected nor copied
  and a click on it would drag the tile. By design it does NOT navigate — it is
  a copy affordance; navigation stays on `.t-name` and the Details button.

Raw-color counts unchanged at 0 / 0 / 3.

Refs #2358

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

* test(#2358): e2e for column alignment, both names, and row-height parity

Four cases on the List and one on the Grid tile, plus one shared fixture
helper.

- **Column alignment (lg, 1280 then 1440):** for each `data-col`, every visible
  row cell's `boundingBox().x` equals the header cell's within 1px. On CI's
  one-agent fleet this proves HEADER parity; row-to-row alignment across
  varying `Exec / Sched` content is structural (one subgrid, one set of tracks)
  and is checked by eye on a real fleet at review. The resize re-run is a
  layout-stability check, not a varying-content one — the header comment says
  so, because a spec that overstates what it covers is how a gap survives.
- **The meter is in grid rows 1-2:** it starts at or above the name line and
  ends at or above the secondary line's bottom. Without definite placement in
  both axes, sparse auto-placement puts it in rows 2-3, hanging an implicit
  third row under every row in the list — a defect the unit guard catches in
  source and this catches in pixels.
- **Both names, and the slug is real:** the visible name link reads the label,
  `[data-testid="agent-slug-lg"]` reads the slug, its computed `user-select` is
  not `none`, and it is not inside an `<a>` (FR-4 wants copyable, and a `title`
  is none of those things). The system row still says SYSTEM.
- **md/base parity:** the slug renders at both, and a row is the same height
  labelled and unlabelled — measured by labelling the fixture, clearing it, and
  comparing, rather than asserting a constant. Any runtime badge still visible
  must sit below the name line.
- **Grid tile:** the slug appears on `.t-repo` for a labelled tile and not at
  all for an unlabelled one, with `user-select: all` and inside `.nodrag`.
  Appended as its own describe block at the END of the file, imports included,
  so the change is purely additive and rebases cleanly.

`e2e/helpers/agent-label.js` is shared rather than copied into both specs, for
the reason `agent-probe.js` states about itself: a fixture pattern re-derived
per spec drifts, and the half that drifts here leaves a live agent wearing a
test label. It reads the PRIOR label and restores exactly that (including
null), restores in `afterEach` because a `finally` in a timed-out body is not
reliably run, and throws loudly on any non-2xx rather than letting a caller
assert against an unchanged surface. Both blocks are `serial` —
`fullyParallel: true` means a sibling worker would otherwise see the borrowed
label. `AgentHeader.vue` hides the label pencil for system agents, so a
stranded label on `trinity-system` has no UI undo; that hazard is now in
`e2e/README.md` beside the probe contract.

Refs #2358

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

* docs(#2358): sync the two adjacent flows the toggle reservation made stale

`/sync-feature-flows` over `origin/dev..HEAD`. Mapping the four changed source
files against every flow that names them turned up two whose claims no longer
matched the code:

- `autonomy-toggle-component.md` documented `invisible` as the
  "column-preserving" pattern for the List panel — a property only the lg
  layout actually honoured. md and base used `v-if`, so the toggle was removed
  rather than reserved.
- `dashboard-list-view.md`'s system-row Run guard said the toggle is "hidden"
  without saying which kind of hidden, which is the whole difference between a
  column that holds and one that shifts.

Checked and left alone: `subscription-usage-tracking.md` (says the badge feeds
the List row, never where in the row it sits — still true after the demotion),
`parallel-capacity.md` and `agent-tags.md` (name the panel as a consumer, do
not describe the row's rendering conditions), `agents-page-ui-improvements.md`
(explicitly Superseded — a historical record, not a live claim).

Refs #2358

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

* docs(#2358): correct a locator note that named the old resolver

`schedules-toggle-scroll.spec.js` explains why it addresses the row by
`data-agent` + `href` rather than link text, and cited
`AgentListPanel` rendering `agentDisplayName(agent)`. That call is gone from
the panel's markup — the structural guard in `agentName.spec.js` now asserts it
is absent — so the note pointed at a function a reader would not find. The
house rule it states is unchanged and still correct: the rendered name is the
owner-settable label, so a text locator would break the moment anyone labels
the agent.

Refs #2358

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

* style(#2358): re-indent the two lg items the wrapper removal left behind

Whitespace only (`git diff -w` is empty). Collapsing the three lg wrapper divs
into one `lg:contents` wrapper left the secondary line and the CapacityMeter
two spaces shallower than the nine cells they sit beside, which made the
wrapper's child list unreadable — and that list is the thing a reviewer has to
count to see that eleven items land in ten tracks the way the placement
classes say they do.

Refs #2358

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

* fix(#2358): the base Run toggle is dropped again, not reserved

The reservation was imported from the md rule ("Reserved, not dropped — see
the md note") but the geometry it was written for does not exist at base. The
trailing group there is `flex-shrink-0` behind a `flex-1` name, so it is
already flush to the row's right edge on every row: measured at 390px, the
chevron sits at the same x whether the Run toggle is present, absent or
reserved. The reservation therefore aligned nothing — and cost the one row
that has no toggle the toggle's ~94px (its `min-w-[3.25rem]` label plus the
`w-9` switch plus the gap), taking the system row's name from 221px to 119px
and turning a labelled `trinity-system` from fitting to truncated at the
narrowest supported width. That is precisely what AC #6 forbids: "whatever
remains does not push the name into truncation at common viewport widths".

md keeps its reservation, for the reason base never had: there the three
toggles sit BEFORE the `flex-1` success bar, so a dropped toggle really does
pull a column — the bar starts ~298px further left on system and shared rows.
Both numbers are measured, not reasoned: the flex structures were reproduced
in a browser and the boxes read off.

The two flow docs said "hidden means RESERVED at every breakpoint" and named
the capacity meter and task counts among what moved at md. Neither was true —
the meter and stats sit after the `flex-1` and never moved, and `base` has no
AutonomyToggle at all. Both corrected with the measured numbers.

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

* test(#2358): guard the row's item count and the runtime mirror

Two contracts the change relies on were written down and left unguarded, and
both fail silently. Each new guard was proven by mutating the source and
watching it red.

The lg row is arithmetic: ten tracks, nine children that auto-place across row
1, two placed by hand. None of those numbers appears in a class string, so the
file's existing source-regex guards cannot see them — a twelfth child added to
the `lg:contents` wrapper leaves all 32 of them green while the new cell
auto-places into row 2 column 1, in the row's left gutter, widening track 1
for the header and every row with it. The shape is now read from the template
AST via `vue/compiler-sfc` (the parser Vite already runs on this file, reached
through the declared `vue` dependency), which also makes the guard immune to
re-indentation — the previous commit on this branch was a re-indent.

Three properties, one per failure: the header has one cell per declared track
(an eleventh silently opens a second header row); the wrapper holds eleven
items of which exactly two are placed by hand; and no auto-placed cell carries
`v-if`/`v-for`/`v-else` — auto-placement is positional, so a conditional cell
5 shifts cells 6-9 one track left on that row only, which is the per-row
misalignment this issue exists to remove. Reserve with `invisible`, as the
system row's Run toggle does, never with `v-if`.

`DEFAULT_RUNTIME_IDS` mirrored `RuntimeBadge.vue`'s `isClaudeRuntime` by
hand-written assertion, which cannot notice the day the original moves.
`RuntimeBadge.vue` is shared and deliberately not edited here, and the
predicate is a `computed` with nothing to import, so the set is now DERIVED
from its source. Divergence is silent both ways and neither way is cosmetic: a
new Claude id the set lacks puts a sunburst pill back on every row of an
all-Claude fleet, and an id the badge drops takes the badge off the one
runtime it exists to mark.

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

* test(#2358): the label borrow cannot strand a live agent

Two spec files now borrow whatever `pickLabelFixture` returns, and
`mode: 'serial'` orders tests only WITHIN a file. `playwright.config.js` pins
`workers: 1` on CI but leaves it at the default locally — which is exactly
where both files are run together, since the verify step names them on one
command line. Two workers, one agent: the second worker's "prior label" read
can land after the first worker's borrow, and it then faithfully restores the
FIXTURE label as though it were the operator's own. That is a permanent
mislabel on a live agent, produced by a run that reports green, and on
`trinity-system` the operator cannot even reach for the pencil to undo it.

`FIXTURE_LABEL` moves into the shared helper as the one label these helpers
ever write, and `readLabel` REFUSES to return it. The late worker throws before
it writes anything, its `afterEach` has nothing to restore, and the worker that
actually holds the borrow still puts the agent back. The same refusal catches a
run killed mid-borrow: the next run finds the sentinel and says so — with the
exact PUT to clear it — instead of adopting a test artefact as the value to
restore forever. `--workers=1` for a clean single pass is documented in both
spec headers and the e2e README.

The string is self-identifying ("Trinity e2e fixture - safe to clear") rather
than the plausible-looking "Platform Orchestrator" for the same reason: if it
is ever seen on a live agent, whoever finds it must be able to tell at a glance
that it is safe to clear.

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

* chore(#2358): drop a dead import and a class the guard forbids

`AgentTile.vue` kept importing `agentDisplayName` after its only use moved to
`agentNameParts`. There is no linter in this package, so an unused import is
not caught anywhere and reads to the next person as a second name-resolution
path in a file whose whole point is that there is one.

`dashboard-list-view.md` drew the header and row as carrying `lg:px-0`. They
carry no horizontal padding class at all, and the structural guard rejects the
whole `lg:p[xlr]-` family — so a reader who made the markup match the diagram
would red the test that exists to keep the insets as margins. Corrected, and
the reason stated beside it.

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

* style(#2358): the tile slug uses the canvas's own mono stack

`.t-slug` declared a fifth spelling of a font stack that appears four times in
`FleetGrid.vue`, inside whose cascade the tile renders. The two differ where it
shows: the canvas stack names `Consolas`, this one did not, so on Windows the
slug would fall through to the browser's generic `monospace` while the mono
text beside it on the same tile did not. Cosmetic, but it is the same defect
class as hand-rolling a primitive — one surface, two recipes.

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

---------

Co-authored-by: trinity-ability <309458136+trinity-ability@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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