Skip to content

Add a drop hook to the source guardrail - #512

Merged
pufit merged 2 commits into
ClickHouse:mainfrom
polyglotAI-bot:polyglot/source-drop-hook
Oct 20, 2026
Merged

pufit merged 2 commits into
ClickHouse:mainfrom
polyglotAI-bot:polyglot/source-drop-hook

Conversation

@polyglotAI-bot

@polyglotAI-bot polyglotAI-bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds an optional drop hook to the source guardrail. sync.github.drop_hook is an argv list. When the guardrail drops records from a batch, Nerve runs the command once and writes one JSON object per dropped record to its stdin.

A guardrail drop is silent by design: the record is never persisted, and the agent never sees it. That is right for the agent. But nothing can respond to the person whose message was dropped, and the only trace is a log line, capped at 10 per batch. With a hook, an operator can act on a drop without sending its content to an agent. For example, a worker that only takes @-mentions from an allowlist can react to a dropped mention, so the commenter knows it was received but not acted on. Or the operator can keep an audit trail.

Behavior

  • Payload: source, id, record_type, summary, timestamp, metadata, and dropped_on (the field of the rule that fired). content is not sent: the guardrail exists to keep untrusted content away from automation, and the identifiers and metadata are enough to act on a drop.
  • No shell. argv exec only. A bare string is one program path (the existing _str_list / @_coerced convention). Entries pass as they are, so argument positions hold.
  • Best effort. A hook that is missing, can't start, exits non-zero, or runs longer than 60 s causes a warning, with the stderr tail. It never fails a sync and never holds back the cursor: spawn and payload errors are caught, and the call site is guarded.
  • Process group. The hook runs in a new session. On timeout or cancellation, Nerve kills the whole process group (killpg). That happens even when the hook already exited and a child it started still holds stdin. Reaping waits at most 5 s. stderr goes to a temporary file, not a pipe, so a background job the hook leaves behind can't stall the sync.
  • At least one attempt, no retry. The hook runs after persist (and condense), before the cursor advances. A failed persist re-fetches the batch, and the hook gets the drops then. A hook that fails is not retried. The docs say so, and ask for idempotent hooks.
  • Once per (id, timestamp). Since GitHub source: read every notifications page instead of only the first #491, the GitHub source queries since=<cursor> inclusively, so a dropped thread in the cursor's own second is listed and dropped again on every run. The runner remembers the last 4096 (id, timestamp) pairs it passed to the hook (in process) and does not pass them again. New activity on a thread changes updated_at, so it reaches the hook. After a restart, a source reload, or a CLI nerve sync, the hook can get a record once more.
  • Default off. With drop_hook unset, behavior does not change. At startup, a wrong-shaped value or an empty program name turns the hook off with a warning. A program that isn't executable, or a hook with no active guardrail, logs a warning.
  • Wired for the GitHub notifications source. SourceRunner(drop_hook=...) is source-agnostic, so another source can take it with one argument (documented under "Extending to other sources").

Changes

  • nerve/sources/runner.py: drop_hook parameter and validation, _run_drop_hook() / _kill_drop_hook(), the (id, timestamp) memory, and the guarded call before the cursor advances.
  • nerve/config.py: GitHubSyncConfig.drop_hook.
  • nerve/sources/registry.py: pass the hook to the GitHub runner, log the validated argv, and warn when the program is missing or no guardrail is set.
  • docs/sources.md: new "Drop hook" section, plus the pipeline diagram and config block. The guardrail paragraph no longer says that dropped records are not fetched again.
  • tests/test_source_drop_hook.py: 32 tests with real subprocesses:
    • Payload: only dropped records, dropped_on, no content.
    • When it runs: after persist and before the cursor advances (spy); not when nothing is dropped, when persist fails, or with an inactive guardrail.
    • Listed again: a dropped record fetched twice reaches the hook once, and again with a new timestamp; a failed hook is not retried; a repeat in one batch; the memory bound; an end-to-end run through GitHubSource with a fake gh (second since = cursor, one hook call).
    • Failures never fail the sync: failing, missing, stdin-ignoring and slow hooks; spawn ValueError; payload error.
    • Process-group kill: a child that outlives the timeout and a child that holds stdin after the hook exited are both dead afterwards (pid-file checks); a background job left by the hook doesn't stall the sync.
    • Config: argv kept as is, empty values mean off, wrong shapes are disabled.
    • Registry: wiring and the startup warnings.

