feat(supervisor): add retry policy tests (F58)

[wendeus0]

Summary

• F58 (Retry Policy Tests): 6 new tests for src/synapse_os/supervisor.py covering retry budget exhaustion, reroute fallback, terminal failures, and review rejection handling.

Technical Changes

New Tests Added

• test_supervisor_marks_terminal_failure_after_security_error — SECURITY state is terminal
• test_supervisor_terminal_failure_when_no_fallback_route — fail when no fallback available
• test_supervisor_retry_budget_exhausted_at_max_retries — retry at exact budget limit
• test_supervisor_reroute_when_budget_exceeded_with_fallback — reroute with fallback
• test_supervisor_ignores_retryable_error_in_non_retryable_state — non-retryable state → fail
• test_supervisor_decision_contains_correct_reason — reason field correctness

How to Test

uv run pytest tests/unit/test_supervisor.py -v  # 10 tests
uv run pytest tests/ -v --tb=short              # 566 tests total

---

Summary by cubic

Adds F58 supervisor retry policy tests and completes the F59–F68 sprint: multi‑agent routing, a local HTTP control plane, DAG execution, a Copilot adapter, memory indexing, plugins, workspace v2, and reporting/runtime/supervisor hardening.

• New Features

  • F58: 6 tests for terminal states, fallback/reroute, retry budgets, and review rejection; SPEC validates.
  • Core: multi_agent.py (adapter registry + capability router), control_plane/ (FastAPI API; adds fastapi/uvicorn/httpx), pipeline_dag.py (DAG executor with linear fallback).
  • Extensions: Copilot adapter with circuit‑breaker classification, indexed memory store, plugin registry/hooks, workspace v2, richer reporting, and advanced supervisor/runtime policies.

• Bug Fixes

  • Control plane: fix cancel signal race; auth middleware guards non‑HTTP scopes; remove unused field from RunCreateRequest.
  • DAG: validation on executor construction; stronger cycle checks.
  • Plugins/adapters/supervisor: filter disabled plugins; stop hook re‑registration leaks; handle AdapterOperationalError in Copilot and align supervisor error shape.
  • Workspace: fix pool acquire/release to reliably reuse idle workspaces.
  • CLI/docs/specs: improve CLI error handling/env var parsing; correct SPEC frontmatter for F60–F68; update ADR‑014 and SDD; correct security audit classification.

^{Written for commit 09b8134. Summary will update on new commits.}

[cubic-dev-ai]

2 issues found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="features/F58-retry-policy-tests/SPEC.md">

<violation number="1" location="features/F58-retry-policy-tests/SPEC.md:2">
P1: Front matter is missing required SPEC metadata fields (`id`, `type`, `summary`, `inputs`, `outputs`, `acceptance_criteria`, `non_goals`), so this SPEC will fail validation.</violation>

<violation number="2" location="features/F58-retry-policy-tests/SPEC.md:11">
P1: Required SPEC sections must be H1 `# Contexto` and `# Objetivo`; using `## Objetivo` and omitting `# Contexto` makes this file fail section validation.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.}

[cubic-dev-ai]

40 issues found across 53 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/synapse_os/workspace.py">

<violation number="1" location="src/synapse_os/workspace.py:71">
P1: `acquire()` never reuses idle workspaces and can allocate beyond `max_size` when `idle_workspaces` is non-empty.</violation>

<violation number="2" location="src/synapse_os/workspace.py:86">
P1: `discard()` decrements `acquired_count` even when discarding an idle workspace, which can corrupt counters (including negative values).</violation>
</file>

<file name="features/F61-dag-pipeline-evolution/SPEC.md">

<violation number="1" location="features/F61-dag-pipeline-evolution/SPEC.md:2">
P1: The SPEC front matter does not match the required metadata schema, so validation will fail before planning/execution.</violation>

<violation number="2" location="features/F61-dag-pipeline-evolution/SPEC.md:10">
P1: The SPEC body is missing required H1 sections `# Contexto` and `# Objetivo`, so it will fail SPEC validation.</violation>
</file>

