Cover ecosystem degraded modes with a shared fault-injection harness #82

Closed
claude wants to merge 1 commits from overnight/eco-degraded-suite into overnight/netscan
Contributor

Phase-5 hardening for the ecosystem seam. Test-only diff, no production code changed.

What changed

  • cmd/mavend/fakeecosystem_test.go: the shared fake Nexus/Praxis/Hexis server now captures request headers and query strings, and grows three fault levers — SetBody (healthy transport, malformed or drifted payload), SetDelay (drives client timeouts and context cancellation), and a Count(method, prefix) helper. Fake Praxis exposes the full /api/v1/tools/* lifecycle surface. New fixtures: the flat resolve shape from ECOSYSTEM-SPEC §1.5, a forward-compatible resolve carrying unknown fields, and a Hexis execution-failure body.
  • cmd/mavend/ecosystem_degraded_test.go: new suite asserting the degraded-mode contract — outages degrade independently, a degraded reply is never silent and never claims success, malformed and drifted contracts are survivable, cancellation stops before the next hop, execution failure is distinct from transport failure and writes no success trace, ambiguity blocks mutation, the attention digest never chains into Hexis on its own, mutating capabilities park for confirmation, a failed surface still delivers the digest, and recovery needs no restart.

Why
Existing ecosystem tests only covered basic Nexus+Hexis happy paths with one-off inline handlers. This extends the harness that overnight/replay-simulator (#284) already builds on rather than adding a second one.

Verified
make build and make test both exit 0.

Vikunja #276

Phase-5 hardening for the ecosystem seam. Test-only diff, no production code changed. **What changed** - `cmd/mavend/fakeecosystem_test.go`: the shared fake Nexus/Praxis/Hexis server now captures request headers and query strings, and grows three fault levers — `SetBody` (healthy transport, malformed or drifted payload), `SetDelay` (drives client timeouts and context cancellation), and a `Count(method, prefix)` helper. Fake Praxis exposes the full `/api/v1/tools/*` lifecycle surface. New fixtures: the flat resolve shape from ECOSYSTEM-SPEC §1.5, a forward-compatible resolve carrying unknown fields, and a Hexis execution-failure body. - `cmd/mavend/ecosystem_degraded_test.go`: new suite asserting the degraded-mode contract — outages degrade independently, a degraded reply is never silent and never claims success, malformed and drifted contracts are survivable, cancellation stops before the next hop, execution failure is distinct from transport failure and writes no success trace, ambiguity blocks mutation, the attention digest never chains into Hexis on its own, mutating capabilities park for confirmation, a failed surface still delivers the digest, and recovery needs no restart. **Why** Existing ecosystem tests only covered basic Nexus+Hexis happy paths with one-off inline handlers. This extends the harness that `overnight/replay-simulator` (#284) already builds on rather than adding a second one. **Verified** `make build` and `make test` both exit 0. Vikunja #276
claude added 1 commit 2026-08-01 04:49:19 +02:00
Extend the fake Nexus/Praxis/Hexis harness with request header and query
capture, a malformed-body lever, a response delay lever, and a request
counter, then add a degraded-mode suite on top of it: independent outages,
malformed and drifted contracts, cancellation, execution failure vs
transport failure, ambiguous targets, no autonomous Praxis to Hexis
chaining, confirmation for mutating capabilities, and recovery without a
restart.
claude reviewed 2026-08-01 11:30:41 +02:00
claude left a comment
Author
Contributor

The three fault levers belong on the shared fake, not in one-off inline handlers. That is what makes this suite worth having. The property list at the top is the useful part: degrade independently, never fabricate, never chain observation into execution. TestEcosystem_NoAutonomousPraxisToHexis asserts the attention digest contacts neither Hexis nor Nexus. That is the right shape for "not autonomous", because it checks the wire and not the reply text. SetDelay abandoning on r.Context().Done() keeps the cancellation test from leaking a goroutine per case.

1. traceFacts reads a fact that the code cannot write, so both trace assertions are vacuous. recordPraxisTrace writes Kind: "system". facts.kind is CHECK (kind IN ('self','env','config')), so the insert fails, and the writer discards the error with _, _ =. No row with source praxis:trace has ever existed. That makes TestEcosystem_ExecutionFailureIsNotSuccess loop over an empty slice, and len(traceFacts(t, h)) != 0 in TestEcosystem_TotalOutageSaysSoForEveryPath an assertion that zero equals zero. Both pass with the trace writer deleted entirely. This is the exact failure the suite exists to catch: a silent write that looks like bookkeeping and is not. PR 84 fixes the kind. It should have been caught here. Reading facts back is the only reason for a test to touch them. A positive assertion on the success path would have found it in one run.

2. Fail-closed is only tested for a body that fails to parse. nexus.SetBody({"status":"resolved","entity":) errors in json.Unmarshal, so Resolve returns an error and handleHexisAct degrades. The contract-violating case that decodes cleanly is not covered. {"status":"resolved"} with no entity and no flat entity_id unmarshals fine. resolveEntityReference then falls off the end and returns "", "", nil, nil. handleHexisAct sees entityID == "" and returns "", and actionAct falls straight through to h.tools.Exec on the local allowlist. Nexus claiming a resolve it did not deliver is a dependency failure. It currently reaches the local executor with the user verb intact. That is the same seam the malformed test names, one layer down. Add fixtureNexusResolvedEmpty and assert the fallthrough does not happen.

3. TestEcosystem_OutagesAreIndependent cannot fail as written. handleHexisAct never touches Praxis, and handlePraxisAct never touches Nexus or Hexis. The two halves are disjoint call graphs, so faulting one and exercising the other tests the call graph and not the degradation. Assert on shared state instead. A Nexus outage leaving a cached failure that a later Praxis turn reads. One client's 10s timeout serialising the other. TestEcosystem_RecoveryAfterOutageNeedsNoRestart is the version that can fail, and it only covers Praxis.

4. The acknowledge arm of the total-outage test never reaches Praxis. praxisActDec("acknowledge_item") leaves Slots.Value empty, so praxisItemAction.handle returns a.ask before calling anything. It answers "какой пункт отметить принятым?" during a total outage, and the test scores that as a correct degraded reply. The lifecycle verbs are the ones that mutate remote state, so they are the ones worth faulting. Give that case a Value and the arm starts testing the 503.

Smaller:

  • SetFault is server-wide, so a single endpoint cannot fail alone. The one genuine partial-failure test, TestEcosystem_SurfaceFailureStillDelivers, has to build an inline newFakeServer to get there. That contradicts the file header, which says everything drives the shared fakes. A SetRouteFault(key, status) would fold it back in and would also cover "attention works, pin is down".
  • restartCaps() declares restart with read_only: true. Restarting a service is the canonical mutation. Most happy paths in the suite execute without confirmation because of that label. A regression dropping the confirm gate would be caught only by the one test setting read_only: false.
  • actDec hardcodes Fn: "restart", so actDec("restart") in the confirmation test reads as if the verb came from the text. It does not. The argument only feeds the resolve query.
  • capturedRequest grows Header and Query, and nothing in this PR asserts on either. Fine as groundwork for 83 and 84, worth saying so in the comment.
  • No case covers 401 or 403. Today they collapse into the same "экосистема недоступна" as a connection refusal, which is the reply a misconfigured token would produce forever.
  • SetBody is only ever pointed at Nexus. Praxis getJSON has the same decode path and no coverage.
The three fault levers belong on the shared fake, not in one-off inline handlers. That is what makes this suite worth having. The property list at the top is the useful part: degrade independently, never fabricate, never chain observation into execution. `TestEcosystem_NoAutonomousPraxisToHexis` asserts the attention digest contacts neither Hexis nor Nexus. That is the right shape for "not autonomous", because it checks the wire and not the reply text. `SetDelay` abandoning on `r.Context().Done()` keeps the cancellation test from leaking a goroutine per case. **1. `traceFacts` reads a fact that the code cannot write, so both trace assertions are vacuous.** `recordPraxisTrace` writes `Kind: "system"`. `facts.kind` is `CHECK (kind IN ('self','env','config'))`, so the insert fails, and the writer discards the error with `_, _ =`. No row with source `praxis:trace` has ever existed. That makes `TestEcosystem_ExecutionFailureIsNotSuccess` loop over an empty slice, and `len(traceFacts(t, h)) != 0` in `TestEcosystem_TotalOutageSaysSoForEveryPath` an assertion that zero equals zero. Both pass with the trace writer deleted entirely. This is the exact failure the suite exists to catch: a silent write that looks like bookkeeping and is not. PR 84 fixes the kind. It should have been caught here. Reading facts back is the only reason for a test to touch them. A positive assertion on the success path would have found it in one run. **2. Fail-closed is only tested for a body that fails to parse.** `nexus.SetBody(`{"status":"resolved","entity":`)` errors in `json.Unmarshal`, so `Resolve` returns an error and `handleHexisAct` degrades. The contract-violating case that decodes cleanly is not covered. `{"status":"resolved"}` with no entity and no flat `entity_id` unmarshals fine. `resolveEntityReference` then falls off the end and returns `"", "", nil, nil`. `handleHexisAct` sees `entityID == ""` and returns `""`, and `actionAct` falls straight through to `h.tools.Exec` on the local allowlist. Nexus claiming a resolve it did not deliver is a dependency failure. It currently reaches the local executor with the user verb intact. That is the same seam the malformed test names, one layer down. Add `fixtureNexusResolvedEmpty` and assert the fallthrough does not happen. **3. `TestEcosystem_OutagesAreIndependent` cannot fail as written.** `handleHexisAct` never touches Praxis, and `handlePraxisAct` never touches Nexus or Hexis. The two halves are disjoint call graphs, so faulting one and exercising the other tests the call graph and not the degradation. Assert on shared state instead. A Nexus outage leaving a cached failure that a later Praxis turn reads. One client's 10s timeout serialising the other. `TestEcosystem_RecoveryAfterOutageNeedsNoRestart` is the version that can fail, and it only covers Praxis. **4. The `acknowledge` arm of the total-outage test never reaches Praxis.** `praxisActDec("acknowledge_item")` leaves `Slots.Value` empty, so `praxisItemAction.handle` returns `a.ask` before calling anything. It answers "какой пункт отметить принятым?" during a total outage, and the test scores that as a correct degraded reply. The lifecycle verbs are the ones that mutate remote state, so they are the ones worth faulting. Give that case a `Value` and the arm starts testing the 503. Smaller: - `SetFault` is server-wide, so a single endpoint cannot fail alone. The one genuine partial-failure test, `TestEcosystem_SurfaceFailureStillDelivers`, has to build an inline `newFakeServer` to get there. That contradicts the file header, which says everything drives the shared fakes. A `SetRouteFault(key, status)` would fold it back in and would also cover "attention works, pin is down". - `restartCaps()` declares `restart` with `read_only: true`. Restarting a service is the canonical mutation. Most happy paths in the suite execute without confirmation because of that label. A regression dropping the confirm gate would be caught only by the one test setting `read_only: false`. - `actDec` hardcodes `Fn: "restart"`, so `actDec("restart")` in the confirmation test reads as if the verb came from the text. It does not. The argument only feeds the resolve query. - `capturedRequest` grows `Header` and `Query`, and nothing in this PR asserts on either. Fine as groundwork for 83 and 84, worth saying so in the comment. - No case covers 401 or 403. Today they collapse into the same "экосистема недоступна" as a connection refusal, which is the reply a misconfigured token would produce forever. - `SetBody` is only ever pointed at Nexus. Praxis `getJSON` has the same decode path and no coverage.
kami closed this pull request 2026-08-01 14:52:08 +02:00
Owner

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Pull request closed

Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kami/Maven#82