refactor(core): remove core.lua and adopt services as shared primitives#360
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy opencode.core module and refactors core runtime behavior into a set of shared services/* primitives, updating UI/handler entrypoints and splitting the previously core-centric unit specs into service-focused test files.
Changes:
- Introduce
opencode.services.session_runtime,opencode.services.messaging, andopencode.services.agent_modelas the new shared primitives and deletelua/opencode/core.lua. - Rewire UI entrypoints and command handlers to call services instead of
opencode.core. - Split/refactor unit tests to align with the new service boundaries and add boundary documentation (
AGENTS.md).
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/services_spec_support.lua | Adds shared test helpers for mocking api_client and snapshot/restore of state/vim globals. |
| tests/unit/services_session_runtime_spec.lua | Migrates legacy core specs to session_runtime-focused tests; adds a setup idempotence test. |
| tests/unit/services_messaging_spec.lua | Adds messaging service unit tests (send flow + state updates). |
| tests/unit/services_agent_model_spec.lua | Adds agent_model service unit tests (mode/model initialization & restore). |
| tests/unit/hooks_spec.lua | Updates hook subscriptions to point at session_runtime service handlers. |
| tests/unit/commands_handlers_spec.lua | Updates tracked modules list to include services and remove core. |
| tests/unit/api_spec.lua | Updates API tests for new services wiring and adds a submit-input routing test. |
| lua/opencode/ui/session_picker.lua | Replaces core session switching/creation calls with session_runtime equivalents. |
| lua/opencode/ui/renderer.lua | Routes model restore-through-messages via services.agent_model. |
| lua/opencode/ui/input_window.lua | Routes submit to services.messaging.send_message. |
| lua/opencode/ui/history_picker.lua | Replaces core open call with session_runtime.open({ focus = 'input' }). |
| lua/opencode/ui/autocmds.lua | Routes cwd change handling through session_runtime.handle_directory_change. |
| lua/opencode/services/session_runtime.lua | New runtime/session orchestration service extracted from core. |
| lua/opencode/services/messaging.lua | New message send + after-run lifecycle service extracted from core. |
| lua/opencode/services/agent_model.lua | New model/mode/provider/variant service extracted from core. |
| lua/opencode/services/AGENTS.md | Documents the services boundary and invariants vs handlers. |
| lua/opencode/quick_chat.lua | Replaces core dependencies with services for session creation and model init. |
| lua/opencode/init.lua | Moves setup/subscriptions out of core into init and adds setup idempotence guard. |
| lua/opencode/core.lua | Deletes the legacy core module (replaced by services). |
| lua/opencode/commands/handlers/workflow.lua | Updates workflow actions to use services (open/send/model init/image paste). |
| lua/opencode/commands/handlers/window.lua | Updates window actions (open/cancel/toggle) to use session_runtime. |
| lua/opencode/commands/handlers/session.lua | Updates session actions to use session_runtime + agent_model. |
| lua/opencode/commands/handlers/diff.lua | Updates diff open helper to use session_runtime open/open_if_closed. |
| lua/opencode/commands/handlers/agent.lua | Updates agent actions to use agent_model service. |
| lua/opencode/commands/handlers/AGENTS.md | Aligns handler boundary doc with new services layer and debt list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4b21675 to
4008866
Compare
test: split legacy core_spec into service-focused specs chore(docs): align handlers/services AGENTS boundaries
4008866 to
b271725
Compare
|
This PR is just a beginning; it doesn't actually clean up the code (call chain). I'll add an analysis and explanation later. The division of phases in this stage is quite rough and not very instructive, but I still hope you can review this architecture diagram or the vision here. I believe this approach is key to improving maintainability in the future. Currently, the hierarchy of dependencies between Lua modules is very messy, so splitting the core merely shifts the focus of the problem without truly solving it. I'm working on a small script to trace the call chains between this plugin's modules. The specific design of the call flow is a topic open for further discussion (defined by JSON). We can use that little tool to see the gap between the current state of the code and the expected outcome, as well as the extent of reference relationship cleanup under each diff. In a way, I think it might be the endpoint of architectural code cleanup? |
|
using #361 |
|
The split make sense to me, even if it does not resolve the dependency you hoped for. Do you need more work on this to reduce the depedency ? I think that will have a hard time to reduce the module coupling since we use require directly. Until we adopt a Dependency injection mechanism, but I'm not sure I want to go there yet as this is time consuming and this plugin is not an entreprise framework but only a hobby |
|
Yes, I agree with your point. The direction of this PR is still correct; it just highlights an issue (which actually has little impact), and I think it can be merged. When I saw that dependency topology, I felt a bit of despair for a moment, as the workload seemed quite substantial. My amateur interest drives me to intermittently do some code cleanup because it’s quite enjoyable for me. Or perhaps we only care about code quality in projects we value, haha. lately, we might all need a break. I imagine you must be quite tired. However, on the positive side, I think there are at least two very good things. This tool helps us outline the current state of the architecture at a global level, although this starting point may not be ideal And I also tried to give what seems like a rather elegant endpoint (if you haven't seen that architecture diagram yet, I'd like to ask you to take another look #322 (comment) —in our internet jargon here, it might be called "alignment" haha). The advantage is that future functional implementations can be relatively freely combined. #318 (comment) As long as we can reach a broad consensus on the starting point and the endpoint, it might not matter if our next large-scale refactoring happens months or even years from now (or maybe others will be interested in continuing the refactoring?). Especially since subsequent refactoring should involve some convergence in direction. We are also unlikely to have serious design reviews, especially for the coordination between large-scale refactorings, as we have more or less created some new issues I intuitively feel that if the current code architecture is to be transformed into my ideal architecture, there might be some layering and migration of modules. The current UI module and state module both seem quite large (haha, I haven’t looked closely at your PR). Just thinking about such a large-scale migration gives me a headache, even though theoretically it’s similar to this PR—some metrics might appear worse, but it should just be temporary localized pain. But let’s take break for now , let's talk about it the next time we find it interesting @sudo-tee , Thank you for your time and effort. by the way, I didn't quite get how this relates to |
|
For the so we call require('opencode.state').something , rather that having But to do this would be a major refactor that I'm not sure is really worth it. |
|
Oh, I mean that even when using require, their dependency graph should still be relatively simple. But I understand your point. I agree; we'll stop at this stage until something interesting gets blocked by the current code. Just finding a random place to jot down potentially interesting PRs |
test: split legacy core_spec into service-focused specs
chore(docs): align handlers/services AGENTS boundaries