test(serve): add endpoint tests for edda-serve HTTP handlers

[fagemx]

Problem

edda-serve has 8,722 lines of code with 12+ HTTP endpoints but only 1 test. All endpoint logic (validation, error mapping, query param parsing, auth middleware) is untested at the HTTP layer.

Current test distribution shows the gap:

Crate	Tests
edda-bridge-claude	525
edda-core	163
edda-ledger	159
edda-serve	1

Scope

Add at minimum one happy-path + one error-case test for each endpoint group:

• Decision endpoints (status, context, list, batch, outcomes, chain)
• Draft endpoints (GET/POST, approve/deny)
• Event recording (note, decide)
• Telemetry (ingest, stats)
• Snapshots & patterns
• Analytics (recap, overview)
• Metrics & quality
• Auth & device pairing
• Ingestion endpoints

Notes

• Use axum::test or tower::ServiceExt for handler testing
• Existing test pattern in the file can be extended
• edda-ledger tests cover storage logic; these tests should focus on HTTP-layer behavior (status codes, validation, serialization)

[fagemx]

Research: edda-serve Endpoint Test Coverage (GH-374)

Current State

Architecture (Post PR #383 Split)

edda-serve has been split from a monolithic lib.rs into 16 files:

• lib.rs (entrypoint + test router + tests)
• error.rs (AppError enum + IntoResponse + classify_open_error)
• state.rs (AppState + ServeConfig + ChronicleContext)
• helpers.rs (validate_iso8601, time_now_rfc3339)
• middleware.rs (auth_middleware, is_localhost, generate_pairing_token)
• api/mod.rs + 12 endpoint modules

API Modules and Routes

Module	Routes	Handler Count
events.rs	/api/health, /api/status, /api/context, /api/decisions, /api/decisions/batch, /api/decisions/{id}/outcomes, /api/decisions/{id}/chain, /api/log, /api/note, /api/decide, /api/events/karvi	11
drafts.rs	/api/drafts, /api/drafts/{id}/approve, /api/drafts/{id}/deny	3
telemetry.rs	POST/GET /api/telemetry, /api/telemetry/stats	3
snapshots.rs	/api/snapshot, /api/snapshots, /api/snapshots/{hash}, /api/villages/{id}/stats, /api/patterns	5
analytics.rs	/api/recap, /api/recap/cached, /api/overview, /api/projects	4
metrics.rs	/api/metrics/quality, /api/metrics/overview, /api/metrics/trends, /api/controls/suggestions, /api/controls/patches, /api/controls/patches/{id}/approve	6
dashboard.rs	/api/dashboard	1
policy.rs	/api/scope/check, /api/scope/whitelist, /api/authz/check, /api/approval/check, /api/tool-tier/{name}	5
briefs.rs	/api/actors, /api/actors/{name}, /api/briefs, /api/briefs/{id}, /dashboard (HTML)	5
auth.rs	/pair, /api/pair/new, /api/pair/list, /api/pair/revoke, /api/pair/revoke-all	5
ingestion.rs	/api/ingestion/evaluate, POST/GET /api/ingestion/records, /api/ingestion/suggestions, /api/ingestion/suggestions/{id}/accept, /api/ingestion/suggestions/{id}/reject	6
stream.rs	/api/events/stream (SSE)	1
Sync (test-only)	/api/sync	1

Total: ~55 handler endpoints

Existing Test Coverage (108 tests)

The issue claims "only 1 test" but this was likely true at filing time. The current state has 108 #[tokio::test] + 3 #[test] covering:

Well-Covered Endpoints:

• events.rs: health, status, context, decisions, log, note, decide, karvi events, outcomes, chain, batch - ~30 tests
• drafts.rs: GET drafts, approve, deny, replay protection, not-found, device-id - ~7 tests
• snapshots.rs: POST snapshot, GET snapshots, GET by hash, patterns - ~7 tests
• analytics.rs: recap, recap/cached, overview, projects - ~4 tests
• metrics.rs: quality (empty, with events, date filter), overview, trends - ~5 tests
• policy.rs: scope/check (5 variants), authz/check (6 variants), tool-tier (2 variants) - ~13 tests
• auth.rs (middleware): localhost bypass, missing header, malformed, invalid token, revoked, valid, public route, IPv6 - ~8 tests
• telemetry.rs: POST create, dedup, bad JSON, GET list, stats - ~5 tests
• ingestion.rs: evaluate (auto, suggest, skip), accept/reject, manual, 404, 409 - ~9 tests
• stream.rs (SSE): content-type, new events, type filter, since replay, Last-Event-ID - ~5 tests
• dashboard.rs: all sections, days param, HTML page - ~3 tests
• briefs.rs: actors empty, actor not found - ~2 tests
• error.rs: classify_open_error (3 variants), 501 without chronicle, 404 for non-workspace, retry-after header - ~6 tests

Missing/Weak Coverage (Gaps):

1. Briefs endpoints (/api/briefs, /api/briefs/{task_id}):

   • No test for GET /api/briefs happy path (with task briefs in ledger)
   • No test for GET /api/briefs/{task_id} happy path
   • No test for GET /api/briefs/{task_id} 404 case

2. Auth pairing endpoints (/api/pair/new, /pair, /api/pair/list, /api/pair/revoke, /api/pair/revoke-all):

   • No test for POST /api/pair/new (create pairing)
   • No test for GET /pair?token=... (complete pairing)
   • No test for GET /api/pair/list (list paired devices)
   • No test for POST /api/pair/revoke (revoke device)
   • No test for POST /api/pair/revoke-all
   • No error cases for empty device_name, expired token, etc.

3. Controls patches (/api/controls/patches, /api/controls/patches/{id}/approve):

   • No test for GET /api/controls/patches
   • No test for POST /api/controls/patches/{id}/approve

4. Approval check (/api/approval/check):

   • No test for POST /api/approval/check

5. Village stats (/api/villages/{village_id}/stats):

   • No test for this endpoint

6. ISO 8601 validation on temporal endpoints:

   • No test for invalid after/before params on decisions endpoint returning 400

7. Log endpoint filters:

   • GET /api/log exists but no test for type/keyword/date filtering

8. Sync endpoint (test-only):

   • No tests exist despite the endpoint being defined

9. Error response format consistency:

   • No tests verifying all error variants produce {"error": "...", "code": "..."} format

Test Infrastructure

• Setup helper: setup_workspace(dir) - initializes ledger, head branch, branches.json
• Router builder: router(repo_root) - builds test router without auth middleware
• Auth router: app_with_auth(repo_root) - builds router WITH auth middleware
• No chronicle: router_no_chronicle(repo_root) - builds router without chronicle context
• Store lock: STORE_LOCK mutex for tests using EDDA_STORE_ROOT env var
• Pattern: tower::ServiceExt::oneshot() with axum::body::Body
• Dependencies: tempfile, tokio (rt-multi-thread, macros), tower (util), http-body-util

Key Design Constraints

1. All tests use real SQLite via tempfile::tempdir() - no mocking
2. Tests that set EDDA_STORE_ROOT must hold STORE_LOCK and use StoreRootGuard
3. Router is rebuilt for each oneshot() call because axum consumes it
4. Tests use Router::new().merge(router(...)) for .clone() support on multi-request tests
5. Auth tests use ConnectInfo extension for IP simulation

dev-dependencies (Cargo.toml)

[dev-dependencies]
tempfile.workspace = true
tokio = { version = "1", features = ["rt-multi-thread", "macros"] }
tower = { version = "0.5", features = ["util"] }
http-body-util = "0.1"

[fagemx]

Innovation: edda-serve Test Coverage Strategy (GH-374)

Problem Restatement

The issue asks for "at minimum one happy-path + one error-case test for each endpoint group." Research reveals 108 tests already exist, covering most endpoint groups. The remaining gaps are concentrated in 5 areas:

1. Auth pairing flow (5 untested endpoints)
2. Briefs query endpoints (2 untested endpoints)
3. Controls patches (2 untested endpoints)
4. Approval check (1 untested endpoint)
5. Village stats (1 untested endpoint)

Plus several edge-case gaps in already-tested endpoints (ISO validation, log filters, etc.)

Solution Approaches

Approach A: Gap-Fill Only (Recommended)

Add tests only for the identified gaps. Focus on HTTP-layer behavior (status codes, validation, serialization) as the issue specifies.

Estimated tests to add: ~25-30 new tests

Pros:

• Targeted, minimal change
• Fills all remaining gaps per issue checklist
• No restructuring risk

Cons:

• All tests remain in one 4700+ line lib.rs

Approach B: Gap-Fill + Test Module Split

Fill gaps AND move test blocks to per-module files (e.g., api/events_tests.rs, api/drafts_tests.rs).

Pros:

• Better organization for future maintenance
• Easier to find tests related to a specific module

Cons:

• Restructuring is out of scope for a test issue
• Risk of breaking existing test infrastructure (STORE_LOCK, setup_workspace, etc.)
• Shared helpers would need to be extracted to a test_utils module

Approach C: Property-Based Tests

Use proptest or quickcheck for fuzz-testing request parsing and validation.

Pros:

• Catches edge cases humans miss

Cons:

• Adds external dependency (violates zero-dep for dev-deps? unclear)
• Overkill for this issue scope
• Complex setup for HTTP handler testing

Recommended: Approach A (Gap-Fill Only)

Priority Order by Value

1. Auth pairing endpoints (HIGH) - Critical security surface, completely untested
2. Briefs endpoints (MEDIUM) - User-facing query API, no happy-path test
3. Village stats (MEDIUM) - Validation logic (ISO 8601) untested
4. Approval check (MEDIUM) - Complex business logic untested
5. Controls patches (LOW) - Depends on bridge-claude file-system state
6. ISO validation / log filters (LOW) - Edge cases in already-tested endpoints

Test Design Patterns

For consistency with existing tests, every new test should follow the established pattern:

#[tokio::test]
async fn endpoint_behavior_description() {
    let tmp = tempfile::tempdir().unwrap();
    setup_workspace(tmp.path());
    // ... seed data if needed ...
    let app = router(tmp.path()); // or Router::new().merge(router(...)) for multi-request

    let resp = app.oneshot(
        Request::builder()
            .method("POST")  // or GET
            .uri("/api/...")
            .header("content-type", "application/json")
            .body(Body::from(payload))
            .unwrap(),
    ).await.unwrap();

    assert_eq!(resp.status(), StatusCode::OK);
    // ... assert response body ...
}

Specific Test Cases

Auth Pairing (8 tests)

Test	Type	Endpoint
create_pairing_returns_token	happy	POST /api/pair/new
create_pairing_rejects_empty_name	error	POST /api/pair/new
complete_pairing_returns_device_token	happy	GET /pair?token=...
complete_pairing_rejects_invalid_token	error	GET /pair?token=bad
list_paired_devices_empty	happy	GET /api/pair/list
list_paired_devices_after_pairing	happy	GET /api/pair/list
revoke_device_success	happy	POST /api/pair/revoke
revoke_device_not_found	error	POST /api/pair/revoke

Briefs (3 tests)

Test	Type	Endpoint
get_briefs_empty	happy	GET /api/briefs
get_brief_not_found	error	GET /api/briefs/{task_id}
get_briefs_with_status_filter	happy	GET /api/briefs?status=...

Village Stats (2 tests)

Test	Type	Endpoint
village_stats_returns_counts	happy	GET /api/villages/{id}/stats
village_stats_invalid_timestamp_400	error	GET /api/villages/{id}/stats?after=bad

Approval Check (2 tests)

Test	Type	Endpoint
approval_check_inline_bundle	happy	POST /api/approval/check
approval_check_nonexistent_bundle_404	error	POST /api/approval/check

Controls Patches (2 tests)

Test	Type	Endpoint
controls_patches_returns_list	happy	GET /api/controls/patches
controls_patches_invalid_status_400	error	GET /api/controls/patches?status=bad

Edge Cases in Existing Endpoints (5 tests)

Test	Type	Endpoint
decisions_invalid_after_param_400	error	GET /api/decisions?after=bad
log_filters_by_type	happy	GET /api/log?type=note
log_filters_by_keyword	happy	GET /api/log?keyword=test
get_telemetry_source_filter	happy	GET /api/telemetry?source=thyra
revoke_all_devices	happy	POST /api/pair/revoke-all

Total: ~22 new tests covering all gaps identified in research.

Ledger Seeding Strategy

Some tests require pre-existing data. The approach:

• Auth pairing: Use the existing pending_pairings mutex on AppState (no ledger needed for create_pairing, use in-test pairing flow for complete_pairing)
• Briefs: Seed via ledger.insert_task_brief() if available, otherwise verify empty response
• Village stats: Seed decisions with village_id field using new_decision_event
• Approval check: Use inline bundle fields (no ledger bundle needed)
• Controls patches: Use EDDA_STORE_ROOT with STORE_LOCK

Risks

1. Flaky tests from shared state: Tests using EDDA_STORE_ROOT env var are serialized via STORE_LOCK. New tests in this category must follow the same pattern.
2. Auth pairing timing: complete_pairing depends on in-memory pending_pairings state. Tests must create+complete in the same router instance (or use Arc<AppState> directly).
3. Controls patches dependency: edda_bridge_claude::controls_suggest reads from filesystem. Tests need proper EDDA_STORE_ROOT setup.

[fagemx]

Plan: edda-serve Endpoint Test Coverage (GH-374)

Overview

Add ~22 new tests to fill remaining coverage gaps in edda-serve, focusing on HTTP-layer behavior (status codes, validation, serialization). All tests go in the existing #[cfg(test)] mod tests block in crates/edda-serve/src/lib.rs.

Pre-conditions

All prerequisite PRs are merged:

• feat(edda-ledger): add error context enrichment across ledger, serve, and CLI #381: .context() error enrichment
• refactor(edda-serve): split lib.rs into api/ sub-modules #383: edda-serve split into 16 files
• feat(edda-serve): differentiate HTTP error codes (GH-379) #384: HTTP error classification
• refactor(edda-ledger): wrap sqlite_store types in domain abstractions (GH-378) #385: domain.rs abstractions

Implementation Steps

Step 1: Auth Pairing Endpoint Tests (8 tests)

Add tests for the completely untested pairing flow in api/auth.rs.

File: crates/edda-serve/src/lib.rs (append to mod tests)

Test Name	Method	URI	Assert
create_pairing_returns_token	POST	/api/pair/new	200, pairing_token present, pairing_url present, expires_in_seconds = 600
create_pairing_rejects_empty_name	POST	/api/pair/new	400, body {"device_name": ""}
complete_pairing_invalid_token	GET	/pair?token=bad	400, "invalid or expired"
list_paired_devices_empty	GET	/api/pair/list	200, empty array
pair_and_list_device	POST+GET	/api/pair/new then GET /api/pair/list	After full pairing flow (create+complete), list shows device
revoke_device_not_found	POST	/api/pair/revoke	404, {"device_name": "nonexistent"}
revoke_device_success	POST	/api/pair/revoke	200, after seeding device via insert_device_token
revoke_all_devices	POST	/api/pair/revoke-all	200, revoked_count in response

Key implementation notes:

• create_pairing + complete_pairing must use same AppState (shared pending_pairings mutex). Cannot use router() helper for multi-step flows since it creates separate state. Need to build the router manually or use the shared state approach.
• For complete_pairing test, create pairing first to get token from pending_pairings, then hit /pair?token=<token>.
• For revoke_device_success, seed a device token via ledger.insert_device_token() + seed_dummy_event() (pattern already exists in auth middleware tests).

Step 2: Briefs Endpoint Tests (3 tests)

Add tests for GET /api/briefs and GET /api/briefs/{task_id}.

Test Name	Method	URI	Assert
get_briefs_returns_empty	GET	/api/briefs	200, {"briefs": [], "count": 0}
get_brief_not_found_404	GET	/api/briefs/nonexistent	404
get_briefs_with_task_intake	GET	/api/briefs	200, briefs array non-empty after seeding task_intake event

Key implementation notes:

• Task briefs are materialized from task_intake events. Use edda_core::event::new_task_intake_event() with TaskIntakeParams to seed data.
• After appending the event, query GET /api/briefs to verify the materialized view.

Step 3: Village Stats Endpoint Tests (2 tests)

Test Name	Method	URI	Assert
village_stats_returns_counts	GET	/api/villages/v-test/stats	200, VillageStats struct fields present
village_stats_invalid_after_400	GET	/api/villages/v-test/stats?after=not-a-date	400

Key implementation notes:

• Seed decisions with village_id: Some("v-test") using new_decision_event() (same pattern as get_patterns_returns_recurring test).
• Validates ISO 8601 validation on after/before params returns 400.

Step 4: Approval Check Endpoint Tests (2 tests)

Test Name	Method	URI	Assert
approval_check_inline_returns_decision	POST	/api/approval/check	200, ApprovalDecision fields present
approval_check_nonexistent_bundle_404	POST	/api/approval/check	404, {"bundle_id": "nonexistent"}

Key implementation notes:

• Inline check uses step + optional risk_level, files_changed etc. No bundle_id needed.
• Needs approval.yaml in .edda/ dir. The default setup_workspace creates .edda/ but may not have approval policy. Use edda_core::approval::load_approval_policy() which falls back to default policy.
• For 404 test, pass a bundle_id that doesn't exist in ledger.

Step 5: Controls Patches Endpoint Tests (2 tests)

Test Name	Method	URI	Assert
controls_patches_returns_empty_list	GET	/api/controls/patches	200, empty array
controls_patches_invalid_status_400	GET	/api/controls/patches?status=invalid	400

Key implementation notes:

• Requires EDDA_STORE_ROOT env var set (uses STORE_LOCK + StoreRootGuard pattern).
• Empty list test: just verify it returns [] when no patches exist.
• Invalid status test: ?status=invalid should return 400 with validation error.

Step 6: Decisions ISO Validation Test (1 test)

Test Name	Method	URI	Assert
decisions_invalid_after_param_400	GET	/api/decisions?after=not-a-date	400, VALIDATION_ERROR

Key implementation notes:

• Tests the validate_iso8601() helper via the decisions endpoint.
• Verifies the error response format includes code: "VALIDATION_ERROR".

Step 7: Log Filter Tests (2 tests)

Test Name	Method	URI	Assert
log_filters_by_type	GET	/api/log?type=note	200, only note events returned
log_filters_by_keyword	GET	/api/log?keyword=test	200, only matching events returned

Key implementation notes:

• Seed multiple event types (note + decision), then filter by type.
• Seed events with specific text, then filter by keyword.

Step 8: Telemetry Source Filter Test (1 test)

Test Name	Method	URI	Assert
get_telemetry_filters_by_source	GET	/api/telemetry?source=thyra	200, only thyra events returned

Key implementation notes:

• POST two telemetry events with different sources, then GET with source filter.

Test Checklist vs Issue Requirements

Issue Checkbox	Status	Tests
Decision endpoints	Already covered (30+ tests)	status, context, list, batch, outcomes, chain
Draft endpoints	Already covered + no new needed	GET/POST, approve/deny, enriched fields
Event recording	Already covered	note, decide, validation errors
Telemetry	Already covered + Step 8	ingest, stats, source filter
Snapshots & patterns	Already covered	POST/GET snapshots, patterns
Analytics (recap, overview)	Already covered	recap, cached, overview, projects
Metrics & quality	Already covered	quality, overview, trends
Auth & device pairing	Step 1 (8 tests)	create, complete, list, revoke
Ingestion endpoints	Already covered	evaluate, accept/reject, manual

Additional coverage from this plan:

• Briefs endpoints (Step 2)
• Village stats (Step 3)
• Approval check (Step 4)
• Controls patches (Step 5)
• ISO validation edge case (Step 6)
• Log filters (Step 7)
• Telemetry source filter (Step 8)

Verification

cargo test -p edda-serve          # All edda-serve tests pass
cargo clippy -p edda-serve        # No new warnings
cargo fmt --check                 # Format check

Expected result: ~130 total tests in edda-serve (108 existing + ~22 new).

Estimated Effort

• Total new tests: ~22
• Files modified: 1 (crates/edda-serve/src/lib.rs)
• Risk: Low (additive only, no production code changes)
• Dependencies: None (all test infrastructure exists)

[fagemx]

Work completed. PR created: #386

All CI checks passing (7/7): Format, Clippy x3, Test x3.

Added 21 new tests covering all previously untested endpoint groups:

• Auth pairing flow (8 tests)
• Briefs endpoints (3 tests)
• Village stats (2 tests)
• Approval check (2 tests)
• Controls patches (2 tests)
• Decisions ISO validation (1 test)
• Log filters (2 tests)
• Telemetry source filter (1 test)

Total: 112 → 133 tests passing.