refactor: extract SnapshotController so the runtime no longer brokers /undo#2707
Open
dgageot wants to merge 1 commit intodocker:mainfrom
Open
refactor: extract SnapshotController so the runtime no longer brokers /undo#2707dgageot wants to merge 1 commit intodocker:mainfrom
dgageot wants to merge 1 commit intodocker:mainfrom
Conversation
… /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
|
❌ PR Review Failed — The review agent encountered an error and could not complete the review. View logs. |
|
❌ PR Review Failed — The review agent encountered an error and could not complete the review. View logs. |
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.
Fixes #2701.
The runtime used to know about snapshots: it carried
snapshotsEnabled, exposedUndoLastSnapshot/ListSnapshots/ResetSnapshotas methods onLocalRuntime, and the TUI commands reached snapshots through that runtime. This PR moves snapshots into aSnapshotControllerreturned bybuiltins.RegisterSnapshot, threaded into both the runtime (as anAutoInjector) and the App.What changed
pkg/hooks/builtins/snapshot.go\u2014 newSnapshotControllerinterface (Enabled,UndoLast,List,Reset) andRegisterSnapshot(r, enabled) (SnapshotController, error)entry point that registers the snapshot builtin and returns a controller. The controller'sAutoInjectmounts the snapshot hook on session/turn boundaries when enabled.pkg/hooks/builtins/builtins.go\u2014 added a genericAutoInjectorinterface;Registerno longer wires the snapshot builtin andAgentDefaults.Snapshotis gone (the snapshot block inApplyAgentDefaultsmoved onto the controller).pkg/runtime/snapshot.go\u2014 deleted.pkg/runtime/runtime.go/hooks.go\u2014 removedWithSnapshots,snapshotsEnabled, and the snapshot methods. NewWithHooksRegistrylets the embedder ship a registry with snapshot pre-registered, andWithAutoInjectorplumbs in anyAutoInjector.buildHooksExecutorsruns the registered injectors.pkg/app/\u2014Appgains asnapshotControllerviaWithSnapshotController(c).UndoLastSnapshot/ListSnapshots/ResetSnapshotdelegate directly to the controller, resolving cwd from the session (withos.Getwdfallback).cmd/root/run.go\u2014 a smallsnapshotRuntimeOptshelper builds a registry, callsRegisterSnapshot, and returns the runtime opts plus the controller;buildAppOptsthreads 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 exemptssnapshot.gofrom the "must be registered in builtins.go" check since it has its ownRegisterSnapshotentry point.Acceptance criteria
pkg/runtime/snapshot.gois gone.LocalRuntimeexposes no snapshot-specific methods.WithSnapshotsis removed fromruntime.Opt; configuration moves to the controller.Validation
task build\u2014 okgo test ./...\u2014 oktask lint\u2014 0 issues