feat: add first-class host UX for plan management (#3844) - #3853
Draft
Sayt-0 wants to merge 5 commits into
Draft
Conversation
… ChangeNotifier - ValidateName: canonical name rule now exported so pkg/plans validates identically to the storage without duplicating the regexp. - CorruptPlanError: typed error replacing the anonymous fmt.Errorf in load(); same message, cause wrapped, distinguishable via errors.As. - SharedStorage(): returns the storage behind the process-wide singleton so a TUI and agent tools in one process share one mutex. - ChangeNotifier interface + SetChangeCallback on ToolSet: fires after every successful write/status/delete, never on failures; stored as an atomic.Pointer so per-turn re-configuration from concurrent sessions is race-free. Discoverable through toolset wrappers via tools.As.
New pkg/plans wraps the existing plan.Storage and sessionplan helpers behind a single Service interface consumed by both the CLI and the TUI, so neither surface ever reads plan files directly. - Unified Plan model with explicit Scope (shared | session) and nil Version for session plans (no revisions). - Six typed host errors: NotFoundError, ValidationError, CorruptError, StorageError, ConflictError (with Expected/Current), UnsupportedError. - Service operations: List, Get, Create, Update, SetStatus, Delete, Export — all with ExpectedVersion / force semantics. - Create is create-only (expected revision 0); Update is update-only (MustExist); session mutations return UnsupportedError immediately. - List with SessionID includes only that session's plan; stale session files are never enumerated globally. - Export is atomic (atomicfile.Write + parent mkdir) for both scopes. - 34 unit tests covering all error kinds, create-only conflict, stale update/status/delete preserving newer content, session scope, injected in-memory storage, and NewService nil-storage panic.
Implements the CLI surface from #3844: list, get, create, update, status, export, delete — all built over pkg/plans.Service to avoid any direct storage access. Key behaviors: - Mutations require exactly one of --expected-version <n> or --force (Cobra MarkFlagsOneRequired + MarkFlagsMutuallyExclusive); nil expected version is never sent by accident. - --expected-version validated >= 1; create is guard-free (service enforces create-only via expected revision 0). - Session plans (--session <id> / --scope session): list/get/export work; mutations surface the service's UnsupportedError with actionable reason. - --json: stable schema_version:"1" envelope on stdout; single-line JSON error object on stderr with code/scope/name/op/expected_version/ current_version, never mixing prose. All codes are typed, not text-parsed. - Exit code 3 for conflicts, 1 for all other failures; Cobra SilenceErrors so stderr stays one object in --json mode. - Service is built lazily inside RunE so plan.SharedStorage() does not resolve the data directory before --data-dir is applied. - 30 hermetic CLI tests with injected temp-backed service.
Adds PlanChangedEvent (type plan_changed) carrying scope/name/action/ version but never content, so a UI refreshes through its own plan service rather than trusting event payloads. - configureToolsetHandlers wires a non-blocking callback via tools.As[plan.ChangeNotifier] — same pattern as the RAG event forwarder — so a stale sink can never hang a tool call. - Event registered in the client registry so remote runtimes decode it as *PlanChangedEvent rather than dropping it. - 3 tests: serialization (no content field), client decode over SSE, and end-to-end write/status/delete emit vs. failed-write no-emit through configureToolsetHandlers.
Implements the TUI host-management surface from #3844. Slash command: - /plans opens the plan browser (command palette discoverable, immediate, silently dropped in lean mode like /settings). Browser dialog (pkg/tui/dialog/plan_browser.go): - Lists shared plans + active session plan (no global session sweep), showing scope, identity, status, version (- for session), relative updated time, and title; all columns safely truncated. - Keyboard: ↑/↓ navigate, / filter (Esc exits filter), Enter → detail, r refresh, x export, s set status, d delete (confirmation), n new plan (name dialog → $EDITOR), e edit ($EDITOR with pre-seeded content). - Session plan rows show an explanatory notification instead of an action dialog for s/d/e. - Double-click opens detail; warnings shown in footer. - Mouse click navigates, double-click opens detail. Detail dialog (pkg/tui/dialog/plan_detail.go): - Explicit scope line ("shared — collaborative, versioned" / "session — owned by its session, read-only here"), all metadata, scrollable markdown-rendered content with width-cap fallback. - Same action keys as browser; session scope hides s/e/d from help and shows an explanation instead. - Implements PlanDetailViewer so the app can re-fetch the exact plan on refresh. App handlers (pkg/tui/plans.go): - Plans service built lazily (plan.SharedStorage() only resolved after --data-dir is applied); injectable for tests. - All mutations version-guarded (never nil by accident); conflicts surface current version, keep newer content, refresh dialogs. - Export: deterministic filename (<name>.md / session-plan-<8chars>.md), refuses to overwrite an existing file. - Create/edit: temp file + tea.ExecProcess, drift detected before editor opens, draft kept on conflict. - SessionPlanUpdatedEvent: refresh open dialogs only for the current session, forward to chat page. - PlanChangedEvent: refresh open dialogs regardless of which session emitted it; background-tab mutations also refresh via handleRoutedMsg. Broadcastable mechanism (pkg/tui/dialog/dialog.go): - Messages implementing Broadcastable are delivered to all dialogs in the stack so a browser buried under the detail stays in sync. 19 browser/detail tests + 17 app-handler tests.
Member
Author
|
not finished yet don't approve or merge |
aheritier
added
area/cli
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
2 participants
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.
Summary
Closes #3844.
Implements the full host-facing plan management surface in four independent layers, each buildable and testable on its own.
What's in this PR
1.
pkg/tools/builtin/plan— minimal backward-compatible extensionsValidateName(name string) errorpkg/plansso both surfaces enforce the same patternCorruptPlanError{File, Err}errors.AsSharedStorage() StorageChangeNotifierinterface +SetChangeCallbackatomic.Pointermakes per-turn re-wiring race-safe2.
pkg/plans— host-facing service packageSingle
Serviceinterface consumed by both CLI and TUI.Planmodel: explicitScope(shared | session),Version *intis nil for session plans (no revisions).NotFoundError,ValidationError,CorruptError,StorageError,ConflictError(withExpected/Current),UnsupportedError.ExpectedVersion/ force semantics.UnsupportedErrorwith an actionable reason.ListwithSessionIDincludes only that session's plan; stale session files are never enumerated globally.3.
docker agent plans— CLI command group--expected-versionor--force(Cobra mutual-exclusion guard).--json:schema_version:"1"envelope on stdout; single-line JSON error on stderr with stablecode/scope/name/op/expected_version/current_version.4. TUI
/plansbrowser + detail/plansslash command (command palette discoverable, dropped in lean mode)./, keyboard nav, mouse click/double-click;rrefresh,xexport,sset status,ddelete (confirmation),nnew,eedit. Session plan rows show explanatory notifications for unsupported actions.SessionPlanUpdatedEventrefreshes for the current session;PlanChangedEventrefreshes for any shared plan mutation, including from background tabs.Broadcastablemechanism ondialog.Manager: data-refresh messages reach all dialogs in the stack so a browser buried under the detail stays in sync.5.
pkg/runtime— PlanChangedEventPlanChangedEvent(typeplan_changed) carries scope/name/action/version — never content — wired viatools.As[plan.ChangeNotifier]with a non-blocking sink (same RAG forwarder pattern).Tests
pkg/tools/builtin/planpkg/planscmd/rootpkg/runtimepkg/tui/dialogpkg/tuiAll pass with
go test -count=1;gofmtclean; lint 0 new issues.Known gaps (noted in issue, out of scope)
FilesystemStorageuses an in-process mutex; two separate processes can race on the revision bump. The existing docs already state this limitation for distinct processes.stat + write; a truly atomicO_EXCLvariant would close the rare race window.