Skip to content

refactor: extract SnapshotController so the runtime no longer brokers /undo#2707

Open
dgageot wants to merge 1 commit intodocker:mainfrom
dgageot:board/a8bb59ca7abad016
Open

refactor: extract SnapshotController so the runtime no longer brokers /undo#2707
dgageot wants to merge 1 commit intodocker:mainfrom
dgageot:board/a8bb59ca7abad016

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented May 7, 2026

Fixes #2701.

The runtime used to know about snapshots: it carried snapshotsEnabled, exposed UndoLastSnapshot / ListSnapshots / ResetSnapshot as methods on LocalRuntime, and the TUI commands reached snapshots through that runtime. This PR moves snapshots into a SnapshotController returned by builtins.RegisterSnapshot, threaded into both the runtime (as an AutoInjector) and the App.

What changed

  • pkg/hooks/builtins/snapshot.go \u2014 new SnapshotController interface (Enabled, UndoLast, List, Reset) and RegisterSnapshot(r, enabled) (SnapshotController, error) entry point that registers the snapshot builtin and returns a controller. The controller's AutoInject mounts the snapshot hook on session/turn boundaries when enabled.
  • pkg/hooks/builtins/builtins.go \u2014 added a generic AutoInjector interface; Register no longer wires the snapshot builtin and AgentDefaults.Snapshot is gone (the snapshot block in ApplyAgentDefaults moved onto the controller).
  • pkg/runtime/snapshot.go \u2014 deleted.
  • pkg/runtime/runtime.go / hooks.go \u2014 removed WithSnapshots, snapshotsEnabled, and the snapshot methods. New WithHooksRegistry lets the embedder ship a registry with snapshot pre-registered, and WithAutoInjector plumbs in any AutoInjector. buildHooksExecutors runs the registered injectors.
  • pkg/app/ \u2014 App gains a snapshotController via WithSnapshotController(c). UndoLastSnapshot / ListSnapshots / ResetSnapshot delegate directly to the controller, resolving cwd from the session (with os.Getwd fallback).
  • cmd/root/run.go \u2014 a small snapshotRuntimeOpts helper builds a registry, calls RegisterSnapshot, and returns the runtime opts plus the controller; buildAppOpts threads the controller into the App. The session spawner does the same so each spawned session has independent snapshot state.
  • lint/hook_builtins_registered.go \u2014 exempts snapshot.go from the "must be registered in builtins.go" check since it has its own RegisterSnapshot entry point.

Acceptance criteria

  • pkg/runtime/snapshot.go is gone.
  • LocalRuntime exposes no snapshot-specific methods.
  • TUI commands drive the controller directly (through the App, no longer through the runtime).
  • WithSnapshots is removed from runtime.Opt; configuration moves to the controller.

Validation

  • task build \u2014 ok
  • go test ./... \u2014 ok
  • task lint \u2014 0 issues

… /undo

Drops WithSnapshots in favour of builtins.RegisterSnapshot returning a
controller the embedder threads into both the runtime (as an
AutoInjector) and the App. LocalRuntime no longer carries snapshot
methods, and pkg/runtime/snapshot.go is gone.

Fixes docker#2701
@dgageot dgageot requested a review from a team as a code owner May 7, 2026 16:00
@aheritier aheritier added kind/refactor PR refactors code without behavior change area/agent For work that has to do with the general agent loop/agentic features of the app effort:medium Multiple files or components, some design decisions needed go Pull requests that update go code labels May 7, 2026
@docker-agent
Copy link
Copy Markdown

PR Review Failed — The review agent encountered an error and could not complete the review. View logs.

@docker-agent
Copy link
Copy Markdown

PR Review Failed — The review agent encountered an error and could not complete the review. View logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/agent For work that has to do with the general agent loop/agentic features of the app effort:medium Multiple files or components, some design decisions needed go Pull requests that update go code kind/refactor PR refactors code without behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(hooks): extract SnapshotController and stop the runtime from knowing snapshots exist

3 participants