From 61cce7e380fc00d4c2f2aaabf8a03de47aec9d5f Mon Sep 17 00:00:00 2001 From: dan Date: Sun, 11 Jan 2026 15:34:15 -0800 Subject: [PATCH] refactor: extract helpers and add developer docs - Add optFromDb generic helper for nullable DbValue conversion - Extract rowToWorkerInfo helper in state.nim (dedup getWorker/getAllWorkers) - Extract loadContextFromPath helper in context.nim (dedup readContext/findContext) - Simplify poll() using optFromDb for optional field extraction - Add developer guide for compiling, testing, and deployment Closes: skills-r3k, skills-luzk, skills-dszn, skills-m0e2 Co-Authored-By: Claude Opus 4.5 --- docs/worker-cli-dev-guide.md | 115 +++++++++++++++++++++++++++++++++++ src/worker/context.nim | 34 ++++------- src/worker/db.nim | 20 +++--- src/worker/state.nim | 53 ++++++---------- src/worker/utils.nim | 7 +++ 5 files changed, 163 insertions(+), 66 deletions(-) create mode 100644 docs/worker-cli-dev-guide.md diff --git a/docs/worker-cli-dev-guide.md b/docs/worker-cli-dev-guide.md new file mode 100644 index 0000000..d5f7164 --- /dev/null +++ b/docs/worker-cli-dev-guide.md @@ -0,0 +1,115 @@ +# Worker CLI Developer Guide + +Quick reference for compiling, testing, and deploying the worker CLI. + +## Prerequisites + +- Nim 2.2+ (via `nix-shell -p nim`) +- SQLite library (`libsqlite3.so`) +- Git + +## Compiling + +```bash +# Standard build +nix-shell -p nim --run "nim c -d:release --mm:orc src/worker.nim" + +# Output: src/worker.out +``` + +Build options in `src/config.nims`: +- `--mm:orc` - ORC memory management (handles cycles) +- `--threads:on` - Background heartbeat thread +- `--opt:size` - Smaller binary +- `-d:release` - Optimizations + +## Testing + +```bash +# Run test suite (requires sqlite3 in PATH) +LD_LIBRARY_PATH=/path/to/sqlite/lib \ +PATH=$PATH:/path/to/sqlite/bin \ +src/worker/tests/test-worker.sh +``` + +On NixOS, find sqlite paths: +```bash +find /nix/store -name "libsqlite3.so" | head -1 +find /nix/store -name "sqlite3" -type f | grep bin | head -1 +``` + +## Project Structure + +``` +src/ +├── worker.nim # CLI dispatch, command implementations +├── config.nims # Nim build configuration +├── worker.nimble # Package metadata +└── worker/ + ├── types.nim # Shared types, constants, errors + ├── db.nim # SQLite operations, message bus + ├── state.nim # State machine transitions + ├── git.nim # Worktree operations + ├── context.nim # Worker context file handling + ├── heartbeat.nim # Background heartbeat thread + ├── review.nim # Review-gate integration + ├── utils.nim # Common helpers + └── tests/ + └── test-worker.sh # Integration test suite +``` + +## Dependencies + +Managed via nimble (`src/worker.nimble`): +- `tiny_sqlite` - SQLite wrapper with RAII +- `cligen` - CLI generation from proc signatures + +Install dependencies: +```bash +nimble install tiny_sqlite cligen +``` + +## Deployment + +The worker binary needs `libsqlite3.so` at runtime. Options: + +1. **System sqlite**: Ensure sqlite is installed system-wide +2. **LD_LIBRARY_PATH**: Point to sqlite library location +3. **Static linking**: Compile with `-d:staticSqlite` (requires sqlite source) + +Copy binary to PATH: +```bash +cp src/worker.out ~/.local/bin/worker +chmod +x ~/.local/bin/worker +``` + +## Runtime Files + +The worker CLI creates these files: +- `.worker-state/bus.db` - SQLite database (WAL mode) +- `.review-state/*.json` - Review gate state files +- `worktrees//` - Git worktrees for workers +- `worktrees//.worker-ctx.json` - Worker context + +## Exit Codes + +| Code | Constant | Meaning | +|------|----------|---------| +| 0 | ExitSuccess | Success | +| 2 | ExitUsageError | Invalid arguments | +| 3 | ExitInvalidTransition | Invalid state transition | +| 4 | ExitGitError | Git operation failed | +| 5 | ExitDbError | Database error | +| 6 | ExitConflict | Merge/rebase conflict | +| 7 | ExitNotFound | Worker not found | + +## Common Issues + +**"could not load: libsqlite3.so"** +Set `LD_LIBRARY_PATH` to include sqlite library directory. + +**Tests show "NOT_FOUND" for state queries** +Ensure `sqlite3` CLI is in PATH for test helper functions. + +**"Cannot start: Expected ASSIGNED, got WORKING"** +Worker already started. Use `worker status` to check state. diff --git a/src/worker/context.nim b/src/worker/context.nim index e98bbad..9bc9ec3 100644 --- a/src/worker/context.nim +++ b/src/worker/context.nim @@ -13,11 +13,9 @@ proc writeContext*(worktree: string, ctx: WorkerContext) = except IOError as e: raise newException(IOError, &"writeContext({path}): {e.msg}") -proc readContext*(worktree: string = ""): WorkerContext = - ## Read context from worktree. If empty, uses current directory. - let dir = if worktree != "": worktree else: getCurrentDir() - let path = dir / ContextFileName - +proc loadContextFromPath*(path: string): WorkerContext = + ## Load and parse context from a specific file path. + ## Validates the file exists and is not a symlink. if not fileExists(path): raise newException(IOError, &"Context file not found: {path}") @@ -30,11 +28,17 @@ proc readContext*(worktree: string = ""): WorkerContext = let j = parseJson(content) return WorkerContext.fromJson(j) except JsonParsingError as e: - raise newException(IOError, &"readContext({path}): invalid JSON: {e.msg}") + raise newException(IOError, &"loadContextFromPath({path}): invalid JSON: {e.msg}") except KeyError as e: - raise newException(IOError, &"readContext({path}): missing field: {e.msg}") + raise newException(IOError, &"loadContextFromPath({path}): missing field: {e.msg}") except TimeParseError as e: - raise newException(IOError, &"readContext({path}): invalid date format: {e.msg}") + raise newException(IOError, &"loadContextFromPath({path}): invalid date format: {e.msg}") + +proc readContext*(worktree: string = ""): WorkerContext = + ## Read context from worktree. If empty, uses current directory. + let dir = if worktree != "": worktree else: getCurrentDir() + let path = dir / ContextFileName + return loadContextFromPath(path) proc findContext*(): WorkerContext = ## Find context by walking up directory tree @@ -42,19 +46,7 @@ proc findContext*(): WorkerContext = while dir != "" and dir != "/": let path = dir / ContextFileName if fileExists(path): - # Security: reject symlinks to prevent reading arbitrary files - if getFileInfo(path).kind == pcLinkToFile: - raise newException(IOError, &"Context file is a symlink (rejected): {path}") - try: - let content = readFile(path) - let j = parseJson(content) - return WorkerContext.fromJson(j) - except JsonParsingError as e: - raise newException(IOError, &"findContext({path}): invalid JSON: {e.msg}") - except KeyError as e: - raise newException(IOError, &"findContext({path}): missing field: {e.msg}") - except TimeParseError as e: - raise newException(IOError, &"findContext({path}): invalid date format: {e.msg}") + return loadContextFromPath(path) dir = parentDir(dir) raise newException(IOError, "No .worker-ctx.json found in directory tree") diff --git a/src/worker/db.nim b/src/worker/db.nim index c71cd66..c372ba0 100644 --- a/src/worker/db.nim +++ b/src/worker/db.nim @@ -173,22 +173,18 @@ proc poll*(db: DbConn, agentId: string, limit: int = 100): seq[Message] = tsMs: row[2].fromDbValue(int64), fromAgent: row[3].fromDbValue(string), msgType: row[5].fromDbValue(string), + toAgent: optFromDb(row[4], string), + correlationId: optFromDb(row[6], string), + inReplyTo: optFromDb(row[7], string), + payloadRef: optFromDb(row[9], string), ) - if row[4].kind != sqliteNull: - msg.toAgent = some(row[4].fromDbValue(string)) - if row[6].kind != sqliteNull: - msg.correlationId = some(row[6].fromDbValue(string)) - if row[7].kind != sqliteNull: - msg.inReplyTo = some(row[7].fromDbValue(string)) - if row[8].kind != sqliteNull: - let payloadStr = row[8].fromDbValue(string) + # Payload needs special handling for JSON parsing + let payloadStr = optFromDb(row[8], string) + if payloadStr.isSome: try: - msg.payload = some(parseJson(payloadStr)) + msg.payload = some(parseJson(payloadStr.get)) except JsonParsingError as e: logWarn("poll", "malformed JSON in message " & msg.id & ": " & e.msg) - # Skip payload but continue processing message - if row[9].kind != sqliteNull: - msg.payloadRef = some(row[9].fromDbValue(string)) result.add(msg) proc ack*(db: DbConn, agentId: string, seq: int64) = diff --git a/src/worker/state.nim b/src/worker/state.nim index af01860..bf954df 100644 --- a/src/worker/state.nim +++ b/src/worker/state.nim @@ -13,6 +13,22 @@ import ./types import ./db import ./utils +proc rowToWorkerInfo*(row: ResultRow, heartbeatMs: Option[int64] = none(int64)): WorkerInfo = + ## Convert a database row to WorkerInfo. + ## Row must have: task_id, state, branch, worktree, description, created_at_ms, state_changed_at_ms + result = WorkerInfo( + taskId: row[0].fromDbValue(string), + state: parseState(row[1].fromDbValue(string)), + branch: row[2].fromDbValue(string), + worktree: row[3].fromDbValue(string), + createdAt: msToTime(row[5].fromDbValue(int64)), + stateChangedAt: msToTime(row[6].fromDbValue(int64)), + ) + if row[4].kind != sqliteNull: + result.description = row[4].fromDbValue(string) + if heartbeatMs.isSome: + result.lastHeartbeat = msToTime(heartbeatMs.get) + const ValidTransitions* = { wsAssigned: @[wsWorking, wsFailed], wsWorking: @[wsInReview, wsConflicted, wsFailed], @@ -130,24 +146,9 @@ proc getWorker*(db: DbConn, taskId: string): Option[WorkerInfo] = if row.isNone: return none(WorkerInfo) - var info = WorkerInfo( - taskId: row.get[0].fromDbValue(string), - state: parseState(row.get[1].fromDbValue(string)), - branch: row.get[2].fromDbValue(string), - worktree: row.get[3].fromDbValue(string), - createdAt: msToTime(row.get[5].fromDbValue(int64)), - stateChangedAt: msToTime(row.get[6].fromDbValue(int64)), - ) - - if row.get[4].kind != sqliteNull: - info.description = row.get[4].fromDbValue(string) - - # Get heartbeat let hb = db.getHeartbeat(taskId) - if hb.isSome: - info.lastHeartbeat = msToTime(hb.get.tsMs) - - return some(info) + let hbMs = if hb.isSome: some(hb.get.tsMs) else: none(int64) + return some(rowToWorkerInfo(row.get, hbMs)) proc getAllWorkers*(db: DbConn): seq[WorkerInfo] = ## Get all workers with their current state @@ -159,22 +160,8 @@ proc getAllWorkers*(db: DbConn): seq[WorkerInfo] = LEFT JOIN heartbeats h ON w.task_id = h.agent_id ORDER BY w.state_changed_at_ms DESC """): - var info = WorkerInfo( - taskId: row[0].fromDbValue(string), - state: parseState(row[1].fromDbValue(string)), - branch: row[2].fromDbValue(string), - worktree: row[3].fromDbValue(string), - createdAt: msToTime(row[5].fromDbValue(int64)), - stateChangedAt: msToTime(row[6].fromDbValue(int64)), - ) - - if row[4].kind != sqliteNull: - info.description = row[4].fromDbValue(string) - - if row[7].kind != sqliteNull: - info.lastHeartbeat = msToTime(row[7].fromDbValue(int64)) - - result.add(info) + let hbMs = optFromDb(row[7], int64) + result.add(rowToWorkerInfo(row, hbMs)) proc createWorker*(db: DbConn, taskId, branch, worktree: string, description: string = ""): WorkerInfo = diff --git a/src/worker/utils.nim b/src/worker/utils.nim index ca0a1bd..5e1c410 100644 --- a/src/worker/utils.nim +++ b/src/worker/utils.nim @@ -74,6 +74,13 @@ proc optInt64*(val: DbValue): Option[int64] = else: some(val.fromDbValue(int64)) +proc optFromDb*[T](val: DbValue, _: typedesc[T]): Option[T] = + ## Generic helper to convert nullable DbValue to Option[T] + if val.kind == sqliteNull: + none(T) + else: + some(val.fromDbValue(T)) + # Transaction helper template withTransaction*(db: DbConn, body: untyped) = ## Execute body within a transaction with automatic rollback on error.