Skip to content

feat: notify actions + LLM-backed rule suggestion (tracks θ + η)#40

Merged
chenliuyun merged 17 commits intomainfrom
feat/notify-llm-suggest
May 7, 2026
Merged

feat: notify actions + LLM-backed rule suggestion (tracks θ + η)#40
chenliuyun merged 17 commits intomainfrom
feat/notify-llm-suggest

Conversation

@chenliuyun
Copy link
Copy Markdown
Collaborator

@chenliuyun chenliuyun commented May 6, 2026

Summary

  • Track θ — NotifyAction: New type: notify action in the rules engine lets rules POST to webhooks, append to files, or push to OpenClaw after firing. Adds discriminated union types, executor, audit rule-notify kind, JSON schema notifyAction definition, lintRules URL/field validation, rule_notifications MCP tool, and doctor notify-connectivity check.

  • Track η — LLM-backed suggestion: rules suggest and rules_suggest MCP tool gain an optional --llm flag. Simple intents use the existing heuristic; complex intents (score ≥ 4) are routed to OpenAI or Anthropic. Auto mode falls back to heuristic with a warning on provider failure. All LLM calls are written to the audit log.

Follow-up fixes (556e2ac)

Four regressions caught in review, fixed before merge:

  • [P1] notify dry-run bypassexecuteNotifyAction now receives globalDryRun from the engine and skips all side effects (webhook POST / file write) when either --dry-run or dry_run: true is set; audit entry is still written with dryRun: true.
  • [P1] CLI --llm not wiredrules suggest was calling suggestRule without forwarding the --llm option; the option is now declared and passed through.
  • [P1] MCP rules_suggest missing llminputSchema and handler both lacked the llm field, making the LLM path unreachable from agents; both are updated and the tool description corrected.
  • [P3] rule_notifications total offtotal was reporting bounded.length (post-limit count) instead of entries.length (full filtered count), breaking pagination semantics.

Follow-up fixes (ef0474a)

Three correctness issues caught in second review pass:

  • [P2] LLM silently drops explicit overridestrigger, event, schedule, days, webhookPath are now appended as explicit constraints in the LLM user message so the model cannot produce a conflicting rule structure.
  • [P2] LLM output not validated before lintRules — added a structural guard checking the parsed YAML is a non-null object with when and then fields before passing to lintRules; malformed model output now surfaces a clean UsageError instead of an internal crash.
  • [P2] Non-HTTP/HTTPS notify URLs silently downgradedsendWebhook now rejects any URL whose protocol is not http: or https: with an explicit error, rather than treating it as plain HTTP.

Follow-up fixes — third review round (MVP A/B/C/D)

Seven additional fixes after a third pass through the diff. Split into four
self-contained MVPs so each can be reviewed and reverted independently.