<file name="features/F60-local-control-plane-foundation/SPEC.md">

<violation number="1" location="features/F60-local-control-plane-foundation/SPEC.md:2">
P1: SPEC frontmatter does not follow the required metadata schema, so this document is likely to fail SPEC validation.</violation>

<violation number="2" location="features/F60-local-control-plane-foundation/SPEC.md:9">
P1: SPEC body headings do not match the required parser format (`# Contexto` and `# Objetivo` as H1), which can make the document unreadable by the SPEC parser.</violation>

<violation number="3" location="features/F60-local-control-plane-foundation/SPEC.md:100">
P2: Use `SYNAPSE_OS_`-prefixed environment variable names to stay consistent with `AppSettings` conventions.</violation>
</file>

<file name="features/F63-memory-engine-enhancement/SPEC.md">

<violation number="1" location="features/F63-memory-engine-enhancement/SPEC.md:2">
P1: Frontmatter does not include the required SPEC metadata keys, so this spec can fail validation.</violation>

<violation number="2" location="features/F63-memory-engine-enhancement/SPEC.md:12">
P1: SPEC body is missing required `# Contexto` and `# Objetivo` H1 sections; current `##` headings are ignored by the parser.</violation>

<violation number="3" location="features/F63-memory-engine-enhancement/SPEC.md:62">
P2: Acceptance criterion #6 references `F59` instead of `F63`, creating a feature-ID mismatch in the spec.</violation>
</file>

<file name="src/synapse_os/pipeline_dag.py">

<violation number="1" location="src/synapse_os/pipeline_dag.py:126">
P2: Avoid tight polling here; this loop spins continuously while waiting for running futures and can cause high CPU usage.</violation>

<violation number="2" location="src/synapse_os/pipeline_dag.py:150">
P1: Validate the DAG spec when constructing the executor; otherwise invalid/cyclic DAGs can exit silently without raising any error.</violation>
</file>

<file name="src/synapse_os/control_plane/server.py">

<violation number="1" location="src/synapse_os/control_plane/server.py:155">
P1: Cancellation finalizes locked runs immediately instead of only signaling cancellation, which can race with an in-flight worker and corrupt run state transitions.</violation>

<violation number="2" location="src/synapse_os/control_plane/server.py:239">
P2: Spec files are written with default filesystem permissions, which can expose prompt contents to other local users.</violation>
</file>

<file name="features/F67-workspace-management-v2/SPEC.md">

<violation number="1" location="features/F67-workspace-management-v2/SPEC.md:29">
P1: The SPEC is missing the required `# Objetivo` H1 section, which can cause spec validation to fail.</violation>
</file>

<file name="features/F68-plugin-extension-system/SPEC.md">

<violation number="1" location="features/F68-plugin-extension-system/SPEC.md:8">
P1: `inputs` and `outputs` cannot be empty; this SPEC will fail validation.</violation>

<violation number="2" location="features/F68-plugin-extension-system/SPEC.md:25">
P1: Add the required `# Objetivo` H1 section; without it, SPEC validation fails.</violation>
</file>

<file name="features/F62-copilot-adapter/SPEC.md">

<violation number="1" location="features/F62-copilot-adapter/SPEC.md:2">
P1: SPEC frontmatter does not match the required schema (`id`, `type`, `summary`, `inputs`, `outputs`, `acceptance_criteria`, `non_goals`), which can break spec validation.</violation>

<violation number="2" location="features/F62-copilot-adapter/SPEC.md:12">
P2: SPEC body is missing required `# Contexto` and `# Objetivo` H1 sections; using only `##` sections can cause the parser to ignore required content.</violation>
</file>

<file name="features/F65-runtime-coordinator-hardening/SPEC.md">

<violation number="1" location="features/F65-runtime-coordinator-hardening/SPEC.md:30">
P1: Use the required `# Objetivo` H1 section; `# Decisão` here leaves the SPEC without a mandatory section expected by the validator.</violation>
</file>

