Background
snapshot is a builtin hook in pkg/hooks/builtins/snapshot.go that records shadow-git checkpoints at six different events: session_start, turn_start, turn_end, pre_tool_use, post_tool_use, and session_end. The implementation lives in pkg/hooks/builtins, but it is by far the leakiest builtin in terms of API surface.
Current coupling
- The builtin keeps per-session state (shadow-git repo + checkpoint history) on
*State.snapshot.
- The runtime exposes four public methods on
*LocalRuntime that simply delegate into that state:
LocalRuntime.SnapshotsEnabled()
LocalRuntime.UndoLastSnapshot(...)
LocalRuntime.ListSnapshots(...)
LocalRuntime.ResetSnapshot(...)
- The TUI's
/undo and /snapshots commands call those runtime methods.
WithSnapshots(bool) is a runtime.Opt that flips a runtime field (r.snapshotsEnabled), which is then read by buildHooksExecutors to auto-inject the six hook entries.
The result: the builtin has two parallel APIs — the hook protocol (six events) and a Go method API (four runtime methods). The runtime knows snapshots exist; the TUI talks to a builtin through the runtime; configuration of an auto-injection flag lives on a runtime opt instead of with the builtin.
Isolation grade
C — implementation is isolated, but coupling flows backward (builtin → runtime → TUI) through Go method calls outside the hook protocol.
Proposed fixes
1. Extract a SnapshotController and have the TUI talk to it directly
Introduce something like:
// pkg/hooks/builtins/snapshot.go
type SnapshotController interface {
UndoLast(ctx context.Context, sessionID, cwd string) (files int, ok bool, err error)
List(sessionID string) []SnapshotInfo
Reset(ctx context.Context, sessionID, cwd string, keep int) (files int, ok bool, err error)
Enabled() bool
}
The snapshot builtin returns this controller from a registration helper (e.g. builtins.RegisterSnapshot(r, opts) (SnapshotController, error)). The TUI obtains it the same way it gets a hook executor today — through whatever wiring the embedder (CLI/TUI) already does — instead of asking the runtime.
Outcome:
LocalRuntime.SnapshotsEnabled / UndoLastSnapshot / ListSnapshots / ResetSnapshot go away.
LocalRuntime.snapshotsEnabled, LocalRuntime.snapshotCwd, runtime/snapshot.go go away.
- The runtime stops knowing snapshots exist.
2. Move auto-injection metadata next to the builtin
ApplyAgentDefaults currently switch-cases each builtin (AddDate, AddEnvironmentInfo, Snapshot, …) to decide which events to wire and from which flag. This central table grows every time we add a builtin.
Replace it with per-builtin defaulting. Each builtin declares its own auto-injection rule:
// e.g. on registration
type AutoInject struct {
Predicate func(AgentDefaults) bool
Events []EventType
Hook Hook
}
ApplyAgentDefaults becomes a generic loop over registered builtins, asking each whether it wants to inject itself for the supplied AgentDefaults. The snapshot's six-event injection lives in the snapshot file, not in builtins.go.
3. Move the WithSnapshots opt out of runtime.Opt
If (1) and (2) land, the snapshots-on/off boolean is a property of the snapshot subsystem, not of the runtime. The CLI/TUI can pass it directly when constructing the SnapshotController, and runtime.WithSnapshots is deleted. The runtime is then unaware of the snapshot feature in any way.
Recommendation
Land (1) first — it removes the four passthrough methods on *LocalRuntime and makes the boundary clean — then (2) as a follow-up to remove the central ApplyAgentDefaults table for the rest of the builtins, and (3) as the final cleanup once nothing in the runtime references snapshot state.
Acceptance criteria
Background
snapshotis a builtin hook inpkg/hooks/builtins/snapshot.gothat records shadow-git checkpoints at six different events:session_start,turn_start,turn_end,pre_tool_use,post_tool_use, andsession_end. The implementation lives inpkg/hooks/builtins, but it is by far the leakiest builtin in terms of API surface.Current coupling
*State.snapshot.*LocalRuntimethat simply delegate into that state:LocalRuntime.SnapshotsEnabled()LocalRuntime.UndoLastSnapshot(...)LocalRuntime.ListSnapshots(...)LocalRuntime.ResetSnapshot(...)/undoand/snapshotscommands call those runtime methods.WithSnapshots(bool)is aruntime.Optthat flips a runtime field (r.snapshotsEnabled), which is then read bybuildHooksExecutorsto auto-inject the six hook entries.The result: the builtin has two parallel APIs — the hook protocol (six events) and a Go method API (four runtime methods). The runtime knows snapshots exist; the TUI talks to a builtin through the runtime; configuration of an auto-injection flag lives on a runtime opt instead of with the builtin.
Isolation grade
C — implementation is isolated, but coupling flows backward (builtin → runtime → TUI) through Go method calls outside the hook protocol.
Proposed fixes
1. Extract a
SnapshotControllerand have the TUI talk to it directlyIntroduce something like:
The snapshot builtin returns this controller from a registration helper (e.g.
builtins.RegisterSnapshot(r, opts) (SnapshotController, error)). The TUI obtains it the same way it gets a hook executor today — through whatever wiring the embedder (CLI/TUI) already does — instead of asking the runtime.Outcome:
LocalRuntime.SnapshotsEnabled / UndoLastSnapshot / ListSnapshots / ResetSnapshotgo away.LocalRuntime.snapshotsEnabled,LocalRuntime.snapshotCwd,runtime/snapshot.gogo away.2. Move auto-injection metadata next to the builtin
ApplyAgentDefaultscurrently switch-cases each builtin (AddDate,AddEnvironmentInfo,Snapshot, …) to decide which events to wire and from which flag. This central table grows every time we add a builtin.Replace it with per-builtin defaulting. Each builtin declares its own auto-injection rule:
ApplyAgentDefaultsbecomes a generic loop over registered builtins, asking each whether it wants to inject itself for the suppliedAgentDefaults. The snapshot's six-event injection lives in the snapshot file, not inbuiltins.go.3. Move the
WithSnapshotsopt out ofruntime.OptIf (1) and (2) land, the snapshots-on/off boolean is a property of the snapshot subsystem, not of the runtime. The CLI/TUI can pass it directly when constructing the
SnapshotController, andruntime.WithSnapshotsis deleted. The runtime is then unaware of the snapshot feature in any way.Recommendation
Land (1) first — it removes the four passthrough methods on
*LocalRuntimeand makes the boundary clean — then (2) as a follow-up to remove the centralApplyAgentDefaultstable for the rest of the builtins, and (3) as the final cleanup once nothing in the runtime references snapshot state.Acceptance criteria
pkg/runtime/snapshot.gois gone (or contains only a small wiring shim).LocalRuntimeexposes no snapshot-specific methods./undo,/snapshots,/reset) drive the snapshot controller directly without going through the runtime.WithSnapshotsis removed fromruntime.Opt(configuration moves to the controller).