Test plan

  • pytest tests/test_source_drop_hook.py: 32 passed, also with -W error::ResourceWarning. 0 failures in 10 serial runs on the rebased branch (and in 40 serial and 20 4-way parallel runs before the rebase).
  • The 5 "listed again" tests fail without the (id, timestamp) memory.
  • Mutation checks: replacing killpg with a leader-only kill() fails 2 tests. Keying the kill on the hook's exit code instead of on communicate() finishing fails the stdin-holder test.
  • Source / config / reload test files (904 tests): pass.
  • Full suite on the final commit, rebased on main 7cfcb15 (with GitHub source: read every notifications page instead of only the first #491): 6547 passed, 39 skipped, 3 failed. The 3 failures (test_setup_api::TestSecretsAndLogs::test_log_lines_carry_no_values, test_slack_channel::TestDispatchBounds × 2) fail the same way on a clean main. test_slack_integration.py was skipped: it needs aiohttp, which the dev environment does not have.
  • Two rounds of independent subagent review before opening. Round 1 found the blocking issue (a 60 s timeout that did not reach the hook's children, so a shell hook could hold the runner lock indefinitely) and several should-fix items. All are fixed. Round 2 found no blocking issues; its remaining items (the kill condition, a test that did not check what it claimed) are fixed too.

🤖 Generated with Claude Code

@pufit

pufit commented Oct 20, 2026

Copy link
Copy Markdown
Member

@polyglotAI-bot please check this PR #491
Probably we should adjust your PR now

@nerveai

nerveai commented Oct 20, 2026 •

Copy link
Copy Markdown

Nerve PR review · ✅ Ready to merge (once CI runs)

Updated for bb2f3da: this resolves the earlier point. The PR is rebased onto #491. The runner now passes each dropped (id, timestamp) to the hook only once (nerve/sources/runner.py:449-462, bounded at 4096 pairs). So GitHub's inclusive since no longer re-fires the hook every 15 min. New activity on a thread still reaches it, and a failed attempt is still not retried. docs/sources.md:579 now describes the redelivery cases (restart, a reload, a CLI nerve sync, a crash), and the guardrail paragraph no longer claims drops are never re-fetched.

This adds an opt-in argv hook that receives the records the guardrail dropped (metadata only, no content), so an allowlist worker can audit or acknowledge them. It is source-agnostic, bounded, and never holds back the cursor. The value is narrow but real for worker deployments.

Completeness: 28 tests against real subprocesses, including an end-to-end GitHubSource re-listing case. Docs and the config row are in place. No migration. It merges cleanly with main 7cfcb15.

Worth considering:

Checked: full suite in an offline sandbox at bb2f3da, in 3 chunks: 6551 passed, 38 skipped (test_slack_integration.py excluded; it hangs offline on main too). Read the runner, the registry and the GitHub cursor path. CI has not run yet (fork approval pending) · automated review by nerveai — mention @nerveai to ask a follow-up.

polyglotAI-bot and others added 2 commits October 20, 2026 19:34
A guardrail drop is silent by design: the record is never persisted and the
agent never sees it. That leaves no way to answer the person whose message
was dropped, and the only trace is a log line capped at 10 per batch.

`sync.github.drop_hook` is an argv list. When the guardrail drops records,
SourceRunner runs it once per batch with one JSON object per dropped record
on stdin: source, id, record_type, summary, timestamp, metadata and
dropped_on (the field of the rule that fired). The content is left out.

The hook is best effort and never fails a sync or holds back the cursor:

- argv exec only, in a new session; stdout discarded, stderr to a temp file
  so a leftover background job cannot hold a pipe;
- 60 s timeout, then the whole process group is killed, also when the hook
  exited but a child still holds its stdin, and on cancellation;
- any spawn or payload error is logged, and the call site is guarded;
- runs after persist and before the cursor advances, so a failed persist
  re-fetches the batch and the hook gets its drops then;
- a wrong-shaped value or an empty program name turns the hook off with a
  warning; startup warns when the program is not executable or when no
  guardrail is set.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since ClickHouse#491, the GitHub source queries since=<cursor> inclusively. Storage
skips a kept record that did not change, but a dropped record is never
stored. So the thread in the cursor's own second was dropped again on every
run, and the hook got it again every run until a newer notification came.

The runner now remembers the (id, timestamp) pairs it passed to the hook
(the last 4096, in process) and does not pass them again. New activity on a
thread changes updated_at, so it reaches the hook. A failed attempt is
remembered too: no retry, as documented.

Two large-payload tests repeated one batch 2000 times. They now use
distinct ids, so the payload still fills the pipe.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@polyglotAI-bot
polyglotAI-bot force-pushed the polyglot/source-drop-hook branch from 1b5ef14 to bb2f3da Compare October 20, 2026 19:57
@polyglotAI-bot

Copy link
Copy Markdown
Contributor Author

@pufit Thanks. I rebased onto main (with #491) and changed the hook in bb2f3da.

The problem. Since #491, since=<cursor> is inclusive. Storage skips a kept record that did not change, but a dropped record is never stored. So the guardrail dropped the thread in the cursor's own second again on every run, and the hook got it every 15 min until a newer notification arrived.

The change.

  • The runner remembers the (id, timestamp) pairs it passed to the hook (the last 4096, in process) and does not pass them again. New activity on a thread changes updated_at, so that still reaches the hook.
  • A failed attempt is remembered too, so "no retry" stays true.
  • The memory is not persisted. After a restart, a source reload, or a CLI nerve sync, the hook can get the newest dropped record once more. The docs say so, and hooks must still be idempotent. A DB table for this seemed too much for what it would prevent.
  • docs/sources.md: the guardrail paragraph said that dropped records are not fetched again. That stopped being true with GitHub source: read every notifications page instead of only the first #491, so I corrected it.

Tests. 5 new: a dropped record listed twice, then with a new timestamp; a failed hook that is not retried; a repeat in one batch; the memory bound; and an end-to-end run through GitHubSource with a fake gh (the second since is the cursor, and the hook runs once). All 5 fail without the change. Two large-payload tests repeated one batch 2000 times. They now use distinct ids, so the payload still fills the pipe. Full suite: 6547 passed, 3 failed. The same 3 fail on a clean main.

Not changed. The guardrail log line and records_dropped still count a re-listed drop on every run. That applies to every dropped record since #491, with or without a hook. I can fix it in a separate PR if you want.

#513 (from the nerveai review) is also mine. Whichever of the two merges second will add drop_hook to github_repo_events.

@pufit
pufit merged commit 3ed5b0c into ClickHouse:main Oct 20, 2026
4 checks passed
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.

3 participants