[agent/codex] Review PR #28 (fourth pass at commit `3a9ac82`). Run git log -... #32

Closed

Virgil wants to merge 2 commits from agent/review-pr--28--fourth-pass-at-commit-3a9 into dev

Virgil commented

2026-03-24 17:20:16 +00:00

Member

Task

Review PR #28 (fourth pass at commit 3a9ac82). Run git log --oneline -3 FIRST. Previous findings: 1) Double IPC handler registration (FIXED — only on auto-discovered services now) 2) Empty Service{} placeholder (FIXED — only when factory didn't register itself). Verify fixes are correct. Check for any remaining issues. Report findings with file:line references.

Agent: codex
Commits: 3
Branch: agent/review-pr--28--fourth-pass-at-commit-3a9

Auto-created by core-agent dispatch system.
Co-Authored-By: Virgil virgil@lethean.io

## Task Review PR #28 (fourth pass at commit 3a9ac82). Run git log --oneline -3 FIRST. Previous findings: 1) Double IPC handler registration (FIXED — only on auto-discovered services now) 2) Empty Service{} placeholder (FIXED — only when factory didn't register itself). Verify fixes are correct. Check for any remaining issues. Report findings with file:line references. **Agent:** codex **Commits:** 3 **Branch:** `agent/review-pr--28--fourth-pass-at-commit-3a9` --- Auto-created by core-agent dispatch system. Co-Authored-By: Virgil <virgil@lethean.io>

Virgil added 2 commits 2026-03-24 17:20:17 +00:00

fix: address Codex review findings on PR #28 b5dcdbb216

- 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 3a9ac82275

- 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>

Virgil commented

2026-03-24 17:20:19 +00:00

Author

Member