<file name="src/synapse_os/plugins.py">

<violation number="1" location="src/synapse_os/plugins.py:86">
P2: Re-registering a hook type leaks the old handler in `_handlers`, leaving stale callbacks active.</violation>

<violation number="2" location="src/synapse_os/plugins.py:93">
P1: Disabled plugins remain active because handler retrieval does not filter by `manifest.enabled`.</violation>
</file>

<file name="src/synapse_os/control_plane/middleware.py">

<violation number="1" location="src/synapse_os/control_plane/middleware.py:18">
P1: Handle non-HTTP ASGI scopes before auth checks. Without a `scope["type"]` guard, websocket/lifespan traffic can receive HTTP response events and fail with protocol errors.</violation>
</file>

<file name="src/synapse_os/adapters.py">

<violation number="1" location="src/synapse_os/adapters.py:398">
P1: Handle `AdapterOperationalError` from `super().execute()`; launcher failures currently bypass Copilot circuit-breaker failure recording.</violation>
</file>

<file name="features/F64-advanced-supervisor-policies/SPEC.md">

<violation number="1" location="features/F64-advanced-supervisor-policies/SPEC.md:27">
P2: Add the required `# Objetivo` H1 section. Using `# Decisão` instead can break SPEC validation, which expects `# Contexto` and `# Objetivo`.</violation>
</file>

<file name="features/F66-reporting-and-observability-evolution/SPEC.md">

<violation number="1" location="features/F66-reporting-and-observability-evolution/SPEC.md:31">
P2: This SPEC is missing the required `# Objetivo` H1 section, which can break SPEC validation/parsing in the project workflow.</violation>
</file>

<file name="src/synapse_os/control_plane/models.py">

<violation number="1" location="src/synapse_os/control_plane/models.py:44">
P2: `mode` is exposed in the run-creation request model but is ignored by the endpoint, so clients cannot actually control sync/async/auto behavior despite the API contract advertising it.</violation>
</file>

<file name="src/synapse_os/supervisor.py">

<violation number="1" location="src/synapse_os/supervisor.py:17">
P2: A second `AdapterOperationalError` type is introduced with a different shape than the one raised by adapters, so `AdvancedSupervisor` won’t recognize real adapter operational failures for short-circuit/reroute/retry decisions.</violation>
</file>

<file name="tests/unit/test_plugins.py">

<violation number="1" location="tests/unit/test_plugins.py:164">
P2: This test patches the wrong target; `load_plugins()` uses `synapse_os.plugins.entry_points`, so the mock is bypassed and the test may hit real entry points.</violation>
</file>

<file name="tests/unit/test_pipeline_dag.py">

<violation number="1" location="tests/unit/test_pipeline_dag.py:405">
P2: The concurrency-limit assertion is ineffective and can pass even when `max_workers` is not enforced.</violation>
</file>

<file name="tests/unit/test_memory.py">

<violation number="1" location="tests/unit/test_memory.py:3">
P2: Remove unused imports to avoid Ruff `F401` lint failures.</violation>
</file>

<file name="tests/unit/test_reporting_evolution.py">

<violation number="1" location="tests/unit/test_reporting_evolution.py:5">
P2: Remove the unused `pytest` import; with Ruff `F` rules enabled, this triggers `F401` and breaks lint checks.</violation>
</file>

<file name="src/synapse_os/cli/app.py">

<violation number="1" location="src/synapse_os/cli/app.py:849">
P2: Catch `CLIError` in `control-plane start` so environment/config errors are reported via `exit_for_cli_error` instead of bubbling as uncaught exceptions.</violation>

<violation number="2" location="src/synapse_os/cli/app.py:871">
P2: Guard `SYNAPSE_CONTROL_PORT` parsing in `control-plane status`; invalid env values currently raise an uncaught `ValueError`.</violation>
</file>

<file name="SECURITY_AUDIT_REPORT.md">

