Generic per-volume hook foundation + on-change trigger#88
Merged
Conversation
Introduce a tool-agnostic, best-effort hook mechanism: per-volume, user-configured commands the agent exec's (without a shell) after a successful index run, passing context via SQUIRREL_* env vars and recording only the generic outcome (exit code, timestamps, triggering run). squirrel never learns what the command does. - config: [volumes.X.hook] with `command` (argv, no shell) and optional `timeout` (default 1h). The single command is distinguished across triggers by SQUIRREL_TRIGGER, leaving room for the interval trigger without an over-built rules engine. - hook package: pure library that exec's argv, sets the SQUIRREL_* contract, bounds the run with a timeout + WaitDelay (so a grandchild holding a pipe can't wedge the agent), and reports a generic Outcome. - store: hook_runs table (migration v10->v11, additive — no existing row rewritten) plus Begin/Finish/List. Records exit code, timestamps, the triggering run, and the SQUIRREL_CHANGED flag. - agent: hookRunner with a per-volume don't-stack guard and tracked goroutines drained on shutdown; the scheduler fires the on-change hook after every successful index run, best-effort and asynchronous so it never affects the run that triggered it. - visibility: `squirrel hooks` CLI listing and a TUI Hooks tab. Closes #85 Part of #84 https://claude.ai/code/session_016huzsmuzXRLEWJSCTMctEa
There was a problem hiding this comment.
Pull request overview
Introduces a generic, tool-agnostic per-volume hook mechanism that runs best-effort external commands after index runs, records their generic outcomes in the store, and surfaces history in both CLI and TUI.
Changes:
- Add hook configuration parsing/validation (
[volumes.<name>.hook]) with default timeout behavior. - Add hook execution library + agent runner to fire on-change hooks asynchronously (don’t-stack, timeout-bounded) and persist outcomes in a new
hook_runstable. - Add visibility via
squirrel hooksCLI and a new Hooks tab in the TUI.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tui/tui.go | Adds “Hooks” as a new top-level TUI tab and keybinding. |
| tui/hooks.go | New TUI screen that lists recorded hook runs and status. |
| store/migrations.go | Bumps schema to v11 and adds hook_runs table + index. |
| store/hookruns.go | Store layer for recording and listing hook run lifecycle. |
| store/hookruns_test.go | Store tests covering hook run insert/finish/list semantics. |
| README.md | Documents hook configuration, behavior, and env contract. |
| hook/hook.go | New hook execution library (no shell, env contract, timeout, bounded stderr). |
| hook/hook_test.go | Tests for hook execution outcomes, env contract, and bounding behavior. |
| config/config.go | Adds VolumeHook config model and TOML resolution/validation. |
| config/hook_test.go | Tests for hook config resolution, defaults, and errors. |
| cmd/squirrel/root.go | Registers the new hooks subcommand. |
| cmd/squirrel/hooks.go | New CLI listing for hook run history. |
| agent/scheduler.go | Wires on-change hook firing into successful index completion + shutdown drain. |
| agent/scheduler_test.go | Scheduler fixture updated to include hook runner. |
| agent/hooks.go | Implements hookRunner (don’t-stack guard, async exec, outcome recording). |
| agent/hooks_test.go | Tests runner behavior and tick-path firing/recording. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+191
to
+202
| func (b *boundedBuffer) Write(p []byte) (int, error) { | ||
| if room := b.limit - b.buf.Len(); room > 0 { | ||
| if len(p) <= room { | ||
| b.buf.Write(p) | ||
| } else { | ||
| b.buf.Write(p[:room]) | ||
| } | ||
| } | ||
| // Report the full length as written: we are intentionally discarding | ||
| // the overflow, not failing the command's write. | ||
| return len(p), nil | ||
| } |
Comment on lines
+100
to
+110
| func Run(ctx context.Context, spec Spec) Outcome { | ||
| out := Outcome{StartedAtNs: time.Now().UnixNano()} | ||
| if len(spec.Command) == 0 { | ||
| out.EndedAtNs = time.Now().UnixNano() | ||
| out.Err = errors.New("hook: empty command") | ||
| return out | ||
| } | ||
|
|
||
| runCtx, cancel := context.WithTimeout(ctx, spec.Timeout) | ||
| defer cancel() | ||
|
|
Comment on lines
+75
to
+82
| // BeginHookRun records the start of a hook invocation and returns its id. | ||
| // The trigger must be one of the HookTrigger* constants. The row begins | ||
| // in status 'running'; FinishHookRun moves it to a terminal state. A | ||
| // non-zero spec.TriggeringRunID is stored as the FK; zero stores NULL. | ||
| func (s *Store) BeginHookRun(ctx context.Context, spec HookRunSpec) (int64, error) { | ||
| if spec.Trigger != HookTriggerChange && spec.Trigger != HookTriggerInterval { | ||
| return 0, fmt.Errorf("BeginHookRun: trigger must be %q or %q, got %q", HookTriggerChange, HookTriggerInterval, spec.Trigger) | ||
| } |
| IndexEvery: time.Minute, | ||
| Hook: &config.VolumeHook{ | ||
| // Touch a marker so we can prove the command actually ran. | ||
| Command: []string{"sh", "-c", "echo x > " + marker}, |
- execute: drop the unused volumeID parameter (the goroutine in fire already owns done(volumeID)). - fire: keep trigger as a parameter so the foundation stays trigger-agnostic, suppressing unparam until the interval caller in #86 exercises the second value. https://claude.ai/code/session_016huzsmuzXRLEWJSCTMctEa
- hook.boundedBuffer now keeps the last `limit` bytes (the tail) instead of the first, so a verbose failing command's recorded stderr shows the context near the error, matching the field's documented intent. - hook.Run rejects a non-positive Timeout with a clear error instead of letting context.WithTimeout fire instantly and surface a phantom "timed out" outcome. - BeginHookRun enforces the trigger↔triggering-run coupling (change ⇒ has a run, interval ⇒ none), backed by a hook_runs CHECK that mirrors the runs table's kind↔destination idiom — a wiring bug now fails loudly instead of writing a misleading NULL run. - Test fixups: the scheduler change-hook test passes the marker path as an argument ($1) rather than concatenating it into the sh -c script. https://claude.ai/code/session_016huzsmuzXRLEWJSCTMctEa
mbertschler
pushed a commit
that referenced
this pull request
Jun 3, 2026
…ling Brings #88's review fixes into the interval branch (stderr tail capture, positive-timeout guard, the trigger↔triggering-run coupling) and updates TestLatestHookRun so its change-hook fixture carries a real triggering run as the new coupling now requires. https://claude.ai/code/session_016huzsmuzXRLEWJSCTMctEa
Resolves the store/migrations.go conflict: main introduced v11 (runs_audit) and v12 (peer_sync_state_history), so the hook_runs migration is renumbered to v13 (migrateV12ToV13) and SchemaVersion bumped to 13. Also reconciles a semantic conflict main shipped broken: c2fb4a4 added an allowRewind bool to Store.UpsertPeerSyncState while 298b9b6 added an agent/sync_test.go caller still using the old 4-arg form, so main did not compile. Pass allowRewind=false (a forward watermark write) to match every other caller. https://claude.ai/code/session_016huzsmuzXRLEWJSCTMctEa
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the tool-agnostic, best-effort hook foundation plus the on-change (event) trigger — the first sub-issue of the generic-hooks tracker. The interval ("check") trigger follows in a stacked PR (#86).
What this does
Per-volume, user-configured commands the agent runs to opportunistically nudge an external tool when a volume's content settles. squirrel never learns what the command does (kopia is just one consumer): it exec's the command without a shell, passes context via environment variables, and records only the generic outcome. A hook failure or timeout never fails or blocks the triggering run.
Config
A single command per volume (no rules engine). It's distinguished across triggers by
SQUIRREL_TRIGGER, so this shape already anticipates the interval trigger without over-building.The shared contract (both triggers will reuse it)
SQUIRREL_VOLUME,SQUIRREL_PATH,SQUIRREL_RUN_ID,SQUIRREL_CHANGED,SQUIRREL_TRIGGER.contexttimeout +cmd.WaitDelayso a hook that spawns a grandchild holding a pipe can't wedge the scheduler.On-change trigger
Fires after every successful index run (success or partial) the agent's scheduler completes — keying off content settled, not off a sync to a remote (a volume need not have any
sync_todestination). Per the issue's lean default it always fires on a completed run and passesSQUIRREL_CHANGED(additions, modifications, or disappearances) so the consumer can cheaply no-op.Outcome recording & visibility
New
hook_runstable (migration v10→v11, additive — no existing row is rewritten, preserving the content-immutability invariant). Records exit code, started/ended timestamps, the triggering run, and the changed flag. Surfaced via:squirrel hooks(new CLI listing)Layout
config/—[volumes.X.hook]parsing + validationhook/— pure exec library (env contract, timeout, bounded stderr capture, genericOutcome)store/—hook_runsschema +BeginHookRun/FinishHookRun/ListHookRunsagent/—hookRunner(don't-stack guard, spawn/reap, recording) wired into the scheduler's post-index pathcmd/squirrel/hooks.go,tui/hooks.go— visibilityTests cover config resolution, the store layer, the exec library (success / non-zero exit / timeout / spawn failure / env contract / parent cancellation), and the agent runner (success, failure recording, don't-stack, and the real scheduler tick firing the change hook).
go vet ./...clean; no new dependencies.Closes #85
Part of #84
Generated by Claude Code