Repository navigation
Phase 4 review: fail-closed release check, docs and 0.4.0 release prep - #41
Conversation
core.compare_runs duplicated lens.compare_runs line for line. Both now run through run_comparison, recorded under their own tool name. The adapter also validates a comparison with the Lens ComparisonReport model and lists the Evals GateConfig threshold names, for the unified release check.
The Phase 4 release check reported ready=true for an empty chaos report, a failed chaos or comparison call, and a 10% evaluation pass rate, and silently ignored every threshold except min_pass_rate. - Evaluation evidence goes through the Evals release gate (schema validation plus every GateConfig threshold, strict by default) via the same adapter as lens.slo_summary. Unknown threshold keys are rejected. - Comparisons are validated with the Lens ComparisonReport model; chaos evidence must be a chaos.run_experiment report. - required_evidence (default: all three kinds) makes missing evidence block readiness, matching the prompt and docs. Gates report passed, failed, invalid or missing with reasons and observed values. - incident_summary no longer reports passing evaluations or routine injected faults as incidents, adds blocked release gates and returns no_signals when nothing failed. - export_report names its generator and is documented as MCP-local. - Rename the unreleased resource to resource://workflows/release-check and describe each step, including the hosted chaos_report path.
Covers the four core tools, gate statuses, required_evidence, Evals thresholds, the incident signals, the MCP-local export envelope and local versus hosted use.
The Phase 4 docs claimed missing evidence was treated as unresolved while the tool passed it, nested the new section under the Core Diagnostics heading, and pointed the workflow diagram at chaos.export_aios_run, which never feeds the release check. Add a README section, a user-guide capability row and core.compare_runs on the landing page.
Sidecar 0.8.5 reports package_status implemented and runtime_ready true, not scaffold/false; MCP still only exposes discovery.
Phase 4 stays in progress: access-containment items are open. Note that the Tower tools ship as tower.* and that the export envelope is MCP-local.
The entry had been appended after 0.1.0 at the bottom of the file.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 421f5df2dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def _chaos_gate(report: dict[str, Any]) -> dict[str, Any]: | ||
| # A crashed or timed-out run is a real report with ok=false, so check its | ||
| # shape before treating ok=false as a failed call. | ||
| absent = [key for key in _CHAOS_REPORT_KEYS if key not in report] |
There was a problem hiding this comment.
When the other required gates pass—or required_evidence is narrowed to chaos—a caller can supply {"crashed": null, "timed_out": false, "chaos_events": []} and receive ready: true. Checking only these three keys does not establish that this came from chaos.run_experiment: it omits required producer fields such as ok, id, name, and timestamps, and the values themselves are not type-validated. Validate the producer's full report shape before marking this gate passed so malformed evidence cannot make the release check fail open.
AGENTS.md reference: AGENTS.md:L48-L50
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same issue as the Copilot chaos-shape finding; fixed in b2d4920 by validating every producer field and its type.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate issues remain in input/evidence validation, stale evidence handling, and documentation accuracy.
4 open issues
What changed in this PR
Prepares the 0.4.0 Phase 4 unified workflows with fail-closed release checks, shared evidence validation, incident/export behavior, and documentation updates.
Changes:
- Validates Lens, Evals, and Chaos evidence with explicit gate statuses.
- Shares comparison logic and applies all configured thresholds.
- Adds tests, examples, workflow resources, and release documentation.
| File | Summary |
|---|---|
tests/test_unified.py |
Adds unified workflow coverage. |
tests/test_server.py |
Updates server workflow tests. |
tests/test_registry.py |
Updates resource URI assertions. |
src/deep_agentic_core_mcp/tools/unified.py |
Implements unified gates and reporting. |
src/deep_agentic_core_mcp/tools/registry.py |
Updates tool metadata and schemas. |
src/deep_agentic_core_mcp/tools/lens.py |
Shares comparison execution logic. |
src/deep_agentic_core_mcp/server.py |
Updates workflow resource content. |
src/deep_agentic_core_mcp/resources/catalog.py |
Renames the workflow resource. |
src/deep_agentic_core_mcp/prompts/registry.py |
Updates release-check guidance. |
src/deep_agentic_core_mcp/adapters/agenticlens.py |
Adds comparison and validation helpers. |
ROADMAP.md |
Records Phase 4 progress. |
README.md |
Documents unified workflows. |
mkdocs.yml |
Adds workflow navigation. |
index.html |
Updates the tool listing. |
examples/unified_release_check.py |
Adds an end-to-end example. |
docs/user-guide.md |
Updates capability documentation. |
docs/unified-workflows.md |
Adds the workflow guide. |
docs/tools.md |
Updates tool reference documentation. |
docs/roadmap.md |
Updates the published roadmap. |
docs/integrations.md |
Corrects integration statuses. |
docs/index.md |
Adds workflow overview content. |
docs/features.md |
Marks unified workflows implemented. |
docs/examples.md |
Documents the new example. |
CHANGELOG.md |
Adds dated 0.4.0 release notes. |
AGENTS.md |
Updates roadmap guidance. |
Review effort: Lite
| | `core.compare_runs` | Lens baseline/candidate comparison; same behavior as `lens.compare_runs` | Stores runs and the comparison | | ||
| | `core.release_check` | Gates comparison, evaluation and chaos evidence; fails closed | Stores the result | | ||
| | `core.incident_summary` | Collects regressions, chaos crashes/timeouts, failed cases and blocked gates | Stores the result | | ||
| | `core.export_report` | Exports everything the session holds as `mcp.unified-report.v1` | Read-only | |
There was a problem hiding this comment.
Fixed in b2d4920: the guide now says the evidence is unchanged but the call is recorded in history. Recording the export keeps the audit trail consistent with every other tool, so I kept the behavior and corrected the docs.
| absent = [key for key in _CHAOS_REPORT_KEYS if key not in report] | ||
| if absent: | ||
| failed = _failed_call("chaos", report) | ||
| missing = f"Not a chaos.run_experiment report; missing {', '.join(absent)}" | ||
| return failed or _gate("chaos", "invalid", [missing]) |
There was a problem hiding this comment.
Fixed in b2d4920: the chaos gate now requires every chaos.run_experiment field (ok, id, name, start_time, end_time, chaos_events, crashed, timed_out) with the expected type, and returns invalid otherwise. Tests cover the three-key lookalike, a string chaos_events and a string timed_out.
| "evaluation": arguments.get("evaluation_report"), | ||
| "chaos": arguments.get("chaos_report", state.last_chaos_report), | ||
| } | ||
| return {kind: value if isinstance(value, dict) else None for kind, value in supplied.items()} |
There was a problem hiding this comment.
Fixed in b2d4920: supplied non-object evidence is now gated as invalid instead of missing (tests for chaos_report: [] and comparison: []).
- A chaos report must carry every chaos.run_experiment field with the right type (ok, id, name, timestamps, chaos_events list, crashed, timed_out); a three-key lookalike is invalid, not passing. - Falsy non-object thresholds ([], "", null) are rejected instead of silently falling back to the default gate. - Supplied non-object evidence is gated as invalid rather than reported as missing. - Document that core.export_report leaves evidence unchanged but records the call in history.
Follow-up review of #40 (MCP Phase 4 unified workflows), which merged without review.
What was wrong in #40
core.release_checkfailed open. Onmainit returnsready: truefor:{}, or a failed chaos call{"ok": false, "error": ...}It also ignored every threshold except
min_pass_rate, which re-implements a weaker version of the Evals release gate that MCP already wraps inlens.slo_summary.core.compare_runswas a copy oflens.compare_runs. The CHANGELOG entry was appended below 0.1.0, the docs claimed fail-closed behaviour, and the Sidecar status indocs/integrations.mdwas wrong.Changes
GateConfigthreshold, strict by default, unknown keys rejected). Comparisons are validated with the LensComparisonReport. Chaos evidence must be achaos.run_experimentreport.required_evidencedefaults to all three kinds. Each gate ispassed/failed/invalid/missingwith reasons and observed values.core.compare_runsshares thelens.compare_runspath.incident_summaryreports only failure signals (plus blocked gates) and returnsno_signalsotherwise.export_reportnames its generator and is documented as an MCP-local envelope, not an AIOS artifact.resource://workflows/release-check(unreleased, so this breaks nothing) and now has accurate steps.tests/test_unified.py(fail-closed matrix, Evals thresholds, invalid options, incidents, export, per-user hosted state).docs/unified-workflows.mdand exampleexamples/unified_release_check.py. README, ROADMAP, AGENTS.md, features/index/integrations/user-guide/roadmap docs, landing page and tool reference are updated, and CHANGELOG 0.4.0 is dated.Phase 4 stays in progress: the access-containment items (authenticated stop/revoke, tool-level authorization, denial evidence) are still open.
No sibling dependency range or hosted image pin changes. Tower's
deep-agentic-core-mcp>=0.3.6,<1already admits 0.4.0.