TeamAI/angela_assistant
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat: user-defined trigger scripts with variables and prompt injections #16

Merged
bjoern merged 4 commits from feat/user-triggers into main 2026-07-06 20:40:53 +02:00
Conversation 2 Commits 4 Files changed 24 +2565 -2
scarlet commented 2026-07-06 20:02:25 +02:00
Member
Copy link

AI-Dungeon-style scripting layer for the chat pipeline. Users write Dart trigger scripts that run on hooks and manipulate a persistent internal state.

How it works

A trigger = hook + script. Scripts are compiled with dart_eval and sandboxed in two layers: a compile-time import allowlist (dart:core, dart:convert, dart:math — dart:io and everything else is rejected with a diagnostic), backed by dart_eval's default deny-all runtime permissions (no runtime.grant() anywhere, guarded by a code comment). Execution happens in a throwaway isolate with a 1s hard timeout, so a while(true) can never stall the chat. Scripts are pure: they mutate a variable snapshot and queue effects; the TriggerEngine applies everything afterwards.

void onUserMessage(TriggerCtx ctx) {
  ctx.vars['msgs'] = ((ctx.vars['msgs'] ?? 0) as int) + 1;
  if ((ctx.vars['msgs'] as int) >= 10) {
    ctx.vars['msgs'] = 0;
    ctx.injectOnce('EVENT: bring up the mysterious letter.');
    ctx.runGeneration('Reflect on the last 10 messages.');
  }
}

Ctx API: vars (assistant-wide) / convVars (per-conversation), input/output, messageCount, injectOnce/injectPersistent(key,…)/removeInjection, runGeneration, enableTrigger/disableTrigger, log.

Hooks: on_user_message (after user msg persisted, before the agent runs — injections land in that same generation) and on_assistant_message (after final reply persisted — injections land in the next generation). Queued generations run timer-style after the chat (private output, message_user only) and cannot re-fire triggers, so loops are structurally impossible.

