Skip to content

feat: resilient system-manifest deploy — continue-on-error + partial-success report (trinity-enterprise#125) - #1760

Merged
dolho merged 1 commit into
devfrom
feature/ent125-resilient-system-deploy
Jul 24, 2026
Merged

feat: resilient system-manifest deploy — continue-on-error + partial-success report (trinity-enterprise#125)#1760
dolho merged 1 commit into
devfrom
feature/ent125-resilient-system-deploy

Conversation

@vybe

@vybe vybe commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • POST /api/systems/deploy becomes best-effort by default: a per-agent create failure is collected into failed: [{name, short_name, template, reason, status_code}] and the remaining agents still deploy — instead of the first failure aborting the whole fleet as an opaque 500 (already-created agents left behind with no report).
  • Tri-state response contract: status = deployed (all created, HTTP 200) / partial (some failed, HTTP 200) / failed (none created, HTTP 500 with the full report as the body so curl -f-style callers don't read total failure as success); valid (dry_run) unchanged. strict: true restores abort-on-first-error, preserving the failing agent's original status code (4xx no longer flattened to 500).
  • Best-effort extends past the create loop: post-create configuration (folders / permissions / schedules / tags) is scoped to the survivor map and each phase individually degrades to a warnings entry — a config failure after successful creates can no longer void the promised report. trinity_prompt is written only when ≥1 agent was created.
  • failed[].reason is treated as a disclosure surface: dict details normalized to their error field, credential-sanitized, URL-userinfo-redacted (git errors embed PAT-bearing remote URLs — learnings 2026-07-14), truncated at 500 chars. New shared redact_url_userinfo() in utils/credential_sanitizer.py.
  • Operator guidance in warnings: partial deploys flag the _N-suffix duplicate-on-redeploy trap (converge/on_conflict deferred to trinity-enterprise#124); an orchestrator-workers deploy whose orchestrator failed flags a possibly non-functional fleet.
  • MCP deploy_system (Invariant feat: SMARTS trading pipeline with Telegram notifications and Miro visualization #13): strict param + failed[] response typing + best-effort description.

Prerequisite for trinity-enterprise#124 (first-run starter-fleet seed) and trinity-enterprise#126 (UI manifest install). Epic: trinity-enterprise#122.

Changes

  • src/backend/routers/systems.py — best-effort create loop, _failure_reason normalization chokepoint, survivor scoping, guarded config phases, post-loop prompt write, 500-with-report on total failure
  • src/backend/models.pySystemDeployFailure, SystemDeployResponse.failed, SystemDeployRequest.strict
  • src/backend/utils/credential_sanitizer.py — additive redact_url_userinfo() (mirrors agent-side bug: git auto-gc can never complete inside agent containers — unbounded, silent workspace .git bloat (273GB observed; 97%-garbage repos) #1595 helper)
  • src/mcp-server/src/tools/systems.tsstrict param, failed[] typing, description
  • tests/unit/test_ent125_resilient_system_deploy.py — 14 hermetic router tests (partial/total/strict/all-success/dry-run, survivor scoping, config-phase degradation, dict-detail normalization, PAT-URL redaction, truncation, prompt-write gating, orchestrator warning)
  • tests/test_systems.py — 3 integration tests using the deterministic pre-side-effect resources: {cpu: "3"} failure vector
  • Docs: requirements/roadmap.md §16.5, feature-flows/system-manifest.md, learnings.md (silent blank-agent pitfall), tests/registry.json

Test Plan

  • Unit: pytest tests/unit/test_ent125_resilient_system_deploy.py -v — 14/14 pass; also green combined with the sys.modules-churning neighbors (test_1081_pull_endpoints.py) in both orders
  • Adjacent guards: test_models_centralized.py (Invariant Add internal health route, without which main didn't start #14) + test_1661_sanitizer_linear.py pass (61 total)
  • Integration: pytest tests/test_systems.py::TestResilientDeploy -v against a running stack — written but not yet executed (local Docker was down during development); please run at next stack-up or via test-runner
  • MCP server type-checks (tsc --noEmit — only pre-existing unrelated yaml-types noise in pipelines.ts)

Notes for review

  • A well-formed but absent local: template silently creates a blank agent (pre-existing; discovered while designing the failure vector — it invalidated local:nonexistent as a test failure trigger). Filed as bug: absent local: template silently creates a blank agent (no error) #1759 with the evidence; matters for the ent#124 seed.
  • Cross-repo close: Refs not Fixes — the private tracker issue (trinity-enterprise#125) is closed manually at release per working agreement.

Refs abilityai/trinity-enterprise#125

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

@dolho

dolho commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

/review report — feature/ent125-resilient-system-deploydev

Files: 10 (+788/−109) · Merge-base: 49b235c7 · Scope: CLEAN · Tests: 14 new unit + 33 systems

Critical findings: 0

Verified clean (evidence, not assumption)

  • Auth unchangeddeploy_system still gated by require_role("creator"); nothing in the diff widens who can deploy
  • Credential exposure on the new egress surface — this is the part I looked hardest at, because the PR turns internal exceptions into a client-visible failed[].reason. It's handled: _failure_reason runs sanitize_text(redact_url_userinfo(reason)) and caps at 500 chars, and the PR adds redact_url_userinfo to the shared sanitizer specifically because git prints full remote URLs (PAT included) in common error lines. Recognising that a new surface for an existing payload inherits the payload's credential hazard — but not its redaction — is exactly the right instinct; that class bites easily.
  • Status-code semantics — total failure returns JSONResponse(status_code=500, content=report), so the body is the report rather than an HTTPException wrapper; strict: true re-raises with status_code or 500, preserving the failing agent's original 4xx instead of flattening it
  • Enum completeness (the category that needs code outside the diff) — the new tri-state has one other consumer, the MCP deploy_system tool, and it was updated: the description spells out 'deployed' | 'partial' | 'failed' and the response type includes failed[]. No frontend caller of /systems/deploy exists, so there is no third consumer silently reading partial as success.

🟡 Informational

I1 — partial returns HTTP 200, which naive callers will read as success (Confidence 8/10)
This is a deliberate, documented contract ("callers must check status, not just the HTTP code"), and the MCP tool description carries it. But the failure mode is silent: any existing curl -f, CI step, or client that branches on the status code alone now treats a half-deployed fleet as a clean deploy. 207 Multi-Status exists for precisely this and would make the partial case impossible to miss without changing the body. If 200 is intentional for client-compat, it's worth an explicit line in the feature-flow doc saying why 207 was rejected, so the next reader doesn't "fix" it.

I2 — reason truncation is a hard 500-char slice (Confidence 6/10)
reason[:_REASON_MAX_LEN] is applied after sanitisation, which is the correct order (sanitise-then-truncate — the reverse can cut a secret so the pattern no longer matches). Worth a brief comment saying so, since the ordering is load-bearing and a future refactor could innocently swap it.

Summary

Critical: 0 · Informational: 2 · Scope: clean

The riskiest part of this change is that failure text now reaches API clients, and that is genuinely handled — sanitised, userinfo-redacted, and length-capped in the right order.

…success report (trinity-enterprise#125)

POST /api/systems/deploy is best-effort by default: a per-agent create
failure is collected into failed[] ({name, short_name, template, reason,
status_code}) and the remaining agents still deploy, instead of the first
failure aborting the whole fleet as an opaque 500.

- Tri-state status: "deployed" (all created, 200) / "partial" (some
  failed, 200) / "failed" (none created, 500 with the full report body so
  code-only callers don't read total failure as success); "valid" (dry_run)
  unchanged.
- strict: true restores abort-on-first-error, preserving the failing
  agent's ORIGINAL status code (4xx no longer flattened to 500).
- Post-create config (folders/permissions/schedules/tags) scoped to the
  survivor map, and each phase individually degrades to a warning — a
  config failure after successful creates can no longer void the report.
- trinity_prompt write moved post-loop, gated on >=1 created agent, so a
  totally-failed deploy never mutates the platform-wide prompt.
- failed[].reason normalized (dict detail -> error field), credential-
  sanitized + URL-userinfo-redacted (git errors embed PAT-bearing remote
  URLs, learnings 2026-07-14) + truncated; new shared
  redact_url_userinfo() in utils/credential_sanitizer.
- Warnings: partial deploys flag the _N-suffix duplicate-on-redeploy trap
  (converge deferred to trinity-enterprise#124); orchestrator-workers
  preset with a failed orchestrator flags a possibly non-functional fleet.
- MCP deploy_system: strict param + failed[] response typing (Invariant #13).
- Tests: 14 hermetic unit tests (router mounted alone, collaborators
  patched at the module binding) + 3 integration tests using the
  deterministic pre-side-effect cpu:"3" failure vector.

Follow-up filed: #1759 (absent local: template silently creates a blank
agent — discovered during review).

Prerequisite for trinity-enterprise#124 (first-run seed) and
trinity-enterprise#126 (UI manifest install).

Refs Abilityai/trinity-enterprise#125

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dolho
dolho force-pushed the feature/ent125-resilient-system-deploy branch from b7db148 to 80f29bc Compare July 24, 2026 10:40
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