Skip to content

Feature/process engine - #10

Merged
vybe merged 69 commits into
mainfrom
feature/process-engine
Jan 20, 2026
Merged

Feature/process engine#10
vybe merged 69 commits into
mainfrom
feature/process-engine

Conversation

@oleksandr-korin

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

Copy link
Copy Markdown
Contributor

Note

Introduces comprehensive process documentation and onboarding content, updates process templates/schema, extends the system agent with a Process Creation Assistant, and tweaks docker config for mounting docs/templates and configurable frontend port.

  • Adds config/process-docs (getting started, patterns, reference, tutorials) with index.json and editor-help.json
  • Extends trinity-system/CLAUDE.md with Process Creation Assistant, YAML schema/patterns, and MCP usage
  • Updates process templates: normalize version: "1.0", replace legacy condition/user_task with gateway/human_approval, and switch outputs valuesource
  • docker-compose: mount config/process-templates and config/process-docs; make frontend port configurable via FRONTEND_PORT
  • Adds platform docs: known issues and multiple process-engine backlogs (access/audit, reliability, onboarding)

Written by Cursor Bugbot for commit 037770d. Configure here.

Implements DDD integration tests for ExecutionEngine with real infrastructure:
- Real SQLite repositories (in-memory)
- Real domain services (DependencyResolver, OutputStorage)
- Real event bus with event capture
- Mock only external boundary (AgentGateway)

Test files (33 tests total):
- test_execution_lifecycle.py (4 tests)
- test_sequential_execution.py (4 tests)
- test_parallel_execution.py (5 tests)
- test_error_retry.py (5 tests)
- test_gateway_routing.py (4 tests)
- test_timer_steps.py (3 tests)
- test_event_publishing.py (5 tests)
- test_output_persistence.py (3 tests)

Infrastructure:
- MockAgentGateway with configurable responses and call tracking
- EventCapturingBus for asserting domain events
- IntegrationTestContext helper for async event handling
- 6 process definition builders
- 3 assertion helpers

Reference: BACKLOG_RELIABILITY_IMPROVEMENTS.md (RI-01 through RI-09)
Implements execution recovery service that runs on backend startup to detect
and recover executions interrupted by restarts (deployment, crash, OOM).

Recovery Actions:
- RESUME: Execution between steps → continue from next pending step
- RETRY_STEP: Step was RUNNING → reset to PENDING and resume
- MARK_FAILED: Execution > 24h old → fail with timeout message
- SKIP: Already in terminal state → no action needed

Implementation (RI-10 to RI-14):

1. ExecutionRecoveryService (services/process_engine/services/recovery.py)
   - recover_on_startup() method called from main.py lifespan
   - RecoveryConfig for customization (max_age_hours, dry_run)
   - RecoveryReport with detailed counts and errors

2. Domain Events (domain/events.py)
   - ExecutionRecoveryStarted: Scan begins
   - ExecutionRecovered: Per-execution recovery action
   - ExecutionRecoveryFailed: Recovery attempt errored
   - ExecutionRecoveryCompleted: Scan complete with summary

3. Backend Integration (main.py, routers/executions.py)
   - Recovery runs during startup lifespan
   - GET /api/executions/recovery/status endpoint for health checks
   - Non-blocking: startup continues even if recovery fails

4. Integration Tests (12 tests in test_execution_recovery.py)
   - test_resume_execution_between_steps
   - test_resume_pending_execution
   - test_retry_running_step
   - test_retry_first_step_running
   - test_mark_failed_old_execution
   - test_mark_failed_custom_age_threshold
   - test_skip_completed_execution
   - test_mixed_batch_recovery
   - test_recovery_continues_on_error
   - test_recovery_events_emitted
   - test_dry_run_mode
   - test_last_recovery_report_stored

Documentation:
- BACKLOG_RELIABILITY_IMPROVEMENTS.md: Added stories RI-10 to RI-14
- PROCESS_ENGINE_ROADMAP.md: Marked IT5 P0 as complete

All 45 integration tests passing.

Reference: PROCESS_DRIVEN_THINKING_IT5.md Section 2.3
Reference: BACKLOG_RELIABILITY_IMPROVEMENTS.md RI-10 to RI-14
- Add ProcessPermission enum (13 permissions) and ProcessRole (5 roles)
- Implement ProcessAuthorizationService with role-based access checks
- Add AuditService with append-only SqliteAuditRepository
- Create audit API endpoints (GET /api/audit - admin only)
- Add ExecutionLimitService (global: 50, per-process: 3 concurrent)
- Integrate authorization into process and execution API routes
- Add limits status endpoint (GET /api/executions/limits/status)
- 64 unit tests for authorization (42) and audit (22)
- Update BACKLOG_INDEX.md and PROCESS_ENGINE_ROADMAP.md

Refs: IT5 Section 5 (Access Management)
Phase 1: Test Agents
- process-echo: Minimal agent with predictable JSON output
- process-worker: Standard agent with file operations
- process-failer: Configurable failure modes for error testing

Phase 2: Test Cases (22 total)
- Tier 1 (T1.1-T1.4): Critical path - single step, sequential, dependencies
- Tier 2 (T2.1-T2.4): Conditional logic - XOR gateway, default route, parallel
- Tier 3 (T3.1-T3.4): Human approval - approved, rejected, timeout, artifacts
- Tier 4 (T4.1-T4.5): Error handling - errors, timeout, retry, skip, cancel
- Tier 5 (T5.1-T5.5): Edge cases - 10-step chain, diamond, nested expr, limits

Phase 3: Results Tracking
- Test run template for documenting findings
- Initial test run document (2026-01-17)

Documentation updates:
- BACKLOG_INDEX.md: Added manual_run reference
- PROCESS_ENGINE_ROADMAP.md: Marked infrastructure ready

Refs: PROCESS_ENGINE_ROADMAP.md Phase 1
Test Results (4/4 PASS):
- T1.1: Single agent step (~10s)
- T1.2: Two sequential steps (~20s)
- T1.3: Three steps with dependencies (~50s)
- T1.4: Four steps with parallel execution (~40s)

Fixes applied to test files:
- Changed trigger type 'manual' to 'webhook' (manual not supported)
- Changed version '1.0.0' to '1.0' (only major.minor format)
- Increased timeout from 30s to 120s for agent tasks
- Replaced '/structured' command with natural language request
- Updated CPU resource from '0.5' to '1' (integer required)

Issues discovered:
- CLAUDE.md not auto-injected during deploy-local (needs code fix)
- Slash commands cause empty response in Claude agents

Documentation:
- Updated test-run-2026-01-17.md with full results
- Documented root causes and fixes

Refs: PROCESS_ENGINE_ROADMAP.md Phase 1
When deploying an agent via the deploy-local endpoint, automatically
inject the CLAUDE.md custom instructions into the agent's workspace
if the file is present in the deployment archive.

This ensures that agents deployed locally receive their custom
instructions without requiring a manual injection step.

Changes:
- Added step 12 to deploy.py: check for CLAUDE.md in extracted archive
- If present, POST content to /api/trinity/inject endpoint
- Follows same pattern as credentials hot-reload (with 2s wait)
- Non-blocking: logs warning if injection fails

Discovered during Process Engine manual testing when agents didn't
receive their custom instructions after deploy-local.
Test Results:
- T2.1: Exclusive gateway (XOR) routing ✅
- T2.2: Gateway with default route fallback ✅
- T2.3: Parallel execution (fork/join) ✅
- T2.4: Step-level conditional skip ✅

Key Findings:
- Gateway routes use 'target' field (not 'next')
- Default route via 'default_route' at step level
- Step conditions use path syntax (steps.x.output.y)
  not template syntax ({{steps.x.output.y}})

Fixes Applied:
- Fixed all T2 YAMLs with correct gateway syntax
- Added depends_on and conditions for path steps
- Documented Issue #4 in test results

Running Total: 8/22 tests passing (36%)
Refs: PROCESS_ENGINE_ROADMAP.md Phase 1
Test Results:
- T3.1: Human approval (approved) ✅
- T3.2: Human approval (rejected) with on_error:skip_step ✅
- T3.3: Approval timeout ⚠️ (not implemented - needs scheduler)
- T3.4: Approval with artifacts/context ✅

Key Findings:
- Approval API works: GET /api/approvals, POST .../approve, POST .../reject
- Rejection causes step to FAIL by design (use on_error:skip_step)
- Timeout enforcement NOT implemented (deadline set but not checked)
- Approval description can include dynamic context

Known Limitations:
- Issue #5: Approval timeout requires scheduler to enforce
- Issue #6: Rejection = failure (by design, documented)

Running Total: 11/22 tests passing (50%)
Refs: PROCESS_ENGINE_ROADMAP.md Phase 1
Test Results:
- T4.1: Agent error (AGENT_UNAVAILABLE) ✅
- T4.2: Agent timeout ⚠️ (bug: step status not updated)
- T4.3: Retry policy ✅
- T4.4: Skip on error (on_error:skip_step) ✅
- T4.5: Cancel execution ✅

Key Findings:
- Non-existent agent triggers clean AGENT_UNAVAILABLE error
- on_error: {action: skip_step} works correctly
- Cancel API works immediately

BUG FOUND (Issue #7):
- Step timeout detected (error.code='TIMEOUT')
- But step status remains 'running' instead of 'failed'
- Execution doesn't transition to failed state
- Impact: Timeout processes may hang indefinitely

Running Total: 15/22 tests passing (68%) - ABOVE TARGET ✅
Refs: PROCESS_ENGINE_ROADMAP.md Phase 1
Test Results:
- T5.1: 10-step sequential chain ✅ (~60s)
- T5.2: Diamond pattern (double fork/join) ✅ (~70s)
- T5.3: Nested expressions ✅
- T5.4: 3 concurrent executions ✅
- T5.5: Recovery test ⚠️ (manual - requires restart)

Key Findings:
- Long chains (10 steps) work without issues
- Complex dependency graphs (diamond pattern) resolve correctly
- Concurrent executions supported
- Average ~6s per agent step

FINAL SUMMARY:
- Total: 19/22 tests PASSING (86%)
- Target was: 14/22 (64%)
- Exceeded target by 22%

All 5 tiers complete!
Refs: PROCESS_ENGINE_ROADMAP.md Phase 1
Live testing with human approver:
- Approval flow: ✅ Works end-to-end
- Rejection flow: ✅ Works (triggers skip via on_error)

UI observations noted:
- 'Paused' status not clear it's waiting for approval
- Approve/Reject buttons require extra click to reveal
- No console errors ✅
Interactive Test Results:
- I1: Approval Routes ✅
- I2: Multi-Stage Approval ✅ (revealed output bug)
- I3: Complex Workflow (Gateway + Approval) ✅
- I4: Parallel Work + Approval ✅

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

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

4 interactive scenarios created for future testing:
- processes/interactive/i1-approval-routes.yaml
- processes/interactive/i2-multi-stage-approval.yaml
- processes/interactive/i3-complex-workflow.yaml
- processes/interactive/i4-parallel-work-approval.yaml
Bug Fixes:
- T4.2 FIXED: Added TIMEOUT to non_retryable_errors
  - Timeouts now cause immediate step failure (no retries)
  - Step status correctly transitions to 'failed'

- T5.5 PASS: Verified ExecutionRecoveryService works
  - Backend restart during execution
  - Recovery service resumed execution successfully

Partial Fix:
- T3.3: Added deadline check in human_approval handler
  - Checks deadline when handler is re-invoked
  - Full enforcement needs background scheduler

Code Changes:
- execution_engine.py: Added TIMEOUT, APPROVAL_TIMEOUT to non_retryable_errors
- human_approval.py: Added deadline expiry check on execute()

Test Results: 21/22 passing (95%)
- Only T3.3 (approval timeout) needs scheduler
- All other tests passing
UI Observations Addressed:
1. 'Paused' status now shows '🔔 Awaiting Approval' when step is waiting_approval
2. Prominent approval alert banner at top of ExecutionTimeline (auto-visible)
3. Added 'Review Now' button in banner to jump to approval step
4. Auto-refresh now includes 'paused' status (needed for approvals)
5. Skipped steps now show WHY they were skipped (reason, error code)
6. Added 'Awaiting Approval' filter option in ExecutionList

Components Updated:
- ExecutionTimeline.vue: Approval banner, skipped reason display
- ProcessExecutionDetail.vue: Enhanced status, paused auto-refresh
- ExecutionList.vue: Display status helpers, filter option
- style.css: Subtle pulse animation for approval alerts
- Show 'Awaiting Approval' for ALL paused executions in list view
- List view doesn't have step data, so can't check waiting_approval status
- Since paused = waiting for approval, this is correct behavior
Implementation of premium onboarding experience and in-app documentation:

**Backlog & Planning (E20-E24)**
- Created BACKLOG_ONBOARDING.md with 17 stories across 5 epics
- Updated BACKLOG_INDEX.md with E20-E24 epics (102 total stories)
- Created feature flow documentation

**E20: Empty States & Quick Wins (Sprint 7)**
- E20-01: Enhanced ProcessList empty state with:
  - Hero section with value proposition
  - Quick-start template cards (3 templates)
  - Create from scratch / import YAML options
- E20-02: OnboardingChecklist component:
  - 5-item progress checklist (3 required, 2 optional)
  - LocalStorage persistence per user
  - Auto-detect completion from API data
  - Collapsible/dismissible UI
- useOnboarding composable for state management

**E21: Documentation Tab (Sprint 8)**
- E21-01: Added Docs tab to ProcessSubNav
- E21-02: ProcessDocs view with:
  - Sidebar navigation with collapsible sections
  - Markdown rendering with syntax highlighting
  - Responsive mobile sidebar
  - Previous/next navigation
- E21-04: Getting started content (3 docs)
- E21-05/E21-06: Reference content (4 docs)
- Backend docs router for content serving

**Documentation Content**
- getting-started/: what-are-processes, first-process, step-types
- reference/: yaml-schema, variables, triggers, error-handling
- troubleshooting/: common-errors

Remaining for future sprints: E21-03 (search), E22 (contextual help),
E23 (guided tours), E24 (wizard)
- Replace 'Start' with 'See above ↑' when already on target page
- Add 'Restart Getting Started' button to Docs page sidebar
- Add confirmation dialog before restart
- Remove debug console.log statements
- Remove hidden keyboard shortcut (replaced by UI option)
- Add notification toast for restart feedback

Refs: BACKLOG_ONBOARDING.md E20-05
- E21-07: Pattern Documentation (sequential, parallel, approvals)
- E21-08: Missing Step Types (notification, sub_process)
- E21-09: Progressive Learning Path with tutorials
- Update story count to 21
- Update dependency graph with new stories

Refs: Review of onboarding coverage gaps
- Getting Started: what-are-processes, first-process, step-types
- Reference: yaml-schema, variables, triggers, error-handling
- Troubleshooting: common-errors
- Backend docs router to serve markdown content

Refs: BACKLOG_ONBOARDING.md E21-04, E21-05, E21-06
- Enhanced empty state with quick-start templates
- Onboarding checklist composable with localStorage persistence
- Auto-detection of completion based on API data
- Feature flow documentation

Refs: BACKLOG_ONBOARDING.md E20-01, E20-02
- Sequential: linear chains, data flow, error handling in chains
- Parallel: fan-out/fan-in, diamond pattern, handling partial failures
- Approvals: single gate, multi-level chains, timeout handling, conditional paths
- Each pattern includes diagrams, complete YAML examples, best practices

Refs: BACKLOG_ONBOARDING.md E21-07
- notification: channels (slack, email, pagerduty), recipients, urgency
- sub_process: process invocation, input_mapping, nested workflows
- Updated overview table with all 6 step types
- Examples for each use case and best practices

Refs: BACKLOG_ONBOARDING.md E21-08
- Level 1: Getting Started (existing - what-are-processes, first-process, step-types)
- Level 2: Intermediate tutorials (NEW)
  - second-process.md: Parallel execution, fan-out/fan-in
  - human-checkpoints.md: Approval gates, routing decisions
- Level 3: Advanced tutorial (NEW)
  - complex-workflows.md: Gateways, multi-path routing, combining patterns
- Updated index.json with tutorials section
- Clear learning progression with prerequisites and 'What's Next' links

Refs: BACKLOG_ONBOARDING.md E21-09
- E21-07: Pattern Documentation ✅
- E21-08: Missing Step Types ✅
- E21-09: Progressive Learning Path ✅

Refs: Sprint 8.5 complete
- Add ./config/process-docs:/app/config/process-docs:ro volume mount
- Enables docs API to serve documentation content

Fixes: 404 errors on /api/docs/* endpoints
- Make restart button amber-colored for better visibility
- Add restart button to mobile sidebar menu
- Minor doc content improvements for clarity
E20-04: First-Run Detection Service
- Enhanced useOnboarding.js with isFirstRun, shouldShowOnboarding
- Added markOnboardingComplete(), dataState tracking
- Auto-detection based on process/execution counts

E22-03: Execution Status Explainers
- Added status explanations with hover tooltips
- Execution-level tooltips in ProcessExecutionDetail
- Step-level tooltips in ExecutionTimeline
- Info icon on status badges

E20-03: Template Cards in Empty State
- Already implemented (verified working)

E22-01: YAML Editor Help Panel
- New EditorHelpPanel component with contextual help
- Cursor position tracking in YamlEditor
- Help content JSON with field descriptions
- Toggle button to show/hide panel
- State persisted in localStorage
- Updated docs router to serve JSON files
- Created onboarding test tier with 35+ test scenarios
- Test categories: UI navigation, empty states, checklist, docs, help panel, status tooltips
- Sample process definitions for testing
- Clear test execution steps and expected results
Only show 'See above ↑' when:
1. User is on the target page AND
2. Prerequisite steps are completed

- runExecution: requires createProcess to be done
- monitorExecution: requires runExecution to be done
Contextual help is already covered by:
- YAML editor help panel
- Execution status explainers
- Onboarding checklist hints

Reduced from 17 to 16 stories.
- Add /processes/wizard route
- Create ProcessWizard.vue with 4-step guided flow:
  1. Goal Selection (Content, Data, Approval, Custom)
  2. Agent Assignment with status badges
  3. Customize step names/messages/timeouts
  4. Review with flow preview and YAML generation
- Add wizard entry points in ProcessList empty state
- Update OnboardingChecklist to link to wizard
- Include 'Run immediately' option after creation
- Integrate with onboarding celebration
All 16 stories in onboarding backlog are now complete:
- E20: Empty States and Quick Wins
- E21: Documentation Tab
- E22: Contextual Help
- E24: First Process Wizard
When creating a new process, users now see a callout suggesting
the guided wizard for beginners, placed between the template
cards and the Continue button.
New 4-story epic for conversational process creation:
- E25-01: Chat Panel UI Component
- E25-02: System Agent Process Creation Prompt
- E25-03: YAML Generation from Conversation
- E25-04: Apply YAML to Editor Integration

Uses Trinity System Agent under the hood with embedded chat UI.
Chat-based process creation using Trinity System Agent:

- ProcessChatAssistant.vue: Chat UI with YAML detection
- ProcessEditor.vue: New 'Chat' tab as default for new processes
- System Agent CLAUDE.md: Extended with Process Creation Assistant mode

Features:
- Suggested prompts for quick start
- YAML code block detection with syntax highlighting
- 'Apply to Editor' button to use generated YAML
- Live YAML preview alongside chat
- System agent status indicator
- Context-aware first message with schema reference
All 20 stories in onboarding backlog are now complete:
- E20: Empty States and Quick Wins
- E21: Documentation Tab
- E22: Contextual Help
- E24: First Process Wizard
- E25: Process Creation Chat Assistant
- Add markdown rendering for assistant messages (using marked)
- Update prompt to be more conversational:
  - Ask ONE question at a time
  - Keep responses concise (2-3 sentences)
  - No markdown bold formatting
  - Build understanding gradually through dialogue
- Add prose styling for rendered markdown
- Add 'Help me fix' banner when validation errors exist
- Pass validationErrors and currentYaml as props
- askForHelp() sends errors + YAML context to assistant
- Fix YAML schema in prompts - fields go at step level, NOT in config block
- Correct agent_task format: agent/message directly in step
- Save conversation to localStorage after each message
- Restore conversation on component mount
- Add 'Clear' button to reset conversation
- Chat survives page refresh and navigation
- Words appear gradually with typewriter effect (30ms per word)
- Blinking cursor shows at end while typing
- 'Thinking...' bouncing dots while waiting for API
- Smooth transition between loading and typing states
- YAML from assistant auto-syncs to editor as it types (no manual apply needed)
- Select code in YAML preview → 'Explain' or 'Edit this' buttons appear in chat
- YamlEditor now emits 'selection-change' events
- Shows 'Auto-synced ✓' indicator instead of 'Apply to Editor' button
- Removed unused manual apply functionality
- Don't ask 'want me to show you?' - just show the updated YAML
- Briefly explain the change, then include the full YAML
- Updated both system agent CLAUDE.md and frontend context
- Pass processStatus prop to ProcessChatAssistant
- Add context about published processes being read-only
- Assistant now tells users to click 'New Version' for published processes
- Also handles archived process status
@oleksandr-korin
oleksandr-korin requested a review from vybe January 19, 2026 17:52
@vybe
vybe merged commit 97fdabb into main Jan 20, 2026
vybe added a commit that referenced this pull request Apr 21, 2026
Closes warning W1 from PR #438 validation — records the `?last-event-id=`
query param, regex gate, REPLAY_GAP_LIMIT ceiling, services/event_bus.py
entry, and updates invariant #10 to name the new transport so future
broadcast sites don't bypass the manager shims.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vybe added a commit that referenced this pull request Apr 21, 2026
#438)

* docs(plan): pivot orchestration sprint to #306 keystone

Adds CLAUDE.md pointer to the orchestration reliability plan and
records the 2026-04-20 revision: pause #294/#291 pending #306, treat
Redis Streams event bus as the keystone for Tier 2.5 simplification,
and gate cleanup collapse on a 2-week push-path soak.

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

* feat: Redis Streams event bus for WebSocket delivery (#306)

Replaces the in-process ConnectionManager + `except: pass` broadcast with
a Redis Streams transport. Reconnects no longer drop events: clients send
`?last-event-id=<stream_id>` and receive missed events via XRANGE catchup
(capped at 5000 entries; trimmed cursors trigger a `resync_required`
marker that the frontend answers with a REST refetch).

Key pieces:
- services/event_bus.py — EventBus publisher (fire-and-forget XADD,
  bounded outbound queue, 10000 MAXLEN env-tunable) + StreamDispatcher
  (one XREAD BLOCK per process, in-memory fan-out, per-client
  asyncio.Queue(256), 3-failure eviction, supervised reader with
  exponential backoff, 2s graceful drain on shutdown).
- main.py — ConnectionManager / FilteredWebSocketManager kept as thin
  shims over the bus so the 33 legacy broadcast call sites don't change.
  /ws and /ws/events accept an optional last-event-id query param
  (regex-validated to `^\d+-\d+$`).
- Frontend WS clients capture `_eid` and replay on reconnect; resync
  handlers refetch authoritative state (agents + activity history +
  pending notifications).
- 23 unit tests: id validation, scope visibility, XADD envelope, fallback
  buffer, eviction, slow-consumer resync, monotonic cursor guard,
  invalid-cursor resync. Plus live roundtrip + reconnect replay verified
  against localhost stack.

Keystone for Tier 2.5 simplification per
`docs/planning/ORCHESTRATION_RELIABILITY_2026-04.md`: #428 (capacity
consolidate), #429 (cleanup collapse), #307 (heartbeat push) will reuse
the same stream primitive; #408 dissolves once agent-push completion
retires the 1h blocking HTTP call. Those are follow-ups — this PR is
WebSocket delivery only.

Closes #306

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

* docs(architecture): document Redis Streams WebSocket transport (#306)

Closes warning W1 from PR #438 validation — records the `?last-event-id=`
query param, regex gate, REPLAY_GAP_LIMIT ceiling, services/event_bus.py
entry, and updates invariant #10 to name the new transport so future
broadcast sites don't bypass the manager shims.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vybe pushed a commit that referenced this pull request Aug 3, 2026
…ership retrofit (ent#109) (#1947)

* fix(lifecycle): one owner for git env across both rebuild paths (ent#109)

`recreate_container_with_updated_config` seeds env from the OLD container and
re-derived only `GITHUB_PAT`, replaying whatever `GITHUB_REPO` / `GIT_SYNC_*`
each container happened to be carrying. The git-env derivation lived only in
`_apply_persisted_auth_env` (`recreate_missing_container`). That split is a
pre-existing fleet-wide bug, not a cosmetic one: the recreate has exactly one
production caller, `start_agent_internal`, which fires on nine config-drift
predicates AND on base-image drift at cold start — so a base-image rebuild
arms the replay for every agent at once.

`_apply_git_env_from_db` is now the single writer. Three load-bearing details:

* **The PAT gate is a parameter, never inherited.** The two paths gate
  differently on purpose. `per_agent_only` (config-drift recreate) preserves
  #211 verbatim — resolve the effective PAT only when the container already
  carries one or a per-agent PAT row exists — so a global-only platform PAT is
  never injected into a previously-tokenless container. A verbatim lift would
  have swapped that for the 2-tier per-agent -> GLOBAL resolver used by
  `effective` (the rebuild-from-nothing path, which has no old container to
  inherit a token from): `configure_push_remote` then clears the push
  blackhole and a tokenless agent can push a private KB to the shared public
  upstream. learnings.md ent#162 names this class exactly.
* **Set-or-clear**, since the recreate writes into a carried-forward dict. A
  deleted `agent_git_config` row pops the whole owned set; a `source_mode`
  flip clears the mode/branch pair. `GITHUB_PAT` alone stays set-only while a
  repo is bound — clearing it would revoke a live agent`s push on an unrelated
  recreate.
* **`GIT_SYNC_AUTO` = DB flag OR baked env**, plus a convergence backfill.
  crud.py`s two writers genuinely disagree (`and not config.ephemeral` sits
  inside a swallowing try/except on the DB side only; the column defaults to
  0), so deriving from `auto_sync_enabled` alone would silently stop auto-push
  for that slice of the fleet. The backfill writes the column the moment the
  disagreement is observed, so the OR retires itself. Making the #389 toggle
  authoritative is a separate follow-up.

ent#123 is preserved: the gate is the REPO, not the PAT, so a tokenless agent
rebuilt after container loss still clones (#843/#1439 silent-empty class).

One deliberate divergence from a verbatim lift, asserted by test: a container
with a baked `GITHUB_PAT` and NO git binding previously had that token
refreshed from the global platform PAT on every recreate; it is now popped.
The per-agent PAT is a column ON `agent_git_config`, so "no row" means no
per-agent credential and no repo to push to by construction.

Tests: tests/unit/test_ent109_git_env_seam.py — each of the four behaviours
proved to have teeth by mutation (un-gate the PAT, flip the call site to
`effective`, derive GIT_SYNC_AUTO DB-only, drop the clear sweep, drop the
source-mode clear, diverge the GIT_SYNC_AUTO literal, unguard the backfill:
all seven go red). Plus a static call-site guard, so flipping either gate
fails CI even though no behavioural test of the helper alone would catch it.

Refs Abilityai/trinity-enterprise#109 (PR 1 of 3)

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

* docs(architecture): _apply_git_env_from_db owns git env on both rebuild paths (ent#109)

Adds the missing agent_service/lifecycle.py catalog entry and records the
per-call-site PAT gate, the set-or-clear contract, and the GIT_SYNC_AUTO
OR-derivation. Amends the ent#123 clause to point at the new shared seam
instead of _apply_persisted_auth_env.

Refs Abilityai/trinity-enterprise#109

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

* test(registry): register test_ent109_git_env_seam.py

Refs Abilityai/trinity-enterprise#109

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

* docs(feature-flows): sync the git env-derivation seam (ent#109)

github-sync.md: retitle the rebuild-recovery section to
"Container-rebuild env — lifecycle.py::_apply_git_env_from_db" and document
the per-call-site PAT gate, the set-or-clear contract, the GIT_SYNC_AUTO
OR-derivation, and the two vars deliberately NOT owned.

git-sync-health.md: GIT_SYNC_AUTO is re-derived on every rebuild as
auto_sync_enabled OR the baked env (the two creation writers disagree), with a
self-retiring backfill; kill-switch row and file table corrected.

agent-lifecycle.md: Revision History row.
feature-flows.md: hand-added Recent Updates row (the skill drops it past ~400
lines). Note: that table is at 56 rows against its stated ~20 cap (#1360) —
pre-existing drift, deliberately not trimmed here.

Refs Abilityai/trinity-enterprise#109

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

* test(#735): re-anchor the lifecycle PAT call-site guard onto _apply_git_env_from_db

ent#109 moved GITHUB_PAT derivation out of the inline block in
recreate_container_with_updated_config (which the guard anchored on via the
comment "Update GITHUB_PAT") into the shared _apply_git_env_from_db. The
guard intent is unchanged and still enforced: that block resolves the
effective per-agent PAT, never the platform-only get_github_pat().

Also fixes a silent-degradation flaw in the guard itself. str.find returns
-1 on a miss, and src[-1:-1+300] slices to an EMPTY string — so a moved
anchor made the guard assert "get_github_pat_for_agent in \x27\x27", failing with no
hint about why. The anchor is now asserted first with a message naming the
fix (re-point it, do not delete it), and the block is sliced to the next
top-level def rather than a fixed byte window.

Both failure modes proved red by mutation: swapping the helper to
get_github_pat() and renaming the anchored function.

Refs Abilityai/trinity-enterprise#109

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

* fix(lifecycle): correct-never-introduce git env; drop the auto-sync backfill (ent#109)

Two defects found reviewing the ent#109 PR 1 env seam.

1. The config-drift recreate blackholed push for agents bound post-creation.
   The repo half of the block is repo-gated (ent#123) while the PAT half keeps
   #211's narrower per-agent gate, and those two disagree for one real row
   shape: an agent bound via POST /{agent}/git/initialize on the GLOBAL
   platform PAT. That path writes an agent_git_config row and pushes, but never
   recreates the container, never bakes git env, never persists a per-agent PAT
   row, and never writes the token into the workspace .env — so its only
   credential is the one embedded in .git/config's origin URL, and startup.sh's
   #1264 fallback does not cover it. Handing startup.sh GIT_SYNC_ENABLED=true
   with no GITHUB_PAT is exactly what it reads as "deliberately tokenless": the
   restart branch rewrites origin to the credential-less CLONE_URL, destroying
   that token, and configure_push_remote blackholes the push remote — silently,
   and fleet-wide on the same base-image drift this helper exists to fix.

   `per_agent_only` now writes the block only when the old container already
   carried GITHUB_REPO or a PAT resolves. It still corrects a stale repo, a
   flipped source_mode and a deleted row — every case the fix is about; a
   tokenless ent#123 agent carries GITHUB_REPO from creation, so the flagship
   is unaffected. `effective` is exempt: with no old container, NOT introducing
   the block is the #843/#1439 silently-empty-agent bug.

2. The GIT_SYNC_AUTO backfill erased an owner's explicit disable.
   PUT /{agent}/git/auto-sync writes the row and nothing else while the agent
   gates on container env, and creation sets both true for the ordinary
   non-source-mode PAT agent — so "baked true / DB 0" is also exactly what an
   owner's disable looks like. The backfill re-enabled it on the next recreate
   and erased the only record of the intent, so the toggle could never stick.
   It was a privilege boundary too: PUT .../auto-sync is OwnedAgentByName while
   POST .../start, which triggers the recreate, is AuthorizedAgentByName — so a
   shared non-owner, or an agent-scoped key resolving to its owner with the
   owner's role (trinity-ops-agent#232), flipped an owner-only flag arming a
   15-minute background commit-and-push loop.

   The OR-derivation stays (crud.py's two creation writers genuinely disagree,
   and DB-only derivation would silently stop auto-push for that slice). The
   write-back is gone; the disagreement is logged. Making the #389 toggle
   authoritative remains the tracked follow-up that retires the OR honestly.

Tests 17 -> 22: a TestIntroduceGuard class (unbaked container untouched,
carried repo still corrected, resolvable PAT still introduces, effective
exempt, clear sweep unaffected) and the derive-only assertion. Both fixes
proved to have teeth by mutation — removing the guard and restoring the
backfill each go red on exactly one test. Two learnings.md entries.

Refs trinity-enterprise#109

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

* test(ent#109): pin the git-env WRITER SET with an AST guard, not a source grep

The previous call-site guard sliced `lifecycle.py` by function header and
counted a single-line literal in each half. Two blind spots:

  1. It pinned only the two KNOWN sites. ent#109's bug WAS that git env had
     two writers and one of them was wrong; a THIRD writer added later on any
     container-seeded path re-opens exactly that hole, and the grep version
     stayed green through a planted `pat_gate="effective"` writer (verified by
     mutation).
  2. `lifecycle.py` names the helper in two comments, so a substring count
     read prose as call sites — the same first blind spot the #1871 guard hit.

The AST walk maps `{enclosing function: pat_gate literal}` and asserts the set
equals exactly `{recreate_container_with_updated_config: per_agent_only,
_apply_persisted_auth_env: effective}`. It also fails loud on a non-literal or
omitted `pat_gate` and on a duplicate call in one function — each of which
would make the guard silently vacuous, which is worse than the leak it guards.

Also drops the stale "convergence backfill" wording from the module docstring
and the registry entry (d8da9d08 removed the backfill; the description still
described it) and re-states the idempotence test as "the DB row is never
mutated".

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

* docs: describe the ent#109 guard as a writer-set gate, not a call-site check

Follow-on to the AST guard: `architecture.md` and the `agent-lifecycle.md`
change log both said the static guard "fails CI if either call site flips",
which understates what it now enforces. It pins the whole writer SET, so a
third writer on any container-seeded path fails CI too — the property that
matters, since ent#109's bug was two writers with one of them wrong.

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

* docs(requirements): add github §11.12 post-creation repo binding (ent#109)

Trinity Rule #1 — requirements before implementation.

§11.12 specifies the "bind to your own repo" retrofit: FR-1 the explicit
supported-row table keyed on source_mode (the column the partial unique index
actually keys on) with named structural refusals for everything else, FR-2
source_mode preserved at 1 so no branch reservation is needed, FR-3 the
destination-scoped fail-closed lock + CAS + compensating restore (never
delete_git_config on a pre-existing row — that is destruction, not rollback),
FR-4 the PAT persisted last, FR-5 the mandatory recreate because startup.sh
rewrites origin unconditionally from baked env, FR-6 owner-only AND human-only
with explicit PAT disclosure, FR-7 the no_write_credentials surfaces.

Also amends §11.11 FR-5: the tokenless push refusal no longer teaches the
create-a-new-agent-and-import workaround.

Refs ent#109

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

* refactor(fork-to-own): extract the shared destination primitive (ent#109 §4.5)

AC #4 asks the post-creation rebind to reuse ent#93's machinery rather than
build a parallel path. The seam is NOT the destination triage lifted whole —
that is not expressible, because the create path's reuse branch IS the
template-tip SHA comparison, interleaved with the triage in one if/elif/else.

So the seam is one level lower: inspect_or_create_destination_repo() reports
created | empty | branches and never decides. Reuse/refuse POLICY stays in
each caller, because the two callers genuinely disagree — the create path
compares against a template tip; the rebind has no template, its content
source is the agent's workspace volume, so any existing branch is a refusal.

validate_destination_pat() is a SIBLING, not folded in: the create path
validates the PAT before resolving the template tip, so 'bad PAT + unreachable
template' reports FORK_PAT_INVALID. Folding it into the inspect primitive
(which runs after the tip resolves) would silently reorder that into a
template error.

Behaviour preservation is asserted, not claimed: the 40 pre-existing
test_fork_to_own.py tests pass unchanged, and both new guards were shown to
have teeth — making the primitive refuse instead of report turns the create
path's SHA-match reuse red, and swapping the validate/resolve order turns the
ordering guard red.

Refs ent#109

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

* feat(git): bind an agent to a GitHub repo you own (ent#109)

POST /api/agents/{name}/git/bind-to-own-repo — create a user-owned repo from a
LIVE agent's current workspace, rebind origin in place, persist the per-agent
PAT, and re-bake the container env so the rebind survives a restart. Plus a
GET .../status companion so a client that eats a proxy timeout can resolve the
outcome from state rather than from a remembered request.

Shape, per requirements §11.12:

- Orchestration in services/agent_service/repo_binding.py, NOT the router
  (Invariant #1). It raises BindError and never HTTPException; the router is a
  thin mapper owning only the two locks, the idempotency claim, and the audit.
- Classification partitions on source_mode — the column the partial unique
  index actually keys on — and refuses every other shape BY NAME rather than
  mis-routing it. Credential state is an orthogonal column and is not used.
- Concurrency: a DESTINATION-scoped lock is the one that serializes the real
  collision (two different agents, one destination repo); the agent-scoped
  lock only guards double-submit. Both FAIL CLOSED with 503 + Retry-After —
  agent_data's fail-open is calibrated for a tar round-trip, not for two repo
  creates and two concurrent recreates of one container.
- The CAS in db.rebind_git_config is the whole commit point, its predicate
  named in the docstring. The loser path restores the captured previous values;
  it never calls delete_git_config, which on a pre-existing row is destruction
  (the next recreate would drop GITHUB_REPO — #843/#1439).
- The PAT is persisted LAST and strictly before the recreate: earlier makes the
  agent look already-writable on a retry, later bakes a repo-bound container
  with no token that startup.sh then blackholes.
- Post-rewire, origin is read back and confirmed — a set-url that exits 0
  without taking effect is exactly the silent mismatch AC #5 forbids.
- Owner-only AND human-only (reject_agent_principal): an agent-scoped key
  resolves to its owner carrying the owner's role, so a role gate alone is
  satisfied by any agent's injected key on a default admin-owned install.

Decision #17 (check_github_repo_env_matches) is deliberately CUT: the only
drift-proof way to build it is to call _apply_git_env_from_db, which turns PR
1's AST writer-set guard red, and idempotent retry already supplies the
convergence it was meant to buy. BIND_RECREATE_FAILED states the retry path
instead of a convergence promise, and warns against a plain restart.

Refs ent#109

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

* test(git): cover the repo-binding commit point, ordering and secret hygiene (ent#109)

31 tests over the properties that would otherwise only be true by inspection:

- Classification: the supported shape succeeds; every other shape is refused
  BY NAME. Two cases asserted rather than argued — an already-writable agent
  is an ORDINARY rebind (the refusal that used to sit there is what made the
  documented retry unreachable), and trinity-system is refused through the
  no-git-config path so it never reaches the recreate that bypasses #1816's
  running-system gate.
- Commit point: a moved row yields 409 with nothing partial, and the
  post-commit loser is RESTORED to its captured previous values. Asserts
  delete_git_config is never called — on a pre-existing row that is
  destruction, and the row is asserted to still exist afterwards.
- Ordering: rebind -> pat -> recreate, proven by recorded call order. A push
  failure persists no PAT; a PAT-persist failure blocks the recreate; and
  fail-at-push -> retry -> success is an explicit regression test for the
  contradiction that a 409-on-retry used to produce.
- The CAS statement runs against a REAL SQLite engine, not a double — the
  predicate is the whole safety argument, so a stub cannot verify it. Includes
  two racers reading the same expected value: exactly one wins.
- Secret hygiene: the PAT is absent from the outcome, the audit dict and every
  error path, and a stale baked token in git output is redacted too.

That last group found a real defect, now fixed: repo_binding composed its
failure messages from foreign text (git output, a docker exception) and
relied on the producer having scrubbed. git_service scrubs what it reads from
a container, but the docker and GitHub exception paths arrive through
libraries that never saw the token. Added _scrub() as a belt at the boundary
where the PAT is actually in scope.

Refs ent#109

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

* fix(git): retire the no_write_credentials create-a-new-agent workaround (ent#109)

ent#230's sharpest AC, which ent#109 omitted: the no_write_credentials
surfaces must point at the retrofit once it exists. Both change together
(Invariant #13):

- git_service.NO_WRITE_CREDENTIALS_MESSAGE (consumed by sync_to_github and
  reset_to_main_preserve_state, mapped 409 in routers/git.py)
- the MCP 409 hint in src/mcp-server/src/tools/git.ts

Neither now teaches 'create a new agent with fork-to-own and import your
data' — an instruction that discards the agent's identity, its 180-day name
reservation and its history. ent#123's carve-out is preserved: this branch
still suppresses the chat_with_agent remedy, because a chat turn cannot
conjure credentials.

The third surface — startup.sh's push-remote blackhole sentinel — is
deliberately unchanged and now asserted as such: it is a git remote URL (one
shell-safe token) that already names a remedy, and editing it would force a
base-image rebuild for cosmetics.

The parity guard was teeth-checked in both directions: reverting the MCP hint
turns it red, AND breaking the source anchor turns it red with a named error
rather than silently asserting against an empty slice — the way a source-grep
guard usually dies.

Refs ent#109

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

* feat(ui): 'Bind to your own repo' panel on the Git tab (ent#109)

A new BindRepoPanel.vue mounted in GitPanel.vue rather than more markup inside
it, for two reasons: GitPanel is already 639 lines, and #1430's raw-color
ratchet is PER FILE — appending a form there would raise counts that may only
shrink. GitPanel's numbers are unchanged at 24 nongray / 146 gray; the new
panel is at ZERO raw non-gray with 51 semantic tokens (its 46 grays are the
contract's own surface/ink vocabulary — there are no Base* primitives in the
repo yet to absorb them).

Design-system contract (read first, per CLAUDE.md rule #10): semantic tokens
only (action-primary / status-success / status-warning / status-danger), both
themes first-class, gray-750 for dark chrome, and no dark:text-gray-500 —
the dark ink floor.

Behaviour worth noting:

- The store method uses raw axios with an explicit 300s timeout, following the
  surrounding idiom. It deliberately does NOT use api.js, whose instance-wide
  30s timeout is far below this call's worst case; aborting the client mid-bind
  strands the user past the commit point with no response, which is the exact
  situation the status endpoint exists to rescue rather than manufacture.
- The PAT is read out of the reactive ref BEFORE the await and cleared
  immediately, so it never lingers regardless of how the request ends.
- A client timeout is reported as PARTIAL, never as a clean failure — the
  request may well have landed.
- Post-commit failures render as 'Partly applied — action needed' in warning
  colour rather than as an error, because the binding genuinely IS saved and
  telling the user it failed would send them looking in the wrong place.
- The restart warning states what happens, what is preserved, and how long.

Both SFCs verified against the real @vue/compiler-sfc (parse + script +
template); npm run check:tokens passes.

Refs ent#109

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

* docs: architecture + feature flow for post-creation repo binding (ent#109)

- architecture.md: two endpoint rows, a Post-Creation Repo Binding subsystem
  block, the two Redis lock keyspaces, repo_binding.py + git_service's new
  primitives in the service catalog, the shared destination seam on the
  fork_to_own entry, and the ent#123 paragraph tail now that its
  no_write_credentials refusal points at the retrofit. PR 1's
  _apply_git_env_from_db prose is already present on this branch and was NOT
  re-added.
- New feature-flows/agent-repo-binding.md: the end-to-end trace, the five
  decisions that carry the design (source_mode partition, destination lock,
  CAS + restore-not-delete, PAT-last, mandatory recreate), the error registry
  with which codes are partial, the ent#93 sharing seam, security, and known
  limits — including why Decision #17's drift predicate was cut.
- feature-flows.md: Recent Updates row added BY HAND (/sync-feature-flows
  drops it past ~400 lines) plus the category-table entry.
- Cross-linked the three affected flows: github-sync.md and mcp-git-tools.md
  had the retired workaround quoted verbatim in their prose, and
  github-repo-initialization.md now names its post-creation sibling and the
  boundary between them.

Refs ent#109

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

* test(git): cover the bind ENDPOINT surface (ent#109)

/update-tests review found the router layer uncovered — its own rule says a
new or changed endpoint needs a caller that exercises the path params and auth
dependency (#1069's 422-every-call class). 26 tests over the five things no
service-level test can see:

- reject_agent_principal really called, and wired in the handler rather than
  merely imported (an agent-scoped key resolves to its owner CARRYING the
  owner's role, so an owner/role gate alone is satisfied by any agent's
  injected key on a default admin-owned install)
- route path-param matches the handler parameter, for both routes
- locks FAIL CLOSED on a Redis outage, on a raising SETNX, and on contention;
  the destination key is case-folded; locks release on success AND failure
- idempotency key is verb-folded; absent header derives nothing; in-flight
  409; completed replay returns the snapshot with X-Idempotent-Replay
- audit on EVERY exit path incl. lock contention and the unexpected 500

Also fixes a regression this work introduced: test_ent123_tokenless_clone.py
asserted the literal retired wording of NO_WRITE_CREDENTIALS_MESSAGE, and I had
not re-run that suite after changing the shared constant. Re-anchored on the
CONSTANT plus the invariant ent#123 actually cares about (named message, still
actionable) — stronger than before, and it cannot drift again; the exact copy
is owned by test_ent109_no_write_credentials_message.py.

Both new guards mutation-verified: deleting reject_agent_principal and making
the lock fail open each turn two tests red.

Full unit suite: 6350 passed, 14 skipped, 0 failed. Identical under random and
fixed order (no sys.modules pollution across the new modules).

Refs ent#109

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

* docs(flows): /sync-feature-flows pass over the ent#109 binding surface

Verification of the hand-written docs against the code found two gaps:

- template-processing.md described the fork-to-own copy pipeline's steps 1-2
  as living inline in fork_to_own.py. They are now the SHARED half
  (validate_destination_pat + inspect_or_create_destination_repo), so a
  reader tracing the code would have found the triage in a different function
  than documented. Updated to name the seam and why it sits one level below
  the triage, with the reuse/refuse policy explicitly still owned by that
  caller. Behaviour there is unchanged.
- The new flow's error registry was missing three codes that ARE reachable on
  the bind path: FORK_DESTINATION_UNREACHABLE (shared primitive),
  BIND_DESTINATION_UNREACHABLE (fail-closed guard-read failure) and
  BIND_UNEXPECTED_ERROR (router catch-all). Verified by diffing the codes in
  the source against the codes in the doc; the seven still absent are
  create-path-only and correctly omitted.

Checked and deliberately NOT changed: git-sync-health.md and
dark-mode-theme.md reference the touched files but document nothing this PR
alters. The Recent Updates table is 66 rows against its own stated ~20 cap —
pre-existing drift (65 before this PR); trimming 46 of other people's entries
is unrelated churn on a feature PR.

Refs ent#109

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

* fix(git): converge the documented bind retry; stop a PAT leaking on rejection (ent#109)

Fixes from /review (C1, I1, I3, I6, I2) and /cso --diff (S1) on PR 2.

── /review C1: every post-commit failure promises an idempotent retry, and all
   four are refused ──────────────────────────────────────────────────────────

The CAS is the commit point, so after it the row names the destination while
the container's origin still names the old repo — and both pre-flight gates
read that skew as a refusal:

  push/rewire fail   -> row moved, origin did not -> 409 BIND_STATE_UNCLASSIFIED
  PAT/recreate fail  -> destination holds our own pushed history
                                                  -> 409 BIND_DESTINATION_EXISTS

The §0.4 class the plan wrote a section to eliminate for the PAT ordering,
re-entering through the classification guard. The vestige was in the signature:
`_classify(agent_name, destination_repo)` never used `destination_repo` — the
carve-out had been designed and not written.

A row already naming the requested destination is now a resumption:

* origin may lag — it never selects what is pushed (step 4 pushes
  refs/heads/<branch> from the workspace by explicit URL, writes origin after),
  and it cannot be tightened anyway: a committed CAS has overwritten the old
  repo name, so "still the old repo" and "something else" are
  indistinguishable, and treating the ambiguity as fatal strands the agent.
* existing branches are accepted — bounded by git, not trust: the push carries
  no --force and no `+` refspec, so unrelated history is rejected
  non-fast-forward and an unrelated branch is untouched.
* previous_repo=None on a resume leaves `upstream` alone instead of repointing
  it at the destination itself, erasing the provenance the rebind preserves.

A mismatch against any OTHER repo stays BIND_STATE_UNCLASSIFIED.

The regression test written for exactly this was green because its double
returned `origin_repo=fake_db.config.github_repo` — the container's observed
state WAS the row, so they could never disagree — and a hand-set
`dest_state = "empty"` stepped around the other gate. The fixture now tracks
the container independently and mirrors the real side effects.

── /cso S1: a GitHub PAT reaches the response body and the platform log ──────

A PAT is sent as `Authorization: Bearer <pat>`, and h11 rejects an illegal
header value by ECHOING it (verified: `LocalProtocolError: Illegal header value
b'Bearer ghp_...\r'`). The validator only checked non-emptiness and returned the
value UNSTRIPPED, so a token carrying a trailing \r or \n — what a paste from a
terminal or clipboard routinely produces — surfaced raw in a 500 body and, via
logger.exception, in the Vector-captured platform log. Trigger is far more often
an ordinary paste than an attacker.

* `models._validate_pat_secret` strips whitespace and rejects anything outside
  printable ASCII, on BOTH BindAgentRepoRequest and ForkToOwnRequest (ent#93's
  create path feeds the same GitHubService constructor).
* That alone would only RELOCATE the leak: Pydantic v2 records the rejected
  value in errors()["input"] and FastAPI returns exc.errors() verbatim — proven
  against a real TestClient. `error_handlers.validation_error_without_input`
  strips `input` from every 422 entry. Dropped for all fields, not for names
  that look sensitive: a name allowlist is the new-producer-missing-from-the-
  consumer's-list class, and the caller already has the value they sent.
* The router catch-all and the PAT-persist log line now scrub, and the
  dual-scrub itself collapses from two copies into one home in
  `utils/credential_sanitizer` (fork_to_own re-exports for its callers).

── Also ─────────────────────────────────────────────────────────────────────

* The bind is `recreate_container_with_updated_config`'s SECOND production call
  site and skipped the `clear_agent_breakers` that `start_agent_internal` runs
  immediately before its own call — both breakers are agent-name-keyed with no
  TTL, so the replacement container inherited its predecessor's verdict
  (#1560). Cleared before the recreate, not after. Two stale "one production
  caller" claims corrected.
* Audit rows on the two idempotency-replay exits, so "exactly once per exit
  path" (#905) is literally true.
* Client timeout resolves against the status endpoint instead of telling the
  user to reload the tab.
* Five test modules registered in tests/registry.json.

Each of the six behaviour fixes was mutation-checked (revert -> red -> restore),
including the breaker clear in both directions (absent, and after the recreate).
Verified: 6332 backtest unit tests pass; the original C1 probe — written before
the fix and unchanged — now reports both post-commit shapes converging; frontend
`vite build` and the design-token check pass; GitPanel's raw-color counts are
unchanged from baseline and BindRepoPanel is at raw_nongray 0.

Refs Abilityai/trinity-enterprise#109

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

* test(git): assert the bind routes are wired to the enumeration-safe deps (ent#109)

Plan §7 lists "uniform 404 for unknown *and* inaccessible agent (Invariant #8)"
as a PR 2 case, and it was the one bullet with no test behind it.

The 404 BEHAVIOUR is not re-tested here — `test_186_enumeration_uniformity.py`
already proves parametrically that both helpers evaluate existence and access
before branching, so nonexistent and inaccessible come back byte-identical.
Re-asserting that would only re-test the shared dependency.

What no dependency-level test can see is whether *this* endpoint routes through
it. So the assertion is the identity of the callable actually bound to
`agent_name` on each route — `get_owned_agent_by_name` on the mutating verb,
`get_authorized_agent_by_name` on the read-only status verb — mirroring the
existing `reject_agent_principal(current_user)` getsource guard: an annotation
that merely looks right in a diff, or a hand-rolled lookup with a 404-then-403
split, is how the enumeration oracle gets reintroduced.

The two scopes are not interchangeable, so both are pinned: swapping them would
either lock a shared reader out of a surface the Git tab already shows them, or
let one rebind an agent they do not own.

Not vacuous: the two dependencies are distinct objects, so binding the wrong one
fails the assertion. Route introspection goes through `route.dependant`, not
`get_flat_dependant` — that symbol drifts in the verify venv.

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

---------

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