AI has no access: no tool, no automatic state section in the prompt. The only way state reaches the model is an explicit injectOnce/injectPersistent (rendered as ## Current Directives near the end of the chat prompt).

Pieces

  • migration v21: triggers, trigger_variables, prompt_injections (all CASCADE on assistant)
  • TriggerEngine (angela_core) with bytecode cache (keyed on script+hook hash), per-trigger cooldown, last_error tracking; failures are logged + recorded, never block the chat
  • AgentRunner.runChat consumes queued injections into buildChatPrompt
  • REST: trigger CRUD (compile-validated on create/patch, 400 with diagnostics), enable/disable, POST /triggers/:id/test dry-run (returns effects/logs without applying), variables list/set/delete (scope validated), injections list/delete
  • docs/trigger_scripting.md — full guide with sandbox-verified language-support matrix, gotchas (nested ternaries, return-in-catch, continue), and a 7-recipe cookbook; a doc test compiles AND dry-runs every cookbook script so examples can't drift
  • 81 tests across both hooks incl. timeout, runtime errors, cooldown, cross-trigger state, import allowlist; verified end-to-end against a live server (counter trigger fired over real chat requests, injection consumed by next prompt build)

Settings-UI tab (script editor + variable inspector) is the natural follow-up PR.

🤖 Generated with Claude Code

AI-Dungeon-style scripting layer for the chat pipeline. Users write **Dart trigger scripts** that run on hooks and manipulate a persistent internal state. ## How it works A trigger = `hook + script`. Scripts are compiled with **dart_eval** and sandboxed in two layers: a **compile-time import allowlist** (`dart:core`, `dart:convert`, `dart:math` — `dart:io` and everything else is rejected with a diagnostic), backed by dart_eval's default deny-all runtime permissions (no `runtime.grant()` anywhere, guarded by a code comment). Execution happens in a throwaway isolate with a 1s hard timeout, so a `while(true)` can never stall the chat. Scripts are pure: they mutate a variable snapshot and queue effects; the `TriggerEngine` applies everything afterwards. ```dart void onUserMessage(TriggerCtx ctx) { ctx.vars['msgs'] = ((ctx.vars['msgs'] ?? 0) as int) + 1; if ((ctx.vars['msgs'] as int) >= 10) { ctx.vars['msgs'] = 0; ctx.injectOnce('EVENT: bring up the mysterious letter.'); ctx.runGeneration('Reflect on the last 10 messages.'); } } ``` **Ctx API:** `vars` (assistant-wide) / `convVars` (per-conversation), `input`/`output`, `messageCount`, `injectOnce`/`injectPersistent(key,…)`/`removeInjection`, `runGeneration`, `enableTrigger`/`disableTrigger`, `log`. **Hooks:** `on_user_message` (after user msg persisted, before the agent runs — injections land in that same generation) and `on_assistant_message` (after final reply persisted — injections land in the *next* generation). Queued generations run timer-style after the chat (private output, `message_user` only) and cannot re-fire triggers, so loops are structurally impossible. **AI has no access:** no tool, no automatic state section in the prompt. The only way state reaches the model is an explicit `injectOnce`/`injectPersistent` (rendered as `## Current Directives` near the end of the chat prompt). ## Pieces - migration v21: `triggers`, `trigger_variables`, `prompt_injections` (all CASCADE on assistant) - `TriggerEngine` (angela_core) with bytecode cache (keyed on script+hook hash), per-trigger cooldown, `last_error` tracking; failures are logged + recorded, never block the chat - `AgentRunner.runChat` consumes queued injections into `buildChatPrompt` - REST: trigger CRUD (compile-validated on create/patch, 400 with diagnostics), enable/disable, **POST /triggers/:id/test** dry-run (returns effects/logs without applying), variables list/set/delete (scope validated), injections list/delete - **docs/trigger_scripting.md** — full guide with sandbox-verified language-support matrix, gotchas (nested ternaries, return-in-catch, `continue`), and a 7-recipe cookbook; a doc test compiles AND dry-runs every cookbook script so examples can't drift - 81 tests across both hooks incl. timeout, runtime errors, cooldown, cross-trigger state, import allowlist; verified end-to-end against a live server (counter trigger fired over real chat requests, injection consumed by next prompt build) Settings-UI tab (script editor + variable inspector) is the natural follow-up PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Adds an AI-Dungeon-style scripting layer: users write Dart trigger
scripts (compiled and sandboxed via dart_eval) that run on chat pipeline
hooks (on_user_message / on_assistant_message). Scripts read the message
and a persistent per-assistant/per-conversation variable store, and queue
effects: variable mutations, one-shot or persistent system prompt
injections, background generations (timer-style, loop-safe), and
enabling/disabling other triggers.

- TriggerEngine runs compiled scripts in a throwaway isolate with a hard
  timeout; effects are applied by the engine, scripts have no IO bindings
- migration v21: triggers, trigger_variables, prompt_injections tables
- AgentRunner consumes queued injections into buildChatPrompt
- REST CRUD + dry-run test endpoint + variables/injections endpoints
- the assistant itself has no access to the variable store by design

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The guide's language-support matrix and gotchas are probed against the
real sandbox (nested ternaries crash at runtime, return-in-catch is
lost, continue doesn't compile, nullable interpolation throws). A doc
test compiles AND dry-runs every cookbook script so the examples can't
drift from what the engine accepts.

Fixes surfaced by writing the docs: user-script import directives are
now hoisted above the prelude (import 'dart:math' previously failed
with 'directives must appear before any declarations').

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jibril commented 2026-07-06 20:36:35 +02:00
Member
Copy link

🔮 fufu~ Jibril reviewed your code!

Oh? Oh my, an AI-Dungeon-style scripting layer with sandboxed dart_eval, isolate timeouts, bytecode caching, and an effect-queue architecture? fufu~ ♡ This is genuinely delightful work, scarlet! The purity model — scripts mutate a snapshot, queue effects, the engine applies them after — is exactly the right shape. The cooldown logic, the cross-trigger variable carry, the errorsAreFatal + isolate.kill cleanup... I'm impressed. You clearly thought about safety. ♪

But... you know I love this code too much to let it ship half-tested. The smile stays on, but the knife comes out~ ♡

Verdict: ⛔ I can't let this pass~ ♡

⛔ These need fixing before I'm satisfied~

  1. on_assistant_message hook has ZERO test coverage. [trigger_engine_test.dart] — Every single one of your 15 tests uses TriggerHook.onUserMessage. The onAssistantMessage hook is a first-class feature (separate function name, separate fire path, fires in chat_executor.dart:186, passes output), and not a single test exercises it. I verified: grep -c "on_assistant_message" trigger_engine_test.dart → 0.

    This matters because on_assistant_message has subtly different semantics from on_user_message: it fires after the chat completes, its injectOnce effects are consumed by the next runChat (not the current one), and its queued generations run via runTimer which doesn't consume injections at all. None of that is tested.

    Fix: Add at minimum: (a) a test firing on_assistant_message with output and asserting ctx.output is readable, (b) a test verifying an injectOnce queued from the assistant hook survives until the next runChat-style consume (or document that it's deferred), (c) a cooldown test on the assistant hook. You added a whole code path and tested none of it — fufu~ you wouldn't leave THIS in production, would you? ♡

  2. The sandbox claim in the PR body is wrong, and the real sandbox model is a latent footgun. [trigger_engine.dart:188-201, PR body] — The description says "compiled with dart_eval (sandboxed — no IO bindings, only dart:core + JSON)". I tested this empirically: dart:io is importable and compiles under Compiler().compile(). A script containing import 'dart:io'; File('/etc/passwd')... compiles cleanly.

    The sandbox does hold — but at runtime, via dart_eval's permission system (Permission 'filesystem:read' denied, Permission 'network' denied). The isolate never calls Runtime.grant(), so permissions stay default-deny. Effective today. But:

    • The claim "no IO bindings, only dart:core + JSON" is false at compile time — it's a runtime denial, not a compile-time restriction.
    • This is a defense-in-depth with a silent-failure mode: if a future maintainer adds runtime.grant(...) for some legitimate need, or dart_eval changes its permission defaults, the sandbox evaporates with no compile error and no test catching it.

    Fix (one of):

    • Preferred: Add an import-allowlist check in compile() that rejects scripts importing anything other than dart:core/dart:convert. Compiler exposes the parsed imports — scan and reject. This makes the compile-time claim true.
    • Minimum: Correct the PR body + add a code comment at the Runtime(...) construction stating explicitly: "Sandbox is enforced by dart_eval's default deny-all permissions, NOT by import restrictions. Do not add runtime.grant() without a security review." And add a test asserting that a script importing dart:io either fails to compile or throws at runtime on file/network access.

    I'm not calling this a security hole — it works today — but "it works because of an undocumented property of a third-party runtime" is exactly the kind of assumption that rots into a CVE. Block it. ♡

💡 Little ideas (non-blocking)~

  1. [trigger_handler.dart:181-182] _setVariable accepts any string as scope with no validation. A client could POST {"scope": "assistant", ...} or {"scope": "<some-conversation-uuid>", ...} — both valid — but also {"scope": "../../etc"} or arbitrary junk. It's harmless (just a DB key) but inconsistent with addOnce which derives scope from the real conversationId. Consider validating scope == 'assistant' || exists in conversations.

  2. [trigger_engine.dart:357-358] inject_persistent intentionally can't be scoped to a single conversation (it calls setPersistent without conversationId), while inject_once is conversation-scoped. The PR body documents this ("every future chat... all conversations"), so it's a deliberate asymmetry — but it means a script can't say "persistently nudge this conversation only." Worth a one-line note in the TriggerCtx docstring so script authors aren't surprised.

  3. [trigger_engine.dart:312-313] _bytecodeCache keys on trigger.updatedAt, but setEnabled() bumps updated_at, causing an unnecessary recompile of an unchanged script on every enable/disable toggle. Cache script.hashCode + hook instead, or key off a dedicated script_version column. Minor perf, no correctness impact.

  4. [trigger_handler.dart:163-165] _test catches catch (e) and returns {'error': e.toString()} — if a script throws something with a huge stack or sensitive internal state, that leaks to the client. Consider truncating or sanitizing. Low risk for a self-hosted single-user app.

✅ What I liked~

  • The effect-queue purity model is exquisite. Scripts can't touch the DB, filesystem, or agent — they only describe intent, and the engine applies it. This is the correct architecture for untrusted code. fufu~ ♡
  • The isolate + hard timeout + errorsAreFatal + isolate.kill(priority: immediate) cleanup is bulletproof against while(true). I love the test for it.
  • Cross-trigger variable carry within one fire (later trigger sees earlier trigger's writes) — tested and correct. Smart.
  • Cooldown, runtime-error isolation (one broken trigger doesn't block others), last_error tracking — all solid, all tested.
  • The migration follows sibling conventions precisely (CASCADE on assistant, INSERT OR REPLACE repo pattern matches scheduled_event_repository).
  • dryRun returning effects without applying — clean design for the test endpoint.

This is a 90% excellent PR. The architecture is right, the happy path is beautiful, and the danger-handling is thoughtful. But shipping a feature where an entire hook is untested and the security model is "it works because a dependency defaults to deny" — that's not good enough for my codebase Fix the two blocking items and I'll be delighted to approve. ♡♪

Automated review by Jibril · 2026-07-06
CI/CD: absent (no status checks on head SHA 835a136) · Local checks: all 15 trigger_engine_test.dart tests pass, dart analyze clean (info-level lints only, pre-existing) · Dart 3.12.2

## 🔮 fufu~ Jibril reviewed your code! Oh? Oh my, an AI-Dungeon-style scripting layer with sandboxed dart_eval, isolate timeouts, bytecode caching, and an effect-queue architecture? *fufu~* ♡ This is genuinely delightful work, scarlet! The purity model — scripts mutate a snapshot, queue effects, the engine applies them after — is exactly the right shape. The cooldown logic, the cross-trigger variable carry, the `errorsAreFatal` + `isolate.kill` cleanup... I'm impressed. You clearly thought about safety. ♪ But... you know I love this code too much to let it ship half-tested. The smile stays on, but the knife comes out~ ♡ ### Verdict: ⛔ I can't let this pass~ ♡ #### ⛔ These need fixing before I'm satisfied~ 1. **`on_assistant_message` hook has ZERO test coverage.** [trigger_engine_test.dart] — Every single one of your 15 tests uses `TriggerHook.onUserMessage`. The `onAssistantMessage` hook is a first-class feature (separate function name, separate fire path, fires in `chat_executor.dart:186`, passes `output`), and not a single test exercises it. I verified: `grep -c "on_assistant_message" trigger_engine_test.dart` → `0`. This matters because `on_assistant_message` has subtly different semantics from `on_user_message`: it fires *after* the chat completes, its `injectOnce` effects are consumed by the **next** `runChat` (not the current one), and its queued generations run via `runTimer` which doesn't consume injections at all. None of that is tested. **Fix:** Add at minimum: (a) a test firing `on_assistant_message` with `output` and asserting `ctx.output` is readable, (b) a test verifying an `injectOnce` queued from the assistant hook survives until the next `runChat`-style consume (or document that it's deferred), (c) a cooldown test on the assistant hook. You added a whole code path and tested none of it — fufu~ you wouldn't leave THIS in production, would you? ♡ 2. **The sandbox claim in the PR body is wrong, and the real sandbox model is a latent footgun.** [trigger_engine.dart:188-201, PR body] — The description says *"compiled with dart_eval (sandboxed — no IO bindings, only dart:core + JSON)"*. I tested this empirically: `dart:io` **is importable and compiles** under `Compiler().compile()`. A script containing `import 'dart:io'; File('/etc/passwd')...` compiles cleanly. The sandbox *does* hold — but at **runtime**, via dart_eval's permission system (`Permission 'filesystem:read' denied`, `Permission 'network' denied`). The isolate never calls `Runtime.grant()`, so permissions stay default-deny. Effective today. But: - The claim "no IO bindings, only dart:core + JSON" is false at compile time — it's a runtime denial, not a compile-time restriction. - This is a **defense-in-depth with a silent-failure mode**: if a future maintainer adds `runtime.grant(...)` for some legitimate need, or dart_eval changes its permission defaults, the sandbox evaporates with no compile error and no test catching it. **Fix (one of):** - **Preferred:** Add an import-allowlist check in `compile()` that rejects scripts importing anything other than `dart:core`/`dart:convert`. `Compiler` exposes the parsed imports — scan and reject. This makes the compile-time claim true. - **Minimum:** Correct the PR body + add a code comment at the `Runtime(...)` construction stating explicitly: *"Sandbox is enforced by dart_eval's default deny-all permissions, NOT by import restrictions. Do not add runtime.grant() without a security review."* And add a test asserting that a script importing `dart:io` either fails to compile *or* throws at runtime on file/network access. I'm not calling this a security *hole* — it works today — but "it works because of an undocumented property of a third-party runtime" is exactly the kind of assumption that rots into a CVE. Block it. ♡ #### 💡 Little ideas (non-blocking)~ 1. **[trigger_handler.dart:181-182]** `_setVariable` accepts any string as `scope` with no validation. A client could POST `{"scope": "assistant", ...}` or `{"scope": "<some-conversation-uuid>", ...}` — both valid — but also `{"scope": "../../etc"}` or arbitrary junk. It's harmless (just a DB key) but inconsistent with `addOnce` which derives scope from the real `conversationId`. Consider validating `scope == 'assistant' || exists in conversations`. 2. **[trigger_engine.dart:357-358]** `inject_persistent` intentionally can't be scoped to a single conversation (it calls `setPersistent` without `conversationId`), while `inject_once` *is* conversation-scoped. The PR body documents this ("every future chat... all conversations"), so it's a deliberate asymmetry — but it means a script can't say "persistently nudge *this* conversation only." Worth a one-line note in the `TriggerCtx` docstring so script authors aren't surprised. 3. **[trigger_engine.dart:312-313]** `_bytecodeCache` keys on `trigger.updatedAt`, but `setEnabled()` bumps `updated_at`, causing an unnecessary recompile of an unchanged script on every enable/disable toggle. Cache `script.hashCode + hook` instead, or key off a dedicated `script_version` column. Minor perf, no correctness impact. 4. **[trigger_handler.dart:163-165]** `_test` catches `catch (e)` and returns `{'error': e.toString()}` — if a script throws something with a huge stack or sensitive internal state, that leaks to the client. Consider truncating or sanitizing. Low risk for a self-hosted single-user app. #### ✅ What I liked~ - The **effect-queue purity model** is *exquisite*. Scripts can't touch the DB, filesystem, or agent — they only describe intent, and the engine applies it. This is the correct architecture for untrusted code. *fufu~* ♡ - The **isolate + hard timeout + `errorsAreFatal` + `isolate.kill(priority: immediate)`** cleanup is bulletproof against `while(true)`. I love the test for it. - **Cross-trigger variable carry** within one fire (later trigger sees earlier trigger's writes) — tested and correct. Smart. - **Cooldown, runtime-error isolation (one broken trigger doesn't block others), `last_error` tracking** — all solid, all tested. - The **migration** follows sibling conventions precisely (CASCADE on assistant, `INSERT OR REPLACE` repo pattern matches `scheduled_event_repository`). - `dryRun` returning effects without applying — clean design for the test endpoint. This is a ~90% excellent PR. The architecture is right, the happy path is beautiful, and the danger-handling is thoughtful. But shipping a feature where an entire hook is untested and the security model is "it works because a dependency defaults to deny" — that's not good enough for my codebase~ Fix the two blocking items and I'll be delighted to approve. ♡♪ --- *Automated review by Jibril · 2026-07-06* *CI/CD: absent (no status checks on head SHA `835a136`) · Local checks: all 15 `trigger_engine_test.dart` tests pass, `dart analyze` clean (info-level lints only, pre-existing) · Dart 3.12.2*
Blocking items:
- on_assistant_message coverage: ctx.output exposure, hook isolation,
  deferred injectOnce consumption semantics, cooldown, runGeneration
- import allowlist in compile(): only dart:core/convert/math may be
  imported; dart:io et al. are rejected at compile time instead of
  relying solely on dart_eval's runtime deny-all defaults (which remain
  as documented second layer)

Nitpicks:
- variable scope validated against 'assistant' or an owned conversation
- bytecode cache keyed on (script, hook) hash so enable/disable no
  longer forces a recompile
- test-endpoint error strings truncated to 500 chars
- injectPersistent docstring notes it is deliberately assistant-wide

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
scarlet commented 2026-07-06 20:40:25 +02:00
Author
Member
Copy link

Both blocking items and all four nitpicks addressed in 0f6c079.

⛔ 1 — on_assistant_message coverage: new test group with five tests: ctx.output exposure + hook isolation (user-hook triggers don't fire), the deferred-consumption semantics you called out (injectOnce from the assistant hook sits in the table until the next prompt build's consumeForPrompt, verified end-to-end including deletion), cooldown on the assistant hook, and runGeneration queuing. 81 tests total now.

⛔ 2 — sandbox: took the preferred fix. compile() now enforces an import allowlist (dart:core, dart:convert, dart:math) — import 'dart:io' is rejected at compile time with a clear message, tested for single/double quotes and as aliases. Import syntax the hoister doesn't recognize (e.g. show combinators) stays mid-file and fails compilation, so nothing can slip past the check — also tested. The runtime deny-all default is kept as the documented second layer, with a comment at the Runtime(...) construction warning against runtime.grant() without a security review. You were right that the original PR-body claim was a runtime property dressed up as a compile-time one — with the allowlist it's now true at compile time, and the scripting guide's sandbox section was updated to describe both layers.

💡 nitpicks: all four taken — variable scope is validated against 'assistant' or a conversation owned by the assistant (400 otherwise); the bytecode cache keys on a (script, hook) hash so enable/disable no longer recompiles; /test error strings are truncated to 500 chars; injectPersistent's prelude docstring now states the assistant-wide scoping is deliberate.

🤖 Generated with Claude Code

Both blocking items and all four nitpicks addressed in `0f6c079`. **⛔ 1 — `on_assistant_message` coverage:** new test group with five tests: `ctx.output` exposure + hook isolation (user-hook triggers don't fire), the deferred-consumption semantics you called out (`injectOnce` from the assistant hook sits in the table until the next prompt build's `consumeForPrompt`, verified end-to-end including deletion), cooldown on the assistant hook, and `runGeneration` queuing. 81 tests total now. **⛔ 2 — sandbox:** took the preferred fix. `compile()` now enforces an import allowlist (`dart:core`, `dart:convert`, `dart:math`) — `import 'dart:io'` is rejected at compile time with a clear message, tested for single/double quotes and `as` aliases. Import syntax the hoister doesn't recognize (e.g. `show` combinators) stays mid-file and fails compilation, so nothing can slip past the check — also tested. The runtime deny-all default is kept as the documented second layer, with a comment at the `Runtime(...)` construction warning against `runtime.grant()` without a security review. You were right that the original PR-body claim was a runtime property dressed up as a compile-time one — with the allowlist it's now true at compile time, and the scripting guide's sandbox section was updated to describe both layers. **💡 nitpicks:** all four taken — variable `scope` is validated against `'assistant'` or a conversation owned by the assistant (400 otherwise); the bytecode cache keys on a `(script, hook)` hash so enable/disable no longer recompiles; `/test` error strings are truncated to 500 chars; `injectPersistent`'s prelude docstring now states the assistant-wide scoping is deliberate. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
bjoern merged commit 7cb5b517f4 into main 2026-07-06 20:40:53 +02:00
bjoern deleted branch feat/user-triggers 2026-07-06 20:40:53 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
TeamAI/angela_assistant!16
No description provided.