MVP A — Contract guarantees (6b5ae1f)

  • [P1] LLM dry_run was advisory. suggestRuleWithLlm now coerces every
    generated rule to dry_run: true regardless of what the model returned,
    with a warning on the result if the model proposed false or omitted it.
    This closes the path where a creative model could ship a live-arming
    suggestion past the audit guards.
  • [P2] LLM ignored explicit caller overrides at the structure level. Even
    though the user message was hinting --trigger/--event/--schedule/
    --days/--webhook-path, nothing actually checked the model's output.
    Now: a mismatched --trigger raises UsageError (caller is right, model
    is wrong); within the same trigger, mismatched event/schedule/days/path
    are rewritten to the caller's value with a warning.
  • [P2] rules lint accepted non-http(s) notify URLs. A to: ftp://...
    passed lint and only failed at runtime (or worse — silently fell through
    on file:// targets in the file-channel path). Lint now emits
    notify-unsupported-protocol for any scheme other than http: / https:,
    matching the runtime guard.

MVP B — UX defenses (86ed243)

  • [P2] --llm had no enum validation in the CLI. Any string fell through
    and produced confusing downstream errors. Now rules suggest --llm garbage
    exits with a clear UsageError (exit 2). MCP rules_suggest already
    enforced this via Zod.
  • [P3] --llm auto silently used the heuristic for low-complexity
    intents.
    Callers couldn't tell whether the LLM was actually invoked. The
    result now carries an explicit warning (intent complexity below LLM threshold; used heuristic. Pass --llm openai|anthropic to force LLM.)
    when this happens.

MVP C — Observability & defense in depth (065e8da)

  • [P3] LLM audit always recorded result: 'ok'. The audit row was
    written immediately after provider.generateYaml, before any
    parse/validate/lint step that could reject the suggestion. Audit is now
    deferred until the final outcome, with result: 'error' and the message
    captured when the pipeline rejects.
  • [P3] No structural validation of LLM output. Defects that bypassed
    lintRules (wrong types, missing required fields, unknown enum values)
    could surface as crashes deeper in the pipeline. Generated rules are now
    wrapped into a synthetic v0.2 policy and run through validateLoadedPolicy
    (the same Ajv schema validator as policy validate) before lint.

MVP D — Documentation (10bc49a)

  • README rules suggest section now spells out the four guardrails:
    forced dry_run, hard alignment with explicit overrides, CLI --llm
    enum validation, and the http(s)-only notify URL rule.
  • Notify channel section explicitly calls out the protocol restriction.

Test plan

  • npx tsc --noEmit — 0 errors
  • npx vitest run — 2105 tests, 0 failures (baseline 2096 + 9 new)
  • rules lint on notify actions validates URL, required to field, and rejects non-http(s) protocols
  • Engine dispatches type: notify actions to executeNotifyAction
  • notify actions are skipped (no webhook/file side effects) under --dry-run
  • rule_notifications MCP tool returns only rule-notify audit entries
  • rule_notifications total reflects filtered count, not page count
  • doctor --section notify-connectivity skips when no webhook notify actions exist
  • LLM mock tests cover: direct backend, auto→LLM, auto→heuristic, auto→LLM-failure-fallback
  • rules suggest --llm auto reaches the LLM path via CLI
  • rules_suggest MCP tool accepts and forwards the llm field
  • rules suggest --llm auto --trigger cron --schedule "0 22 * * *" — constraint forwarded to model
  • malformed LLM output (null / array / plain text) raises UsageError, not crash
  • notify URL with ftp:// protocol rejected at lint and at send time
  • LLM dry_run: false is coerced to true with a warning
  • --trigger mismatch raises UsageError; --event/--schedule/--days/--webhook-path mismatches rewrite with warning
  • --llm garbage exits 2 with a clear message
  • --llm auto on a simple intent emits a heuristic-fallback warning
  • LLM audit row records result: 'error' and the error string when the pipeline rejects
  • LLM rule that fails Ajv schema raises UsageError (caught before lint)

chenliuyun added 17 commits May 6, 2026 14:41
- Add suggestRule llm param (openai | anthropic | local | auto)
- Auto mode scores intent complexity and routes to LLM when score >= 4
- LLM path calls provider.generateYaml, lints result, falls back to
  heuristic with warning on provider failure
- Add buildRuleSuggestSystemPrompt embedding v0.2 schema snippet
- Wire --llm flag into CLI rules suggest command
- Add llm param to rules_suggest MCP tool
- Add llm-suggest kind to audit log with backend/model/latency fields
- Fix tests/rules/suggest.test.ts: add async/await (suggestRule is now async)
- Fix tests/llm/suggest.test.ts: set mock implementations in beforeEach
  to survive vitest restoreMocks:true between tests
…ifications total

- engine.ts: pass globalDryRun to executeNotifyAction so notify actions
  respect both rule-level dry_run and the --dry-run flag
- notify.ts: skip webhook/file side effects and write audit with dryRun:true
  when dry-run is active; add dryRun field to NotifyResult and NotifyContext
- rules.ts: add --llm <backend> option to `rules suggest` and pass it
  through to suggestRule
- mcp.ts: add llm field to rules_suggest inputSchema and handler;
  fix rule_notifications to report total as entries.length (pre-limit count)
…non-http(s) URLs

- suggest.ts: append trigger/event/schedule/days/webhookPath as explicit
  constraints in the LLM user message so models cannot silently override them
- suggest.ts: validate parsed YAML is a non-null object with when/then fields
  before passing to lintRules, preventing internal crashes on malformed output
- notify.ts: reject non-http/https protocols in sendWebhook with a clear error
  instead of silently treating them as plain HTTP
…p URLs at lint time

- LLM-generated rules are now always coerced to dry_run:true with a warning
  when the model returned dry_run:false or omitted the field.
- Caller-provided --trigger now hard-fails when the LLM returns a different
  trigger source. Within the same trigger, --event/--schedule/--days/
  --webhook-path mismatches override the LLM value and emit a warning.
- Notify webhook URLs with non-http(s) protocols (e.g. ftp://, file://) are
  now rejected at lint time with code notify-unsupported-protocol, matching
  the runtime guard.
…heuristic

- `rules suggest --llm <backend>` now rejects unknown values with a
  UsageError (exit 2). Previously any string fell through and was passed
  along, producing confusing downstream errors. MCP rule_suggest already
  enforced this via Zod enum.
- When `--llm auto` falls back to the heuristic because intent complexity
  is below the LLM threshold, the result now carries a warning telling
  the caller they can pass `--llm openai|anthropic` to force LLM.
… against policy schema

- writeAudit for kind=llm-suggest now runs after the parse/validate/lint
  pipeline, with result='ok' or 'error' set from the final outcome and
  the error message captured in the entry. Previously every LLM call
  recorded result='ok' even when the suggestion was rejected downstream.
- Generated rules are now wrapped into a synthetic v0.2 policy and run
  through validateLoadedPolicy before lint, so structural defects (wrong
  types, missing fields, unknown enums) are caught with a clear schema
  error instead of crashing further down the pipeline. Test fixtures
  updated to use plausible SwitchBot device IDs.
- README rules-suggest section now spells out the four guardrails users
  should expect: forced dry_run, hard alignment with explicit overrides,
  CLI --llm enum validation, and the http(s)-only notify URL rule.
- Notify channel descriptions explicitly call out that lint rejects
  schemes other than http:// / https://.
…mplete llm prompt schema

PR review P2 fixes:

- notify template now flattens nested event payloads into dotted keys
  (event.context.deviceMac, event.list.0). Renderer used to spread only
  top-level payload keys, so documented nested placeholders rendered
  literally. Depth-capped at 6; Date instances become ISO strings.
- channel: file rejects non-absolute paths at lint (notify-relative-path)
  and at runtime. Schema already documented absolute-path requirement;
  enforcement was missing, so logs/notify.jsonl resolved against process
  cwd, which differs across rules run / daemon / systemd contexts. Tilde
  is not expanded.
- llm rule-suggest prompt embeds trigger and triggerWebhook defs in
  addition to triggerMqtt/triggerCron, eliminating the dangling \$ref on
  rule.when. Adds a Rules-section bullet describing the webhook trigger
  shape so --trigger webhook flows reach schema validation cleanly.
- runtime appendToFile error mirrors the lint hint (absolute path
  example, tilde-not-expanded note) so users skipping lint get the
  same guidance
- comment near rule-notify writeAudit explains keeping action.to
  unsanitised is intentional forensic behaviour, not an oversight
- comment near the rule-suggest prompt template warns that
  tests/llm/rule-prompt.test.ts string-slices on the
  "JSON Schema:\n" / "\n\nRules:" delimiters
@chenliuyun chenliuyun merged commit be51931 into main May 7, 2026
11 checks passed
@chenliuyun chenliuyun deleted the feat/notify-llm-suggest branch May 7, 2026 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant