fix(agents/playbook): correctness pass on engine evaluation + flattening

[beenuar]

Summary

Five silent-correctness bugs in services/agents/app/playbook/engine.py were producing wrong-but-passing playbook runs. None failed loudly, so existing happy-path tests didn't catch them.

Bugs fixed in engine.py

1. Expression-string conditions silently always-true. expression: \"x > 1\" form was accepted by the schema but the evaluator only matched the structured field/operator/value shape, so any expression-string condition fell through to True. Now routed through a new _evaluate_expression that parses <lhs> <op> <rhs> (with is null / is not null sugar) against the run context.
2. gt / lt crashed on non-numeric values. Direct > / < on mixed types raised TypeError. Added _to_float coercion that returns None on unparseable input, so the comparison cleanly evaluates to False instead of bombing the run.
3. contains was substring-only. Documented playbook intent supports both substring (str haystack) and list-membership (list/tuple haystack); now does the right thing for both.
4. _resolve_field couldn't index lists. Paths like entities.0.name bailed at the first non-dict node. Now traverses list/tuple indices alongside dict keys.
5. Context flattening clobbered later step results. PlaybookEngine.run used pr.context.setdefault(k, v), so a later step's output couldn't override an earlier step's key in the flat context. Switched to direct assignment; the per-step _step_<id> namespaced snapshot is still preserved for auditability.

Tests

Adds services/agents/tests/test_playbook_engine_correctness.py — 61 tests across seven classes covering:

• _resolve_field list/tuple index traversal
• _to_float coercion (None, empty string, bool, numeric strings, bogus input)
• Structured condition evaluation (eq/ne/exists/not_exists/contains/gt/lt + list membership + null handling)
• Expression-form condition evaluation
• _evaluate_expression edge cases (literals, dot-paths, is null / is not null)
• PlaybookEngine.run context flattening override semantics
• The engine's condition-gated step-skip path (handler not invoked, StepStatus.SKIPPED recorded)

_emit is monkeypatched per-test via fixture to avoid hitting _REALTIME_URL during tests.

Out of scope

Bounds + SSRF guards land separately on a later branch (those files don't exist on this branch's base).

Test plan

• poetry run pytest tests/test_playbook_engine_correctness.py — 61 passed
• poetry run pytest tests/ -k playbook --ignore=tests/test_explain_endpoint.py — 69 passed (no regressions in existing playbook tests; the ignored file has a pre-existing unrelated asyncpg import drift on this branch)
• CI green on PR

Made with Cursor