-
Notifications
You must be signed in to change notification settings - Fork 0
Red-team architecture review #110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
99808a2
Add red-team architecture review
claude 9cd7cd6
Correct line-number citations in architecture review
claude 221ccdf
Merge branch 'master' into claude/repo-architecture-review-12ukT
groupthinking c3ab305
Merge branch 'master' into claude/repo-architecture-review-12ukT
groupthinking File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,264 @@ | ||
| # Red-Team Architecture Review: `self-correcting-executor` | ||
|
|
||
| > Objective adversarial review. The repository is treated as evidence, not as the | ||
| > source of truth. Findings are grounded in direct inspection of the code on | ||
| > `master` as of this review; file paths and line references are included so every | ||
| > claim is verifiable. | ||
|
|
||
| --- | ||
|
|
||
| ## Executive Summary | ||
|
|
||
| This repository presents itself as an "autonomous self-correcting MCP executor" | ||
| with a quantum-accelerated enterprise platform layered on top. The reality, as | ||
| established from the code itself, is that there is no coherent system — there is a | ||
| pile of disconnected prototypes, vendored repositories, business pitch decks, and | ||
| AI-generated scaffolding, and the load-bearing core does not even compile. | ||
|
|
||
| Three findings collapse the project's central claims: | ||
|
|
||
| 1. **The core does not run.** `agents/executor.py` and `agents/mutator.py` — the | ||
| two modules `main.py` imports (lines 4-5) — contain **unresolved Git | ||
| merge-conflict markers committed to `master`** (`agents/executor.py:11-15`, | ||
| `agents/mutator.py:15-19,116-120`). `python -c "import ast; ast.parse(open('agents/executor.py').read())"` | ||
| fails with a `SyntaxError`. The advertised entry point cannot be imported. | ||
|
|
||
| 2. **The "self-correction" is theater.** A "protocol" is a coin flip | ||
| (`protocols/loader.py` auto-generates `bool(randint(0,1))`). The mutator's idea | ||
| of "learning from failure" is to overwrite the protocol's source with a | ||
| hardcoded `if random() < 0.8` (`agents/mutator.py:70-80`) — it does not analyze, | ||
| fix, or improve anything; it rigs a random-number generator to report success | ||
| more often. It has **already destroyed its own test fixture**: | ||
| `protocols/always_fails.py`, a protocol whose entire purpose is to fail, now | ||
| contains the generic 80%-success stub with the header | ||
| `# Previous failure rate: 100.00%`. Six distinct protocols have been homogenized | ||
| into the identical mutated coin flip. | ||
|
|
||
| 3. **The frontend talks to a backend that does not exist.** `frontend/` (a real | ||
| React/Vite app) calls `http://localhost:8080/api/v1/stats` and `/api/v2/intent` | ||
| (`frontend/.../Dashboard.tsx:47`, `IntentExecutor.tsx:40`). Grep finds **no such | ||
| endpoints anywhere in the Python code.** The only FastAPI apps live in | ||
| `agents/a2a_mcp_integration.py`, a specialized agent, and a *vendored template* | ||
| (`mcp_runtime_template_hg/api/app.py`). The UI is wired to a phantom API. | ||
|
|
||
| The repo is best understood as **three orphaned subsystems** with no call path | ||
| between them, surrounded by residue: | ||
|
|
||
| - **Working core (~200 LOC):** `main.py → executor → mutator → tracker`. The only | ||
| live system — and the part that does not compile. | ||
| - **MCP layer (~800 LOC):** *two* competing servers (`mcp_server/main.py` hand-rolls | ||
| JSON-RPC; `mcp_server/real_mcp_server.py` uses official FastMCP) plus a quantum | ||
| stub. None is called by the core loop. | ||
| - **Orchestration layer (~422 LOC):** `orchestrator.py` is never instantiated, never | ||
| imported, never run. Dead code wearing a six-layer architecture diagram. | ||
|
|
||
| Proportion: roughly **~200 lines of real (broken) system to ~3,000+ lines of | ||
| orphaned scaffolding.** Treat this repo as evidence of a *process* — rapid, | ||
| AI-assisted, trial-and-error that absorbed external repos and never cleaned up — | ||
| not as a product. | ||
|
|
||
| --- | ||
|
|
||
| ## Core Architectural Assumptions (and why they are fragile) | ||
|
|
||
| | Layer | Implicit assumption | Reality / fragility | | ||
| |---|---|---| | ||
| | **Product** | One product: a self-improving execution engine | At least **three competing visions** coexist — "Self-Correcting Executor v2.0," "Hyperion Nexus Quantum-MCP Fusion OS" (`hyperion_nexus_..._prd.md`), and "Quantum Agent Networks" (`Quantum Agent Networks_....md`). The latter two are pitch decks, not systems. The repo serves a *narrative*, not a spec. | | ||
| | **Self-correction** | A feedback loop that improves behavior | The loop optimizes a **synthetic random success metric it controls**. "Mutation" = rewriting code to a higher hardcoded RNG threshold (`agents/mutator.py:70-80`). It cannot improve real work because protocols mostly do not do real work. | | ||
| | **Data model** | Protocols are units of meaningful work | Protocols are mostly `random()` stubs. A few (`protocols/database_health_check.py`, `api_health_checker.py`, `data_processor.py`) are real, so there is **no consistent protocol contract** — some return live results, some flip coins. | | ||
| | **State management** | A persistence layer exists | **Three competing state implementations**: `utils/tracker.py` (file/JSON), `utils/db_tracker.py` (Postgres), `utils/protocol_registry.py` (a third concept). The codebase picks between them with `try/except ImportError` at every call site — and *those very blocks are where the conflict markers live*. State is non-deterministic by construction. | | ||
| | **Backend** | A single API server on :8080 | No unified server. Multiple FastAPI fragments, `mcp_server/`, `quantum_mcp_server/`, and vendored `mcp_runtime_template_hg/`. `entrypoint.sh` launches four different "modes" pointing at different mains. | | ||
| | **Frontend** | One UI | **`frontend/` and `ui/` are two unrelated attempts.** `frontend/` is a real React app wired to a phantom API; `ui/` is an HTML glassmorphism mockup plus an uncompiled Next.js "Build a Website Guide" tutorial with mock data. | | ||
| | **Deployment** | Docker Compose brings it up | **Two conflicting compose files.** `docker-compose.quantum.yml` builds `./frontend/Dockerfile`, **which does not exist**. The standard compose has no frontend at all. Frontend hardcodes `:8080`; quantum compose injects `:8000`. Neither stack is buildable end-to-end. | | ||
| | **Secrets/config** | `.env.example` is the template | **`.env` is committed and tracked**, containing KEY/SECRET/TOKEN/PASSWORD entries. A live secret leak in version control. Separately, `config/mcp_config.py` (with a `check_no_mocks()` validator) is never loaded by `main.py` — the config meant to enforce "no mocks" is itself unwired. | | ||
| | **Argument handling** | CLI args are protocols | `python main.py --development` caused the loader to **auto-create `protocols/--development.py`** — a garbage file committed to the repo. The entry point manufactures junk as a side effect of broken arg parsing. | | ||
|
|
||
| --- | ||
|
|
||
| ## Success Criteria (independent of the current implementation) | ||
|
|
||
| Stripping away the mockups, here is what a system of this *stated intent* must | ||
| accomplish to be structurally sound. The current repo meets almost none of them. | ||
|
|
||
| **Must accomplish (product-level):** | ||
|
|
||
| 1. Accept a unit of real work (a "protocol"/task) with a **typed, enforced | ||
| contract** (inputs, outputs, success definition) — not an untyped dict from a | ||
| coin flip. | ||
| 2. Execute that work against **real external effects** (DB, API, filesystem, MCP | ||
| tool) and capture a **truthful** outcome. | ||
| 3. Persist outcomes to **one** durable, queryable store and compute honest | ||
| success/failure statistics. | ||
| 4. Detect degradation and apply a **correction that demonstrably changes real | ||
| behavior** — verified by re-execution against the same contract, not by editing | ||
| the success threshold. | ||
| 5. Expose this via a **single, documented API**, optionally surfaced by a UI. | ||
|
|
||
| **Minimum technical capabilities for "structurally sound":** | ||
|
|
||
| - The entry point **imports and runs** (currently fails). | ||
| - One canonical execution path, one state store, one server, one UI. | ||
| - A protocol abstraction that cannot "succeed" without doing the thing it claims. | ||
| - Self-correction that is **falsifiable** — measured against an external ground | ||
| truth the system cannot rig. | ||
| - No secrets in VCS; config injected at runtime; reproducible build. | ||
|
|
||
| **Critical distinction:** the product must *correct real failures*. The current | ||
| repo *manufactures fake failures and fakes correcting them*. That is not a smaller | ||
| version of the goal — it is the opposite of it. | ||
|
|
||
| --- | ||
|
|
||
| ## Separation-of-Concerns Analysis | ||
|
|
||
| Boundaries are absent or inverted: | ||
|
|
||
| - **Business logic ↔ persistence:** Fused. `executor.py`/`mutator.py` choose their | ||
| storage backend inline via `try/except ImportError`. No repository interface. You | ||
| cannot test execution without exercising backend selection. | ||
| - **Execution ↔ code generation:** Violated catastrophically. The mutator | ||
| **rewrites source files on disk** (`agents/mutator.py:85-94`) as a runtime | ||
| behavior, with no sandbox, review, or test gate, and commits the damage into the | ||
| working tree. Runtime mutates its own repository. | ||
| - **Orchestration ↔ execution:** Two disconnected brains. `main.py` drives a simple | ||
| `execute → mutate` loop; `orchestrator.py` is a separate intent-based | ||
| `OrchestrationEngine` that is never instantiated. Parallel universes with no | ||
| bridge. | ||
| - **UI ↔ API:** No contract. The frontend invents endpoints the backend never | ||
| implements. No shared schema, no generated client. | ||
| - **Auth / middleware:** Directories exist (`auth/basic_auth.py`, | ||
| `middleware/security_middleware.py`) but are not wired into any request path the | ||
| frontend actually hits. | ||
| - **External services (MCP, quantum, LLM):** Real, sophisticated connectors | ||
| (`connectors/dwave_quantum_connector.py` 21KB, `github_mcp_connector.py` 27KB, | ||
| `llm_connector.py`) exist with **zero call paths from any live entry point** — | ||
| only tests and stub deploy scripts touch them. | ||
|
|
||
| Net effect: the system is **untestable** (core will not import), **undebuggable** | ||
| (state backend nondeterministic), **unscalable** (runtime edits its own source), | ||
| and **unextendable** (no stable contracts to extend against). | ||
|
|
||
| --- | ||
|
|
||
| ## Major Technical Debt & Structural Risks | ||
|
|
||
| **Architectural debt (not just code smells):** | ||
|
|
||
| - **Broken core committed to `master`** — conflict markers in the two most important | ||
| files. Highest severity: nothing downstream of `main.py` can run. | ||
| - **Self-modifying-source as a feature** — the mutation mechanism is an | ||
| architectural dead end and a safety hazard; it corrupts the repo and erases test | ||
| signal (`always_fails.py` no longer fails). | ||
| - **Triple state implementations** selected by exception handling — guarantees | ||
| inconsistent behavior across environments. | ||
| - **Two UIs, two compose files, two MCP servers, multiple entry points** — every | ||
| major axis has competing implementations and no canonical choice. | ||
| - **Vendored/absorbed repos** (`ported/wrkflw`, `ported/workflows-api`, | ||
| `mcp_runtime_template_hg/`) merged in wholesale (commit "absorb WRKFLW + | ||
| workflows repos #107") — foreign architectures glued on, not integrated. | ||
| - **Deploy scripts import packages that are not installed** (`mcp-use` in | ||
| `deploy_production_mcp.py`) — cannot run against their own repo. | ||
| - **Secret leak** in committed `.env`. | ||
|
|
||
| --- | ||
|
|
||
| ## What to Discard, Preserve, or Reconsider | ||
|
|
||
| ### Kill list (experimental residue — delete) | ||
|
|
||
| | Path / pattern | Why | | ||
| |---|---| | ||
| | `resolve_*conflicts.py`, `fix_formatting.py`, `fix_python_files.py`, `fix_remaining_conflicts.py`, `resolve_merge_conflicts.py`, `resolve_formatting_conflicts.py` | One-off merge/lint fixer scripts. Throwaway. | | ||
| | `repomix-output.xml` (970KB) | Full-codebase tool dump. Not source. | | ||
| | `cursor_installing_docker_and_integratin.md` (158KB), `Module 3 Citations and Additional Resources.html` | Tutorial/copy-paste artifacts. | | ||
| | `hyperion_nexus_..._prd.md`, `Quantum Agent Networks_....md`, `DEEPGIT_*.md`, all `*_SUMMARY.md`, `PROJECT_ANALYSIS_REPORT.md`, `json_serialization_bug_fix_summary.md`, `GIT_PUSH_SUMMARY.md` | Pitch decks and post-mortems of past iterations. ~40 markdown files of narrative residue. | | ||
| | `arc_agent_prototype_v2.py`, `simple_quantum_example.py`, `mcp_promise.js`, `guardian_linter_watchdog.py` | Loose prototypes with no integration. | | ||
| | `quantum_mcp_server/`, `Dockerfile.quantum`, `docker-compose.quantum.yml`, D-Wave deps | Pitch-driven; quantum is mostly mocked. No real implementation behind the claim. | | ||
| | `ui/` | Mockup + uncompiled Next.js tutorial. Superseded by `frontend/`. | | ||
| | `ported/`, `mcp_runtime_template_hg/` | Wholesale-absorbed external repos; defer until a real integration decision. | | ||
| | Mutator-corrupted protocols (`always_fails.py`, `default_protocol.py`, `--development.py`, and the other homogenized stubs) | Destroyed by the self-mutation loop; no longer meaningful. | | ||
| | Root `test_mcp_*.py`, `test_real_dwave_quantum.py` | Manual/exploratory; belong in `tests/` if kept at all. | | ||
| | `deploy_production_mcp.py`, `deploy_mcp_llm_integration.py` | Import non-existent packages; scaffolding. | | ||
|
|
||
| ### Preserve (as ingredients, not as-is) | ||
|
|
||
| | Path | Why | | ||
| |---|---| | ||
| | `frontend/` (React/Vite app) | Real, reusable once pointed at a real API and given a generated client. | | ||
| | Protocol-as-task concept (`protocols/loader.py`) | Sound abstraction; needs a typed, falsifiable contract. | | ||
| | Real protocols: `database_health_check.py`, `api_health_checker.py`, `data_processor.py` | Actually do work; good seed examples. | | ||
| | `utils/db_tracker.py` | Postgres-backed outcome store — the *one* state store to keep. | | ||
| | A single FastAPI server pattern | Exists in fragments; consolidate into one. | | ||
|
|
||
| ### Reconsider | ||
|
|
||
| - **Rotate the secrets in the committed `.env` immediately**, then remove it from | ||
| VCS and rely on `.env.example` + runtime injection. | ||
| - **Redefine "self-correcting."** If it means "detect a failing integration and | ||
| retry/reroute/alert," that is achievable and valuable. If it means "rewrite my own | ||
| code to pass," that is the current dead end — drop it. | ||
|
|
||
| --- | ||
|
|
||
| ## Clean Target Architecture | ||
|
|
||
| The simplest structure that satisfies the stated intent: | ||
|
|
||
| ``` | ||
| One service, one state store, one contract. | ||
|
|
||
| [React frontend] ──calls──▶ [FastAPI app] (single server, :8080) | ||
| │ | ||
| ├─ /api: thin route layer (validation only) | ||
| ├─ ExecutorService (business logic, pure) | ||
| │ └─ Protocol(Protocol ABC): run() -> Outcome | ||
| │ • typed contract; success defined by | ||
| │ real post-conditions, not RNG | ||
| ├─ OutcomeRepository (interface) | ||
| │ └─ PostgresRepository (the ONE impl) | ||
| └─ CorrectionPolicy (strategy) | ||
| • retry / reroute / circuit-break / alert | ||
| • NEVER edits source; verified by re-run | ||
| ▲ | ||
| external effects ──┘ MCP tools / HTTP / DB via one adapter layer | ||
| ``` | ||
|
|
||
| Principles: | ||
|
|
||
| - **One** entry point, **one** server, **one** state store, **one** UI, **one** | ||
| compose file. | ||
| - Protocols implement an interface with a **falsifiable success post-condition**. | ||
| - "Correction" is a **strategy applied to real outcomes**, verified by | ||
| re-execution — code is never the thing that gets mutated. | ||
| - Frontend consumes an **OpenAPI-generated client**, eliminating phantom-endpoint | ||
| drift. | ||
| - Quantum, A2A, and multi-agent orchestration are out of scope until the | ||
| single-node loop is real and tested. Add them behind the adapter boundary later, | ||
| if ever. | ||
|
|
||
| This discards roughly 80% of the current tree and loses **nothing the product | ||
| actually needs.** | ||
|
|
||
| --- | ||
|
|
||
| ## Final Red-Team Verdict | ||
|
|
||
| The repository is not a system; it is a sedimentary record of experiments that was | ||
| never excavated. Its headline feature — self-correction — is non-functional and | ||
| conceptually inverted: it games a metric it controls and corrupts its own source to | ||
| do so, having already erased its only honest test case. Its core does not compile. | ||
| Its UI points at an API that was never built. Its deployment story is two | ||
| incompatible drafts, neither buildable. Its real intent is buried under pitch-deck | ||
| markdown for a quantum platform that exists only as prose. | ||
|
|
||
| **Do not refactor this. Do not reconcile it.** A repair plan ("keep the core, pick | ||
| one MCP server, bridge them, fix the conflicts") dignifies trial-and-error residue | ||
| as architecture. You do not bridge a coin-flip loop — whose self-correction | ||
| mechanism corrupts its own source — to a phantom-API frontend. | ||
|
|
||
| Extract the three or four genuine ingredients (the React shell, the protocol | ||
| concept, the real health-check protocols, the Postgres tracker), define a | ||
| falsifiable success contract, and rebuild the single-node loop clean. | ||
|
|
||
| **Severity: Critical. Recommended action: Rebuild, do not repair. First, rotate the | ||
| committed secrets.** | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.