Dev (#13)
* feat: restore functional option pattern for New() New() returns Result, accepts CoreOption functionals. Restores v0.3.3 service registration contract: - WithService(factory func(*Core) Result) — service factory receives Core - WithOptions(Options) — key-value configuration - WithServiceLock() — immutable after construction Services registered in New() form the application conclave with shared IPC access. Each Core instance has its own bus scope. Co-Authored-By: Virgil <virgil@lethean.io> * fix: address Codex review findings on PR #28 - WithOptions copies the Options slice (constructor isolation regression) - WithService auto-discovers service name from package path via reflect - WithService auto-registers HandleIPCEvents if present (v0.3.3 parity) - Add test for failing option short-circuit in New() Co-Authored-By: Virgil <virgil@lethean.io> * fix: prevent double IPC registration + empty service placeholder - HandleIPCEvents only auto-registered for services the factory didn't register itself (prevents double handler registration) - Auto-discovery only creates Service{} placeholder when factory didn't call c.Service() — factories that register themselves keep full lifecycle Addresses Codex review findings 1 and 2 from third pass. Co-Authored-By: Virgil <virgil@lethean.io> * fix: move HandleIPCEvents discovery to New() post-construction WithService is now a simple factory call — no reflect, no auto-registration. New() calls discoverHandlers() after all opts run, scanning Config for service instances that implement HandleIPCEvents. This eliminates both double-registration and empty-placeholder issues: - Factories wire their own lifecycle via c.Service() - HandleIPCEvents discovered once, after all services are registered - No tension between factory-registered and auto-discovered paths Co-Authored-By: Virgil <virgil@lethean.io> * feat: RegisterService with instance storage + interface discovery Restores v0.3.3 service manager capabilities: - RegisterService(name, instance) stores the raw instance - Auto-discovers Startable/Stoppable interfaces → wires lifecycle - Auto-discovers HandleIPCEvents → wires to IPC bus - ServiceFor[T](c, name) for typed instance retrieval - Service DTO gains Instance field for instance tracking WithService is a simple factory call — no reflect, no magic. discoverHandlers removed — RegisterService handles it inline. No double-registration: IPC wired once at registration time. Co-Authored-By: Virgil <virgil@lethean.io> * feat: Options struct + Result methods + WithOption convenience Options is now a proper struct with New(), Set(), Get(), typed accessors. Result gains New(), Result(), Get() methods on the struct. WithOption("key", value) convenience for core.New(). options_test.go: 22 tests passing against the new contract. Other test files mechanically updated for compilation. Co-Authored-By: Virgil <virgil@lethean.io> * feat: App struct with New(Options) + Find() as method App.New() creates from Options. App.Find() locates programs on PATH. Both are struct methods — no package-level functions. 8 tests passing. Co-Authored-By: Virgil <virgil@lethean.io> * fix: update Cli doc comment + tests for new Options contract Cli struct unchanged — already conforms. Tests use WithOption() convenience. 9 tests passing. Co-Authored-By: Virgil <virgil@lethean.io> * feat: Cli.New(c) constructor — Core uses it during construction Cli{}.New(c) replaces &Cli{core: c} in contract.go. 9 tests passing. Co-Authored-By: Virgil <virgil@lethean.io> * wip: checkpoint before v0.3.3 parity rewrite Cli as service with ServiceRuntime, incomplete. Need to properly port v0.3.3 service_manager, message_bus, WithService with full name/IPC discovery. Co-Authored-By: Virgil <virgil@lethean.io> * feat: WithService with v0.3.3 name discovery + IPC handler auto-registration - WithService now calls factory, discovers service name from package path via reflect/runtime (last path segment, _test suffix stripped, lowercased), and calls RegisterService — which handles Startable/Stoppable/HandleIPCEvents - If factory returns nil Value (self-registered), WithService returns OK without a second registration - Add contract_test.go with _Good/_Bad tests covering all three code paths - Fix core.go Cli() accessor: use ServiceFor[*Cli](c, "cli") (was cli.New()) - Fix pre-existing })) → }}) syntax errors in command_test, service_test, lock_test - Fix pre-existing Options{...} → NewOptions(...) in core_test, data_test, drive_test, i18n_test (Options is a struct, not a slice) Co-Authored-By: Virgil <virgil@lethean.io> * feat: WithService with v0.3.3 name discovery + IPC handler auto-registration WithService now: calls factory, discovers service name from instance's package path via reflect.TypeOf, discovers HandleIPCEvents method, calls RegisterService. If factory returns nil Value, assumes self-registered. Also fixes: Cli() accessor uses ServiceFor, test files updated for Options struct. Co-Authored-By: Virgil <virgil@lethean.io> * feat: WithName for explicit service naming Co-Authored-By: Virgil <virgil@lethean.io> * test: lifecycle + HandleIPCEvents end-to-end via WithService Co-Authored-By: Virgil <virgil@lethean.io> * fix: WithServiceLock enables, New() applies after all opts — v0.3.3 parity Co-Authored-By: Virgil <virgil@lethean.io> * feat: MustServiceFor[T] + fix service names test for auto-registered cli Co-Authored-By: Virgil <virgil@lethean.io> * wip: v0.3.3 parity — Tasks 1-7 complete, data/embed tests need fixing WithService: full name discovery + IPC handler auto-registration via reflect WithName: explicit service naming RegisterService: Startable/Stoppable/HandleIPCEvents auto-discovery MustServiceFor[T]: panics if not found WithServiceLock: enable/apply split (v0.3.3 parity) Cli: registered as service via CliRegister, accessed via ServiceFor @TODO Codex: Fix data_test.go and embed_test.go — embed path resolution after Options changed from []Option to struct. Mount paths need updating. Co-Authored-By: Virgil <virgil@lethean.io> * fix: Result.New handles (value, error) pairs correctly + embed test fixes Root cause: Result.New didn't mark single-value results as OK=true, breaking Mount/ReadDir/fs helpers that used Result{}.New(value, err). Also: data_test.go and embed_test.go updated for Options struct, doc comments updated across data.go, drive.go, command.go, contract.go. All tests green. Coverage 82.2%. Co-Authored-By: Virgil <virgil@lethean.io> * feat: New() constructors for Config, Fs + simplify contract.go init Config.New() initialises ConfigOptions. Fs.New(root) sets sandbox root. ErrorLog uses Default() fallback — no explicit init needed. contract.go uses constructors instead of struct literals. All tests green. Co-Authored-By: Virgil <virgil@lethean.io> * fix: Service() returns instance, ServiceFor uses type assertion directly Co-Authored-By: Virgil <virgil@lethean.io> * feat: Core.Run() — ServiceStartup → Cli → ServiceShutdown lifecycle Co-Authored-By: Virgil <virgil@lethean.io> * feat: Core.Run() handles os.Exit on error Co-Authored-By: Virgil <virgil@lethean.io> * feat: New() returns *Core directly — no Result wrapper needed Co-Authored-By: Virgil <virgil@lethean.io> * fix: shutdown context, double IPC registration - Run() uses context.Background() for shutdown (c.context is cancelled) - Stoppable closure uses context.Background() for OnShutdown - WithService delegates HandleIPCEvents to RegisterService only Fixes Codex review findings 1, 2, 3. Co-Authored-By: Virgil <virgil@lethean.io> * test: add _Bad/_Ugly tests + fix per-Core lock isolation Tests: Run, RegisterService, ServiceFor, MustServiceFor _Bad/_Ugly variants. Fix: Lock map is now per-Core instance, not package-level global. This prevents deadlocks when multiple Core instances exist (e.g. tests). Coverage: 82.4% → 83.6% Co-Authored-By: Virgil <virgil@lethean.io> --------- Co-authored-by: Virgil <virgil@lethean.io> Co-authored-by: Virgil <virgil@lthn.ai>
This commit is contained in:
GitHub
parent
a6be0df3ea
commit
de6fa038d1