Skip to content

feat(vigil): docs and e2e fixtures (slice 5 of 5) - #820

Draft
allen-munsch wants to merge 4 commits into
vigil/04-plugin-bridgefrom
vigil/05-docs-e2e
Draft

feat(vigil): docs and e2e fixtures (slice 5 of 5)#820
allen-munsch wants to merge 4 commits into
vigil/04-plugin-bridgefrom
vigil/05-docs-e2e

Conversation

@allen-munsch

Copy link
Copy Markdown
Collaborator

Summary

Slice 5 of 5 in the vigil decomposition: documentation and the e2e fixture harness. No source changes — this slice is docs and test scaffolding only, so the CI matrix is unchanged from slice 4.

What's in

  • docs/proposals/vigil.md — the feature proposal (trigger taxonomy, rite gate, observance, the toll/watcher/harbinger trigger types).
  • docs/vigils/* — usage, use-cases, and a loop/MCP comparison.
  • tests/fixtures/vigil/* — podman-compose harness with Jenkins/Prefect/Airflow echo services, poller plugins, sanity-check JSON, setup scripts, and a watch-dir trigger fixture.

What's deliberately not here

The feature/vigil-phase5 branch this was ported from sits on pre-0.24.1 main, so its remaining diff against slice 4 is dominated by version drift that is not vigil work and is correctly excluded here:

  • /effort reasoning-effort support (src/config/mod.rs, src/cli.rs, src/plugin/mod.rs, src/agent/agent_loop/*).
  • ThinkingLevel::Max and the max_tokens budget fix.
  • The ServiceError boxing in src/provider/adapter.rs.
  • Cargo.toml/Cargo.lock/nix/bin.nix/CHANGELOG.md version bump (0.24.1 vs 0.24.0).

Two vigil-adjacent divergences from phase5 are also left out, both worth a deliberate decision rather than slipping into a docs PR:

  1. vigil is not in default features. phase5 adds 'vigil' to the default feature set; slices 1–4 kept it opt-in (vigil = ["dep:notify"]). Flipping it would pull notify into every default build and change what windows-default compiles, so it needs a separate call.
  2. Obsolete vigil tests remain. phase5 deletes src/tests/vigil_tests.rs and the test module in src/extras/vigil_db.rs (superseded by the keeper-level tests in src/extras/vigil/tests.rs); slices 1–4 still carry them. A follow-up cleanup PR should remove them.

Verification

  • cargo fmt --all --check — clean.
  • All 26 files are new (no Rust, no tracked-file modification); the only working-tree untracked files are the two pre-existing DAP fixture binaries, left unstaged.
  • Fixture plugins reference only symbols that exist after slices 1–4: harness/json-decode, harness/notify, harness/register-command, vigil/emit.

Land the vigil documentation and the podman-compose e2e fixture harness.
Docs: the feature proposal plus usage, use-cases, and a loop/MCP
comparison under docs/vigils/. Fixtures: Jenkins/Prefect/Airflow echo
services, poller plugins, sanity-check JSON, and setup scripts that drive
the toll/watcher/harbinger triggers end to end.

No source changes — the slice is documentation and test scaffolding only,
so the CI matrix is unaffected.
nextest runs each test in its own process; two trace-test processes can
stamp the same wall-clock nanosecond and share one sink file, so one
truncates the other's write and traced() reads back empty. Add the pid
to the filename so parallel processes never collide.
Vendor evil-janet 1.37.2 with a one-line patch in janet_collect that
marks janet_vm.top_dyns. Upstream Janet never roots that table, so
dirge — which populates it via janet_setdyn() with no active fiber —
gets it swept and then use-after-freed in janet_dyn("err-color")
inside janet_stacktrace_ext (drg-76a1).

Also root the half-built arrays/tables/tuples in the plugin worker's
json_to_janet paths, which were collectible mid-fill by the same GC.
Two interleaving fixes so a vigil event that lands while the agent is
thinking is queued for the next idle window rather than lost:

- Reaper: when an observance is already running, send the drained
  events back into the vigil's own channel for the next reap window
  instead of dropping them.
- UI: gate the vigil wake arm on !ui.is_running so a mid-turn wake
  token is not consumed — leaving the token queued lets the arm re-fire
  once the agent goes idle and drain the observance then.
@allen-munsch

allen-munsch commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Quick note on the one-line evil-janet change, since the vendored diff is large and mostly just the vendored source:

The bug: Janet's garbage collector never marks janet_vm.top_dyns — the table that holds dynamic bindings when no fiber is active. dirge hits this because it calls janet_setdyn outside any fiber, so those bindings land in top_dyns. On the next GC the table gets swept, and a later janet_dyn (via the error printer) dereferences freed memory — the SIGSEGV.

The fix is one line in vendor/evil-janet/csrc/janet.c, in janet_collect, added right after the root fiber is marked:

if (janet_vm.top_dyns) janet_mark_table(janet_vm.top_dyns);

That tells the collector to treat the table as a GC root, exactly how every other VM-global GC pointer in that function is already handled. Upstream Janet (1.37.2 through 1.41.3 and current master) has the same missing root, which is why this had to be a vendored patch rather than a version bump.

edit: I'm not sure if this should be vendored, or if we should attempt a PR to upstream

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