[agent/codex] Review PR #28 (fifth pass at commit 98d0781). Run git log --... #33

Closed
Virgil wants to merge 4 commits from agent/review-pr--28--fifth-pass-at-commit-98d0 into dev

4 commits

Author SHA1 Message Date
Virgil
d413e34097 test(core): cover embed root mount paths
Co-Authored-By: Virgil <virgil@lethean.io>
2026-03-24 20:20:19 +00:00
Snider
98d078130e 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>
2026-03-24 17:24:50 +00:00
Snider
3a9ac82275 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>
2026-03-24 17:14:51 +00:00
Snider
b5dcdbb216 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>
2026-03-24 16:59:33 +00:00