Skip to content

refactor(daemon): remove unused source-label socket API (closes #21) - #23

Merged
JosephOIbrahim merged 1 commit into
masterfrom
refactor/d3-remove-socket-api
Jun 22, 2026
Merged

refactor(daemon): remove unused source-label socket API (closes #21)#23
JosephOIbrahim merged 1 commit into
masterfrom
refactor/d3-remove-socket-api

Conversation

@JosephOIbrahim

@JosephOIbrahim JosephOIbrahim commented Jun 22, 2026

Copy link
Copy Markdown
Owner

Resolves #21 (Tier 5 / D3) with the remove decision — no agents/harness.py adoption planned.

Removed (test-only, zero production callers)

Unaffected

  • The production path acquire_listening_socket (used by daemon/main.py) — it has its own adopt_launchd_socket + D69 live-listener guard and never called the removed helpers. Its tests live in test_accept_loop.py.

Verification

  • py_compile + import OK; __all__ reduced to the 3 production symbols.
  • Repo-wide grep → zero dangling refs to the removed symbols.

Closes #21.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Refactor

    • Consolidated socket activation to a single production-ready API, eliminating alternate acquisition paths.
  • Improvements

    • Strengthened socket ownership handling with stricter file descriptor management.
    • Improved platform-specific socket acquisition and adoption.
  • Tests

    • Removed test coverage for legacy socket activation paths.

Tier 5 / D3 decided: remove (no agents/harness.py adoption planned).
Deletes acquire_listen_socket + try_launchd_socket / try_systemd_socket /
bind_dev_socket and their test module — test-only code with zero
production callers.

The D69-guarded acquire_listening_socket (used by daemon/main.py) is the
sole acquisition path and is unaffected: it has its own
adopt_launchd_socket + live-listener guard and never used the removed
helpers. Also drops the now-unused `import ctypes.util` and the ponytail
deferral marker added in #22.

Verification: py_compile + import OK; zero dangling refs to the removed
symbols; production acquire_listening_socket intact. pytest not run
locally (no env) — CI gate.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e16ff524-72a1-4c9a-973d-0bb09886373b

📥 Commits

Reviewing files that changed from the base of the PR and between e1ef285 and 3d0918e.

📒 Files selected for processing (3)
  • docs/PONYTAIL_REVIEW_2026-06-20.md
  • python/harlo/daemon/socket_activation.py
  • tests/test_daemon/test_socket_activation.py
💤 Files with no reviewable changes (1)
  • tests/test_daemon/test_socket_activation.py

📝 Walkthrough

Walkthrough

Removes the alternate acquire_listen_socket / try_launchd_socket / try_systemd_socket / bind_dev_socket API from socket_activation.py, introduces adopt_launchd_socket (ctypes launchd adoption with free(3) cleanup) and a _socket_path_has_live_listener probe, hardens acquire_listening_socket with FD-ownership semantics, deletes the associated test module, and marks the review doc item resolved.

Changes

Socket Activation API Replacement

Layer / File(s) Summary
New macOS and live-listener helpers
python/harlo/daemon/socket_activation.py
Module docstring is rewritten to describe launchd → systemd → dev ordering. adopt_launchd_socket(name) is added to call launch_activate_socket via ctypes and release the fd buffer with free(3), returning None on non-applicable platforms/names. _socket_path_has_live_listener(sock_path, timeout) is added as a conservative probe returning False for stale/missing sockets and True for ambiguous errors.
acquire_listening_socket and __all__ reduction
python/harlo/daemon/socket_activation.py
acquire_listening_socket is updated to prefer adopted launchd fds (owns_node=False), fall through to the systemd fd-3 path, and refuse to unlink/rebind when _socket_path_has_live_listener detects an active listener (raising SystemExit). __all__ is reduced to LAUNCHD_SOCKET_NAME, acquire_listening_socket, and adopt_launchd_socket.
Test deletion and review doc resolution
tests/test_daemon/test_socket_activation.py, docs/PONYTAIL_REVIEW_2026-06-20.md
test_socket_activation.py (five tests covering the removed helpers) is deleted. The review doc Tier 5 / D3 entry is updated from "parked/needs a roadmap call" to resolved.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant acquire_listening_socket
  participant adopt_launchd_socket
  participant _socket_path_has_live_listener

  Caller->>acquire_listening_socket: sock_path, launchd_name
  acquire_listening_socket->>adopt_launchd_socket: launchd_name (macOS only)
  alt launchd fd available
    adopt_launchd_socket-->>acquire_listening_socket: [fd, ...]
    acquire_listening_socket-->>Caller: (socket, owns_node=False)
  else LISTEN_FDS set (systemd)
    acquire_listening_socket-->>Caller: (socket from fd 3, owns_node=False)
  else dev fallback
    acquire_listening_socket->>_socket_path_has_live_listener: sock_path
    alt live listener detected
      _socket_path_has_live_listener-->>acquire_listening_socket: True
      acquire_listening_socket-->>Caller: SystemExit
    else no live listener
      _socket_path_has_live_listener-->>acquire_listening_socket: False
      acquire_listening_socket-->>Caller: (bound socket, owns_node=True)
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • JosephOIbrahim/Harlo#22: Added the ponytail review marker deferring the acquire_listen_socket API decision that this PR now resolves by removing it.

Poem

🐇 Hop, hop — the old sockets are gone!
No more acquire_listen_socket to carry on.
adopt_launchd_socket now leads the way,
With free(3) cleanup at end of day.
One production path, clean and true —
This bunny tidied the queue! 🧹

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor(daemon): remove unused source-label socket API (closes #21)' accurately describes the main change: removal of the unused acquire_listen_socket API and related functions.
Linked Issues check ✅ Passed The PR fully implements the 'Remove' decision from issue #21 by deleting the unused acquire_listen_socket function, its three helper functions, and their associated test module.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #21: removal of the acquire_listen_socket API, helpers, tests, and the ponytail deferral marker as intended.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/d3-remove-socket-api

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@JosephOIbrahim
JosephOIbrahim merged commit 85e989a into master Jun 22, 2026
6 checks passed
@JosephOIbrahim
JosephOIbrahim deleted the refactor/d3-remove-socket-api branch June 22, 2026 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Decide fate of socket_activation source-label API (acquire_listen_socket): adopt in agents/harness.py or remove

1 participant