feat(chaos-loop): add evidence-gated resilience plugin - #32
feat(chaos-loop): add evidence-gated resilience plugin#32RenzoPrettoMS wants to merge 3 commits into
Conversation
Add the Chaos Loop controller, deterministic state engine, five bounded phase skills, schemas, packaging, tests, and Azure SRE Agent setup. Reuse the existing Chaos Studio MCP package for Azure operations. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Require a validated workspace request at start, deterministically reuse or create a compatible Chaos Studio workspace, persist its immutable evidence, and fail closed on discovery, provisioning, or RBAC errors. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Version chaos-mcp at 0.4.0 for the new public workspace listing tool. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Nikhil Kaul (nikhilkaul1234)
left a comment
There was a problem hiding this comment.
Hi Renzo — thanks for this, it's a substantial and genuinely well-built piece of work. The phase contracts, the deterministic evaluator/apply split, the immutable-workspace invariant, and the breadth of the Pester + pytest suites are all strong, and CI is green at 5adb667.
I reviewed the full diff at exact head 5adb667452095bd135da648952da9a3965c05500 and reproduced five defects locally against the shipped state tool. Each has a small, local fix. Requesting changes primarily on #1 and #3, since #1 weakens the PR's central safety claim and #3 lets the controller persist state that violates its own published schema.
Required fixes
1. Empty targetEnv silently satisfies the external merge-and-deploy gate
Where: copilot-cli-plugin/scripts/chaos_loop_state.py:3361 (validate_gate); apply_coding (~2404-2448); copilot-cli-plugin/skills/coding/SKILL.md:73
Every other gate field is checked non-empty — mergeCommit, expectedBuildId, expectedArtifact, expectedDeploymentId, expectedRevision all use require(change[field]). targetEnv alone is only compared for equality:
require(change["targetEnv"] == required["targetEnv"], f"Target environment mismatch for {change_id}")So "" == "" passes. This is the default path, not an edge case: skills/coding/SKILL.md:73 ships "targetEnv": "" in the coding output template, and apply_coding never requires the field either.
Reproduced: with targetEnv: "" in both state and the gate payload, resume returned ok=true and advanced the run to resilience-analysis, iteration=1.
Impact: a fix can clear the hard gate without ever being bound to a named environment, which is the one thing the gate exists to prove.
Fix: add require(change["targetEnv"], f"targetEnv is missing for {change_id}") in validate_gate alongside the other non-empty checks, and require(change.get("targetEnv"), ...) in apply_coding so it can't enter state empty. Please add a Pester case with an empty targetEnv — the five existing cases in ChaosLoopState.Tests.ps1 (1037, 1121, 1160, 1202, 1208) all use "staging", so this path is currently untested.
2. Absent targetEnv raises a bare KeyError and permanently strands the run
Where: same line, chaos_loop_state.py:3361
If targetEnv is missing from the implemented change rather than empty, the lookup raises KeyError: 'targetEnv'. From awaiting-external-gate the only available command is resume, which now always fails; terminate-analysis-only is restricted to resilience-analysis. The run cannot be advanced or closed.
Reproduced: resume → errorType: KeyError, run stuck in awaiting-external-gate.
Fix: the Finding 1 change converts this into a structured ContractError with the actionable message. Separately, I'd suggest an explicit operator escape (an abandon/terminate path usable from any non-terminal phase) so a malformed payload can never leave a run with no legal next command.
3. approve accepts duplicate advisory IDs, producing schema-invalid state and a coding deadlock
Where: chaos_loop_state.py:3307 (cmd_approve); schemas/chaos-loop/run-state.v1.schema.json:110-114
cmd_approve splits on comma and assigns directly. The subset check passes for duplicates, so --advisory-ids 'adv-1,adv-1' is accepted and persists approvedAdvisoryIds: ["adv-1", "adv-1"] — which violates this PR's own published contract:
"approvedAdvisoryIds": { "type": "array", "items": {...}, "uniqueItems": true }The run then advances to coding, where apply_coding is mathematically unsatisfiable: line 2446 requires sorted(covered) == sorted(approvedAdvisoryIds) (so covered must contain the duplicate), while line 2449 requires covered to be unique.
Reproduced: approval accepted and phase advanced; all three possible coverage strategies (one change, two changes, implemented+notImplemented) rejected. Run permanently stuck in coding.
Fix: one line in cmd_approve, before assignment:
require(len(advisory_ids) == len(set(advisory_ids)), "Approved advisory IDs must be unique")4. Stale state lock is unrecoverable and undocumented
Where: chaos_loop_state.py:174-195 (state_lock), CLI parser 3487-3567
The lock is O_CREAT|O_EXCL with no PID liveness check and no timeout. It writes pid and a timestamp into the lock file but never reads them back, and there is no unlock subcommand.
Reproduced: with a stale state.json.lock, both migrate and workspace-fail fail with State is locked by another controller. Only read-only status still works.
Impact: a controller lost to SIGKILL, container eviction, or host restart bricks the run. Recovery requires manually deleting a lock file, which isn't mentioned in docs/chaos-loop.md or any SKILL.md.
Fix: on FileExistsError, read back the recorded PID and break the lock if that process no longer exists — or add an explicit unlock subcommand and document the recovery step.
5. Case-sensitive user-assigned identity lookup can leave role assignment silently empty
Where: copilot-cli-plugin/mcp/chaos_mcp/server.py:157-165
workspace_identity.get("userAssignedIdentities", {})
.get(user_assigned_identity_resource_id, {})
.get("principalId")ARM commonly echoes resource IDs with different casing (/resourcegroups/ vs /resourceGroups/). On any casing drift, principal_id is None, rbac short-circuits to [], and the tool still returns ok: true with a created workspace and no Reader grants.
Reproduced: targeted pytest with only the resourceGroups → resourcegroups difference returns ok=true, roleAssignments=[].
The Chaos Loop itself is protected here — verify_workspace_readback requires assigned_scopes == expected_scopes and escalates. The exposure is the standalone startchaos / SRE Agent path, where this surfaces later as confusing permission failures during fault execution.
Fix: case-insensitive lookup, e.g.
identities = {k.casefold(): v for k, v in (workspace_identity.get("userAssignedIdentities") or {}).items()}
principal_id = (identities.get(user_assigned_identity_resource_id.casefold()) or {}).get("principalId")test_create_workspace_user_assigned_identity_grants_reader (test_workspace_tools.py:172-214) uses a byte-identical key, so it can't catch this — worth a casing-drift variant.
Blocking: #31 / #32 merge sequence
This PR and #31 both bump chaos-mcp and the plugin to exactly 0.4.0. git merge-tree shows four conflicting files:
copilot-cli-plugin/CHANGELOG.mdcopilot-cli-plugin/mcp/chaos_mcp/azure.pycopilot-cli-plugin/mcp/tests/test_monitor_tools.pycopilot-cli-plugin/plugin.json
The real risk is what doesn't conflict: mcp/chaos_mcp/__init__.py and mcp/pyproject.toml auto-merge cleanly to 0.4.0, so the second merge would silently ship two independent feature sets under one version. release.yml publishes to PyPI on v* tags and PyPI versions are immutable.
Proposal: whichever PR merges first keeps 0.4.0. The second rebases onto main, resolves those four conflicts, ensures the CHANGELOG and tool-count assertions cover both feature sets, and bumps package + plugin (and marketplace.json) to 0.5.0. Happy to take them in whichever order you prefer — just let me know so the second one can be rebased.
Non-blocking questions
These are contract confirmations, not defects — no changes needed unless you think otherwise:
- Workspace-create autonomy. Creating the workspace plus Reader role assignments is a real Azure write executed autonomously during
start, gated only by prose inskills/chaos-loop/SKILL.md:114-115("keep tool approval enabled") rather than by the state machine's own gates (advisory selection, external gate). Is that the intended autonomy boundary? - Terminal-on-readback-mismatch.
fail_workspaceescalates the whole run with no retry, so ARM eventual consistency on a reuse readback would end a run. Intentional, or would a bounded retry be worth it? - Release cut. All the
0.4.0content sits under## [Unreleased]whileplugin.json,marketplace.json,pyproject.toml, and__init__.pyall declare0.4.0; prior releases use dated headings (## [0.3.0] — 2026-05-29). Should this become a dated0.4.0heading in this PR? - Package version.
package/chaos-loop/plugin.jsonis1.0.0while the host plugin is0.4.0. This reads as deliberate given the standalone SRE Agent bundle — just confirming. - Identity-less workspaces.
normalize_workspace_resourcerequires anidentityobject, so a workspace whose ARM payload omits it can never be reused. Likely intended, but it surfaces as a generic normalization error rather than a purposeful "not reusable because…" reason.
Also two cosmetics, entirely optional: duplicate step number 7 in skills/chaos-loop/SKILL.md (lines 207 and 215), and cli_main/main don't catch OSError, so a disk/permission error surfaces as a traceback rather than the {ok: false} envelope agent callers expect.
Once #1-#5 are in and we've settled the merge order, I'm happy to re-review promptly. Nice work on this.
Summary
Adds Chaos Loop to the native
startchaosplugin: one public controller, five bounded internal phase skills, durable repository state, and a deterministic evaluate/apply engine for evidence-gated resilience remediation.Type of change
Architecture
The controller auto-advances the prescribed sequence:
resilience-analysis -> chaos-execution -> diagnostic -> advisory -> coding -> reassessment -> identical chaos verification -> diagnostic verificationPhase skills produce semantic proposals and decisive handoffs.
chaos_loop_state.pyexclusively owns state revisions, schema/policy migration, validation, calculations, Scenario catalog eligibility, stable ranking, routing, verdict eligibility, advisory ledger diffs, approval coverage, external-gate checks, and package invariants. The phases reuse the repository's existingchaos-studioMCP server for Scenario and Azure Monitor operations; no overlapping Azure wrapper is vendored.Customer gates
There are exactly two normal customer pauses:
Validation
Invoke-Pester ./copilot-cli-plugin/skills— 120 passedpython -m pytest -qfromcopilot-cli-plugin/mcp— 36 passedstartchaosplugin load viacopilot --plugin-dirPackage and Azure SRE Agent import
Build the standalone bundle with:
The output is
tmp/chaos-loop-package/chaos-loop-1.0.0.zip. Import/setup guidance is incopilot-cli-plugin/docs/sre-agent-chaos-loop-import.md.Manual portal work remains: create the
chaos_loop_statePython tool, add the bundledchaos-studioMCP connector, attach managed identity and least-privilege RBAC, configure tool approvals and Advisor/repository/build connectors, add the controller plus five skills, configure persistent repository storage, and smoke-test read-only discovery/state creation before enabling execution.Checklist
plugin.jsonversion bumped for user-visible behaviorCHANGELOG.mdentry added under UnreleasedRelated issues
N/A