core/go
8
0
Fork 0
generated from lthn/LEM
Code Issues 5 Pull requests 1 Projects Releases Packages Wiki Activity Actions

[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
pull from: agent/review-pr--28--fifth-pass-at-commit-98d0
merge into: core:dev
Conversation 3 Commits 4 Files changed 5 +13739 -126183
Virgil commented 2026-03-24 17:31:03 +00:00
Member
Copy link

Task

Review PR #28 (fifth pass at commit 98d0781). Run git log --oneline -3 FIRST. Key change: WithService is now a simple factory call. HandleIPCEvents discovery moved to New() post-construction via discoverHandlers(). Verify: 1) No double-registration possible 2) No empty Service{} placeholders 3) discoverHandlers scans Config correctly 4) Any remaining issues. Report findings with file:line references.

Agent: codex
Commits: 4
Branch: agent/review-pr--28--fifth-pass-at-commit-98d0

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

## Task Review PR #28 (fifth pass at commit 98d0781). Run git log --oneline -3 FIRST. Key change: WithService is now a simple factory call. HandleIPCEvents discovery moved to New() post-construction via discoverHandlers(). Verify: 1) No double-registration possible 2) No empty Service{} placeholders 3) discoverHandlers scans Config correctly 4) Any remaining issues. Report findings with file:line references. **Agent:** codex **Commits:** 4 **Branch:** `agent/review-pr--28--fifth-pass-at-commit-98d0` --- Auto-created by core-agent dispatch system. Co-Authored-By: Virgil <virgil@lethean.io>
Virgil added 3 commits 2026-03-24 17:31:03 +00:00 
- 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>
- 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>
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>
Virgil commented 2026-03-24 17:31:05 +00:00
Author
Member
Copy link

Tests Passed — Merge Failed

go test ./... passed but merge failed: forgeMergePR: HTTP 409: merge push out of date

## Tests Passed — Merge Failed `go test ./...` passed but merge failed: forgeMergePR: HTTP 409: merge push out of date
Virgil commented 2026-03-24 17:31:07 +00:00
Author
Member
Copy link

Tests Passed — Merge Failed

go test ./... passed but merge failed: forgeMergePR: HTTP 409: merge push out of date

## Tests Passed — Merge Failed `go test ./...` passed but merge failed: forgeMergePR: HTTP 409: merge push out of date
Virgil added the
needs-review
 label 2026-03-24 17:31:08 +00:00
Virgil commented 2026-03-24 17:31:08 +00:00
Author
Member
Copy link

Needs Review

Merge conflict persists after rebase. Auto-merge gave up after retry.

Labelled needs-review for human attention.

## Needs Review Merge conflict persists after rebase. Auto-merge gave up after retry. Labelled `needs-review` for human attention.
Virgil added 1 commit 2026-03-24 20:20:52 +00:00 
Co-Authored-By: Virgil <virgil@lethean.io>
Virgil closed this pull request 2026-03-24 20:25:21 +00:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
needs-review

   
needs-review

   
needs-review

   
needs-review

   
needs-review

   
needs-review

   
needs-review
  
athena

Assign to Athena AI agent (M3 Ultra)

  
athena-gemini

Dispatch to Gemini CLI on M3 Ultra

  
audit

Code audit task

  
clotho

Assign to Clotho AI agent for processing

  
clotho-gemini

Dispatch to Gemini CLI on darbs (AU)

  
codex

Dispatch to Codex CLI on gateway for analysis

  
darbs-claude

Dispatch to Claude on darbs (AU)

  
security

Security review task

  
wiki

Documentation/wiki update

No labels
needs-review
needs-review
needs-review
needs-review
needs-review
needs-review
needs-review
athena
athena-gemini
audit
clotho
clotho-gemini
codex
darbs-claude
security
wiki
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

-

Dependencies

No dependencies set.

Reference: core/go#33
No description provided.
Delete branch "agent/review-pr--28--fifth-pass-at-commit-98d0"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?