[refactor] : clean code for api.lua with handlers#337
[refactor] : clean code for api.lua with handlers#337jensenojs wants to merge 2 commits intosudo-tee:mainfrom
Conversation
Move command execution into a single handlers layer and keep api.lua as a thin action map. This removes the split handlers/usecases execution path so command parsing, dispatch, and execution boundaries are explicit and reviewable.
Codify command-layer invariants so the refactor is reviewable and regression-safe. The tests lock parser/dispatcher behavior, handler boundaries, keymap routing, and registry contracts while tightening shared types used by those checks.
What comes nextThis refactor is intended as groundwork for a longer direction: moving feature work toward recipe-style extension, instead of repeatedly editing core files.
|
|
My modifications here were a bit rushed, but I need to get some sleep (and unfortunately, I had to submit another large PR...). @sudo-tee |
|
Thanks for the PR. I will have a look |
There was a problem hiding this comment.
Pull request overview
Refactors Opencode’s command execution into an explicit pipeline (entry -> router -> parse -> dispatch -> handlers) and introduces an extension-backed registry so command/slash definitions and handlers are centrally defined and validated.
Changes:
- Added a registry + loader layer for builtin and user extensions (commands, slash commands, handlers, hooks, etc.) with conflict policies.
- Implemented the new command stack (router/parse/dispatch/handlers) and updated entry points (Neovim
:Opencode, slash commands, keymaps) to route through it. - Slimmed
api.luainto a public “actions surface” that composes handler action modules, and added extensive unit coverage/guards for dependency boundaries and error normalization.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/registry_loader_spec.lua | Validates extension loading, isolation, conflict policy, and legacy usecases adaptation. |
| tests/unit/keymap_spec.lua | Updates keymap tests for router-based dispatch and legacy alias behavior. |
| tests/unit/entry_dependency_guard_spec.lua | Adds dependency boundary guards (router boundary, no dynamic api[...] access). |
| tests/unit/commands_router_spec.lua | Covers API-method-to-command argv routing and invalid_command behavior. |
| tests/unit/commands_parse_spec.lua | Covers parse results and stable error shapes (unknown/missing/invalid subcommand). |
| tests/unit/commands_handlers_spec.lua | Validates handler isolation, duplicate detection, and error normalization via executor. |
| tests/unit/commands_dispatch_spec.lua | Covers lifecycle ordering, hook isolation, registry hook pipeline, and error normalization. |
| tests/unit/api_spec.lua | Updates expectations for registry-backed command defs and public API boundary. |
| lua/opencode/types.lua | Adds/updates types for command registry, lifecycle hooks, dispatch results, and extension config. |
| lua/opencode/state/init.lua | Updates state type annotations to include common top-level fields. |
| lua/opencode/registry/loader.lua | Adds extension loader with builtin/user extensions and legacy usecases warning/adaptation. |
| lua/opencode/registry/init.lua | Implements registry capabilities, conflict policy handling, and lazy setup. |
| lua/opencode/registry/extensions/slash.lua | Provides builtin slash-command specs mapped to API methods. |
| lua/opencode/registry/extensions/commands.lua | Exposes builtin command definitions via builtin_commands.get(). |
| lua/opencode/registry/builtin_commands.lua | Defines builtin command schema (handler_id, completions, nested validation, etc.). |
| lua/opencode/keymap.lua | Refactors keymap binding to router + explicit non-command whitelist callbacks and legacy aliases. |
| lua/opencode/init.lua | Initializes registry and registers :Opencode via opencode.commands. |
| lua/opencode/config.lua | Adds extensions configuration defaults (builtin toggles + conflict policy). |
| lua/opencode/commands/slash.lua | Builds runtime slash commands from registry + user commands, routes via router. |
| lua/opencode/commands/router.lua | Routes entry invocations to parse+dispatch, provides API-method-to-command argv mapping. |
| lua/opencode/commands/parse.lua | Parses argv/range into intent and returns stable parse errors. |
| lua/opencode/commands/init.lua | Registers :Opencode and provides completion via registry + completion providers. |
| lua/opencode/commands/handlers/*.lua | Implements command semantics in handler modules (window/session/diff/workflow/agent/permission). |
| lua/opencode/commands/handlers.lua | Central handler executor, duplicate detection, and extension handler integration. |
| lua/opencode/commands/dispatch.lua | Dispatch lifecycle (before/after/error/finally), hook pipeline, normalized errors. |
| lua/opencode/commands/completion_providers.lua | Adds completion provider registry (e.g., user_commands). |
| lua/opencode/api.lua | Replaces monolithic API with composed action tables from handler modules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
registry part should remove in this pr |
|
I was wondering. If the plan is to move to a registry. The core should also use it all the way. I agree that this could be a seperate PR |
|
That was my local exploratory code, and I still believe some kind of registration mechanism is necessary. It would unify built-in features with those added in the future via recipes or plugins, but it isn't a problem with an immediately obvious, elegant design. I initially wanted to find the answer to this before starting changes on api.lua, but I failed. The time might not be right to solve this problem yet (or perhaps my expectations for it were too high). |
|
No worries, take your time, there is no rush |
Summary
for #322
This PR makes the command path explicit and predictable.
entry (command/slash/keymap) -> router -> parse -> dispatch -> handlersSo instead of spreading command behavior across API shortcuts and implicit fallbacks, we now have one clear path to read, debug, and review.
routeronly decides where to goparseonly validates intentdispatchowns execution lifecycle + normalized errorshandlersown command semanticsapi.luais just a thin mapping layerIntentional behavior tightening
This is not a pure no-op move. A few behaviors are intentionally stricter now:
api[func]dispatchinvalid_commandusecases -> handlerscompatibility is kept at loader edge, not inside registry core flow