Architecture review · smolpaws

The turn-monitoring loop lives in two places

smolpaws polls the agent-server the same way from two functions. monitorTurn (shared, 3 callers) and monitorConversationTurn (a local copy, 1 caller) run the identical poll → claim → terminal → result cycle — and have already drifted apart.

One loop, two implementations. Fold the local copy's two extras (task-command claiming, retry) into the shared monitorTurn as hooks, delete monitorConversationTurn, and the polling cycle has a single home with four callers. Strongin-process

1The duplication

Both functions implement the same contract: submit a turn, then loop until a terminal status, claiming outbound messages each tick, and fetch the result. They differ only at the edges.

 monitorTurn (shared)monitorConversationTurn (local)
LocationturnClient.ts:248local-agent-server.ts:124
Poll loopwhile (Date.now() < deadline) :286while (Date.now() < deadline) :143
Terminal checkTERMINAL_TURN_STATUSES.has :309TERMINAL_STATUSES.has :183
Extra 1onOutboundMessage callback :305claimTurnTaskCommands :155
Extra 2retry wrapper + 5× token read :147+
Callersgithub :220, slack :90, bridge :191index / task-scheduler (1 path)

2Before → after

turn-monitoring: before (two loops) vs after (one deep monitor) Before: three callers use the shared monitorTurn while one path uses a duplicate local loop. After: all four callers use one deepened monitorTurn. BEFORE — two loops, drifting AFTER — one deep monitor monitorTurn turnClient.ts:248 monitorConversationTurn local-agent-server.ts:124 same loop, copied github · slack · bridge index / scheduler duplicates monitorTurn (deep) + onTaskCommand + retry hooks turnClient.ts:248 4 callers github slack bridge local
The dashed red edge is the whole problem: the local loop is a second copy of the shared one. After, it's gone — one deep module, four callers.

3Where they drifted

Because the loop was copied rather than shared, each side grew a capability the other lacks — the classic cost of duplication.

  • Local-only: claims task commands each tick (:155) and wraps every call in a retry helper, reading SMOLPAWS_RUNNER_TOKEN?.trim() at 5 sites.
  • Shared-only: exposes an onOutboundMessage callback (:305) that the local copy re-derives inline.
⚠ A bug fixed in one loop is not fixed in the other. Terminal-status handling, timeout, and outbound-claim ordering must be kept in sync by hand today.

4The fix

Deepen the shared module; delete the copy.

// turnClient.ts — monitorTurn gains two optional hooks
monitorTurn({
  ...existing,
  onOutboundMessage,           // already there
  onTaskCommand,               // from the local copy
  wrap: retryRunnerOperation,   // inject the retry wrapper
})
// local-agent-server.ts — monitorConversationTurn: deleted (~107 lines)
★ Result: one interface exercised by four callers, the drift risk gone, and the repeated token read collapses behind the seam.

Generated by smolpaws · /improve-codebase-architecture · grounded to smolpaws@09b843e