HarperFast / HarperFast/harper
logRotationTransport hand-wraps four manageThreads exports it does not customize
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 2h
- Merged PRs (30d)
- 205
Description
`server/threads/logRotationTransport.ts` exists to hand the log-rotation generation coordinator a thread mesh from the `server/threads/` side of a deliberate dependency edge — `utility/logging/` must never import `manageThreads.js`, because `manageThreads` already imports `harper_logger`.
The way it does that is a hand-written object literal that re-declares every method the coordinator's `RotationTransport` interface names, four of which are pure pass-throughs:
```ts
setRotationTransport({
threadId,
broadcast(message) { broadcast(message); },
sendToThread(target, message) { threads.sendToThread(target, message); },
onMessage(type, handler) { onMessageByType(type, handler); },
onThreadExit,
peerThreadIds() { /* the only real customization */ },
});
```
Only `peerThreadIds()` carries a decision — it deliberately includes job workers, which `isEligibleBroadcastRecipient` excludes from the schema broadcast but which do hold log descriptors. Everything else is boilerplate that has to be edited again every time the interface gains a member, and that silently goes stale if a `manageThreads` signature changes underneath it.
Raised by @dawsontoth reviewing https://github.com/HarperFast/harper/pull/2475: *"this smells like something that will be brittle later ... peerThreadIds is the real customization we're making."*
**On the suggested shape.** Subclassing is not available as written: `manageThreads.js` is a CommonJS module, not a class, and `threads` is the `connectedPorts` array with `sendToThread`/`onMessageByType` bolted onto it. The equivalent cleanup is for `manageThreads` to export the narrow mesh interface itself (or for the transport to close over `connectedPorts` directly) so this file only supplies `peerThreadIds` and the eligibility decision behind it. That is the change worth making; the wrapper is only the symptom.
Related: the file is loaded by a bare `require('./logRotationTransport.ts')` at the very bottom of `manageThreads.js` — load-order-sensitive coupling chosen to keep the dependency direction. An explicit init call from the server bootstrap would be testable without importing the whole thread manager, and belongs in the same cleanup.
No behavior change, no user-visible impact — this is maintainability of a module that shipped in #2475.
Contributor guide
Research direction
Start by reading server/threads/logRotationTransport.ts and manageThreads.js, including the bare require at the bottom and the RotationTransport interface it adapts. Trace how the thread mesh is exported and initialized, then simplify the coupling so only peerThreadIds and its eligibility decision remain customized. Done means the dependency direction and behavior are unchanged, while initialization is explicit and testable.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js, typescript
- Domain
- backend, distributed-systems
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100