<violation number="1" location="SECURITY_AUDIT_REPORT.md:183">
P2: This comment labels `sys.argv[1]` usage as code injection, but the prompt is passed as data argument, not injected into the Python source.</violation>
</file>

<file name="tests/unit/test_control_plane.py">

<violation number="1" location="tests/unit/test_control_plane.py:380">
P2: The cancel test is too permissive: it does not assert `mark_run_cancelled` received `run_id` and `current_state`. This can miss regressions that would fail against the real repository signature.</violation>
</file>

<file name="tests/unit/test_copilot_adapter.py">

<violation number="1" location="tests/unit/test_copilot_adapter.py:3">
P2: Remove unused `unittest.mock` imports to avoid Ruff `F401` lint failure.</violation>
</file>

<file name="docs/architecture/SDD.md">

<violation number="1" location="docs/architecture/SDD.md:537">
P2: This line states DAG parallel execution is already implemented, but the main `PipelineEngine` still executes a linear state flow. Update the status text to avoid documenting unfinished runtime behavior as complete.</violation>
</file>

<file name="docs/adr/014-http-control-plane.md">

<violation number="1" location="docs/adr/014-http-control-plane.md:25">
P2: A lista de endpoints no ADR não corresponde às rotas reais do control plane e documenta caminhos inexistentes.</violation>

<violation number="2" location="docs/adr/014-http-control-plane.md:30">
P2: O ADR descreve incorretamente o gatilho de ativação da API; o control plane sobe via comando explícito `synapse control-plane start`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.}

[cubic-dev-ai]

9 issues found across 26 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/adr/014-http-control-plane.md">

<violation number="1" location="docs/adr/014-http-control-plane.md:28">
P2: The ADR documents the wrong environment variable for control-plane Bearer auth; it should match the implemented `SYNAPSE_API_TOKEN` name.</violation>
</file>

<file name="tests/unit/test_pipeline_dag.py">

<violation number="1" location="tests/unit/test_pipeline_dag.py:404">
P2: This assertion is ineffective for concurrency limiting: `concurrent` only contains `1`s, so `max(concurrent)` is always `1` and cannot detect worker over-parallelism.</violation>
</file>

<file name="src/synapse_os/plugins.py">

<violation number="1" location="src/synapse_os/plugins.py:88">
P1: Removing the old handler here can unregister a shared callable that is still used by another plugin.</violation>

<violation number="2" location="src/synapse_os/plugins.py:101">
P2: Handler filtering is incorrect for shared callables because it checks only the first plugin match.</violation>
</file>

<file name="features/F67-workspace-management-v2/SPEC.md">

<violation number="1" location="features/F67-workspace-management-v2/SPEC.md:10">
P2: The SPEC defines conflicting WorkspacePool operations (`discard` in outputs vs `reset` in scope), which makes the feature contract ambiguous.</violation>
</file>

<file name="features/F60-local-control-plane-foundation/SPEC.md">

<violation number="1" location="features/F60-local-control-plane-foundation/SPEC.md:16">
P2: Use a consistent route parameter name for the cancel endpoint (`{run_id}`) to avoid spec ambiguity.</violation>

<violation number="2" location="features/F60-local-control-plane-foundation/SPEC.md:17">
P3: Clarify that 401 behavior applies when API-token auth is enabled; the current wording conflicts with the dev-mode auth-disabled rule.</violation>
</file>

<file name="src/synapse_os/pipeline_dag.py">

<violation number="1" location="src/synapse_os/pipeline_dag.py:153">
P2: Validate the spec unconditionally in `DAGExecutor.__init__`; gating on `mode == "dag"` allows unsupported modes to bypass `DAGValidator` and proceed silently.</violation>
</file>

<file name="features/F62-copilot-adapter/SPEC.md">

<violation number="1" location="features/F62-copilot-adapter/SPEC.md:43">
P2: The SPEC now requires `SYNAPSE_OS_GH_TOKEN`, but the codebase has no corresponding setting or adapter wiring for it. This creates a misleading contract for implementers and reviewers.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.}