From 766069371789cd3e62f5a2e09330e36ffbfeb93d Mon Sep 17 00:00:00 2001 From: dan Date: Sun, 11 Jan 2026 00:26:02 -0800 Subject: [PATCH] feat(worker): add test suite and fix worktree DB path Test infrastructure: - Add comprehensive test-worker.sh with 59 tests - Cover spawn, start, done, approve, reject, cancel, fail, heartbeat - Test status filtering, context files, review-gate integration - Test invalid state transitions and error cases Bug fixes: - Fix agent commands failing when run from worktrees (skills-y3f2) - Add getMainRepoBusDbPath() to find DB in main repo - Update start, done, fail, heartbeat to use correct path - Fix review.nim crash when review-gate not installed (OSError) - Gracefully return exitCode=-1 instead of crashing Closes skills-y3f2 Co-Authored-By: Claude Opus 4.5 --- .beads/issues.jsonl | 4 +- src/worker.nim | 15 +- src/worker/review.nim | 21 +- src/worker/tests/test-worker.sh | 536 ++++++++++++++++++++++++++++++++ src/worker/utils.nim | 26 +- 5 files changed, 587 insertions(+), 15 deletions(-) create mode 100755 src/worker/tests/test-worker.sh diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 327ab62..a952473 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -160,6 +160,7 @@ {"id":"skills-lie","title":"Compare DEPENDENCIES.md with upstream","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-03T20:15:53.925914243-08:00","updated_at":"2025-12-03T20:19:28.665641809-08:00","closed_at":"2025-12-03T20:19:28.665641809-08:00","dependencies":[{"issue_id":"skills-lie","depends_on_id":"skills-ebh","type":"discovered-from","created_at":"2025-12-03T20:15:53.9275694-08:00","created_by":"daemon","metadata":"{}"}]} {"id":"skills-luzk","title":"Extract rowToWorkerInfo helper in state.nim","description":"[REDUNDANCY] LOW state.nim:136-143,165-172 - WorkerInfo construction duplicated in getWorker() and getAllWorkers(). Extract proc rowToWorkerInfo(row): WorkerInfo.","status":"open","priority":3,"issue_type":"task","created_at":"2026-01-10T19:49:53.238303032-08:00","created_by":"dan","updated_at":"2026-01-10T19:49:53.238303032-08:00"} {"id":"skills-lvg","title":"Compare ISSUE_CREATION.md with upstream","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-03T20:15:54.609282051-08:00","updated_at":"2025-12-03T20:19:29.134966356-08:00","closed_at":"2025-12-03T20:19:29.134966356-08:00","dependencies":[{"issue_id":"skills-lvg","depends_on_id":"skills-ebh","type":"discovered-from","created_at":"2025-12-03T20:15:54.610717055-08:00","created_by":"daemon","metadata":"{}"}]} +{"id":"skills-lxb9","title":"Return proper exit codes for InvalidTransition errors","description":"When approve/reject/start fail due to invalid state transitions, the InvalidTransition exception bubbles up unhandled, causing exit code 1 instead of ExitInvalidTransition (3). Should catch these exceptions and return proper exit codes.","status":"open","priority":2,"issue_type":"bug","created_at":"2026-01-11T00:17:51.270060721-08:00","created_by":"dan","updated_at":"2026-01-11T00:17:51.270060721-08:00"} {"id":"skills-lzh2","title":"Create utils.nim with common helpers","description":"Extract repeated patterns into src/worker/utils.nim:\n- branchName(taskId): string - from git.nim:36,59,89\n- worktreePath(taskId): string - from git.nim:37,53\n- msToUnix(ms): int64 - from state.nim (8 occurrences)\n- optField[T](row, idx): Option[T] - from db.nim:167-176\n- withTransaction template - from state.nim:37-74\n- validateTaskId(id): string - new, for CLI validation\n\nConsolidates: skills-3d9o, skills-5x2o, skills-r3k, skills-luzk, skills-qiq0, skills-2xc, skills-73yu, skills-vuj2\n\nParent: skills-g2wa","status":"closed","priority":1,"issue_type":"task","created_at":"2026-01-10T20:18:49.280359755-08:00","created_by":"dan","updated_at":"2026-01-10T20:32:28.34903461-08:00","closed_at":"2026-01-10T20:32:28.34903461-08:00","close_reason":"Created utils.nim with common helpers"} {"id":"skills-lzk","title":"Simplify branch name generation in create-new-feature.sh","description":"File: .specify/scripts/bash/create-new-feature.sh (lines 137-181)\n\nIssues:\n- 3 nested loops/conditionals\n- Complex string transformations with multiple sed operations\n- Stop-words list and filtering logic hard to maintain\n\nFix:\n- Extract to separate function\n- Simplify word filtering logic\n- Add input validation\n\nSeverity: MEDIUM","status":"closed","priority":3,"issue_type":"task","created_at":"2025-12-24T02:51:14.286951249-05:00","updated_at":"2026-01-03T12:13:27.083639201-08:00","closed_at":"2026-01-03T12:13:27.083639201-08:00","close_reason":"Simplifed generate_branch_name logic, added main() function, and BASH_SOURCE guard for testability."} {"id":"skills-m0e2","title":"Write developer docs for compiling/deployment workflow","status":"open","priority":3,"issue_type":"task","created_at":"2026-01-10T23:14:36.685506396-08:00","created_by":"dan","updated_at":"2026-01-10T23:14:36.685506396-08:00"} @@ -187,7 +188,7 @@ {"id":"skills-rex","title":"Test integration on worklog skill","description":"Use worklog skill as first real test case:\n- Create wisp for worklog execution\n- Capture execution trace\n- Test squash → digest\n- Validate trace format captures enough info for replay\n\nMigrated from dotfiles-drs.","status":"closed","priority":3,"issue_type":"task","created_at":"2025-12-23T19:21:18.75525644-05:00","updated_at":"2025-12-29T13:55:35.814174815-05:00","closed_at":"2025-12-29T13:55:35.814174815-05:00","close_reason":"Parked with ADR-001: skills-molecules integration deferred. Current simpler approach (skills as standalone) works well. Revisit when complex orchestration needed.","dependencies":[{"issue_id":"skills-rex","depends_on_id":"skills-3em","type":"blocks","created_at":"2025-12-23T19:22:00.34922734-05:00","created_by":"dan"}]} {"id":"skills-roq","title":"Design: Branch-per-worker isolation","description":"Each worker operates on its own git branch for code isolation.\n\n## Pattern\n- worker spawn creates branch: worker/\u003cid\u003e\n- Worker does all work on that branch\n- On completion, branch ready for review/merge\n- Orchestrator or human merges to main\n\n## Benefits\n- Clean isolation between parallel workers\n- Easy rollback (just delete branch)\n- Familiar git workflow\n- No conflicts during work\n\n## Implementation\nworker spawn:\n 1. git checkout -b worker/\u003cid\u003e\n 2. Run agent\n 3. Agent commits to branch\n 4. On completion, branch stays for review\n\nworker merge \u003cid\u003e:\n 1. Review diff\n 2. Merge to main (or rolling branch)\n 3. Delete worker branch\n\n## Open Questions\n- Merge from main during work? (rebase vs merge)\n- Rolling branch pattern? (main \u003c- rolling \u003c- workers)","design":"docs/design/branch-per-worker.md","notes":"Design complete. Key decisions: (1) type/task-id naming (not worker-id), (2) git worktrees for parallel agents, (3) rolling integration branch before main, (4) orchestrator creates branches, (5) trivial conflict auto-resolve, semantic escalates, (6) SQLite=process truth, Git=code truth, (7) serialize cross-worker deps, (8) archive failed branches. See orch consensus with flash-or/gemini/gpt.","status":"closed","priority":2,"issue_type":"task","created_at":"2026-01-10T13:24:21.364434026-08:00","created_by":"dan","updated_at":"2026-01-10T21:29:25.697839488-08:00","closed_at":"2026-01-10T21:29:25.697839488-08:00","close_reason":"Implemented in worker CLI - spawn, status, state machine, branch isolation all working","dependencies":[{"issue_id":"skills-roq","depends_on_id":"skills-s6y","type":"blocks","created_at":"2026-01-10T13:24:35.976245936-08:00","created_by":"dan"}]} {"id":"skills-rpf","title":"Implement playwright-visit skill for browser automation","description":"## Overview\nBrowser automation skill using Playwright to visit web pages, take screenshots, and extract content.\n\n## Key Findings (from dotfiles investigation)\n\n### Working Setup\n- Use `python312Packages.playwright` from nixpkgs (handles Node driver binary patching for NixOS)\n- Use `executable_path='/run/current-system/sw/bin/chromium'` to use system chromium\n- No `playwright install` needed - no browser binary downloads\n\n### Profile Behavior\n- Fresh/blank profile every launch by default\n- No cookies, history, or logins from user's browser\n- Can persist state with `storage_state` parameter if needed\n\n### Example Code\n```python\nfrom playwright.sync_api import sync_playwright\n\nwith sync_playwright() as p:\n browser = p.chromium.launch(\n executable_path='/run/current-system/sw/bin/chromium',\n headless=True\n )\n page = browser.new_page()\n page.goto('https://example.com')\n print(page.title())\n browser.close()\n```\n\n### Why Not uv/pip?\n- Playwright pip package bundles a Node.js driver binary\n- NixOS can't run dynamically linked executables without patching\n- nixpkgs playwright handles this properly\n\n## Implementation Plan\n1. Create `skills/playwright-visit/` directory\n2. Add flake.nix with devShell providing playwright\n3. Create CLI script with subcommands:\n - `screenshot \u003curl\u003e \u003coutput.png\u003e` - capture page\n - `text \u003curl\u003e` - extract text content \n - `html \u003curl\u003e` - get rendered HTML\n - `pdf \u003curl\u003e \u003coutput.pdf\u003e` - save as PDF\n4. Create skill definition for Claude Code integration\n5. Document usage in skill README\n\n## Dependencies\n- nixpkgs python312Packages.playwright\n- System chromium (already in dotfiles)\n\n## Related\n- dotfiles issue dotfiles-m09 (playwright skill request)","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-12-16T16:02:28.577381007-08:00","updated_at":"2025-12-29T00:09:50.681141882-05:00","closed_at":"2025-12-29T00:09:50.681141882-05:00","close_reason":"Implemented: SKILL.md, visit.py CLI (screenshot/text/html/pdf), flake.nix devShell, README. Network down so couldn't test devShell build, but code complete."} -{"id":"skills-s6y","title":"Multi-agent orchestration: Lego brick architecture","description":"Multi-agent orchestration: Lego brick architecture\n\nCoordinate 2-4 AI coding agents with human oversight.\n\nLanguage: Nim (ORC, cligen, tiny_sqlite) - see skills-q40\n\nCore components:\n- Worker state machine (skills-4oj)\n- Message passing layer (skills-ms5) \n- Branch-per-worker isolation (skills-roq)\n- Worker CLI primitives (skills-sse)\n- Human observability (skills-yak)\n- Review-gate integration (skills-byq)\n\nDesign docs: docs/design/\n- mvp-scope.md (v3)\n- message-passing-layer.md (v4)\n- worker-cli-primitives.md (v3)\n- worker-state-machine.md\n- branch-per-worker.md\n- human-observability.md","status":"open","priority":1,"issue_type":"epic","created_at":"2026-01-10T12:14:16.141746066-08:00","created_by":"dan","updated_at":"2026-01-10T18:04:12.614750539-08:00"} +{"id":"skills-s6y","title":"Multi-agent orchestration: Lego brick architecture","description":"Multi-agent orchestration: Lego brick architecture\n\nCoordinate 2-4 AI coding agents with human oversight.\n\nLanguage: Nim (ORC, cligen, tiny_sqlite) - see skills-q40\n\nCore components:\n- Worker state machine (skills-4oj)\n- Message passing layer (skills-ms5) \n- Branch-per-worker isolation (skills-roq)\n- Worker CLI primitives (skills-sse)\n- Human observability (skills-yak)\n- Review-gate integration (skills-byq)\n\nDesign docs: docs/design/\n- mvp-scope.md (v3)\n- message-passing-layer.md (v4)\n- worker-cli-primitives.md (v3)\n- worker-state-machine.md\n- branch-per-worker.md\n- human-observability.md","status":"in_progress","priority":1,"issue_type":"epic","created_at":"2026-01-10T12:14:16.141746066-08:00","created_by":"dan","updated_at":"2026-01-11T00:11:49.50673558-08:00"} {"id":"skills-s92","title":"Add tests for config injection (deploy-skill.sh)","description":"File: bin/deploy-skill.sh (lines 112-137)\n\nCritical logic with NO test coverage:\n- Idempotency (running twice should be safe)\n- Correct brace matching in Nix\n- Syntax validity of injected config\n- Rollback on failure\n\nRisk: MEDIUM-HIGH - can break dotfiles Nix config\n\nFix:\n- Test idempotent injection\n- Validate Nix syntax after injection\n- Test with malformed input\n\nSeverity: MEDIUM","status":"closed","priority":3,"issue_type":"task","created_at":"2025-12-24T02:51:01.314513824-05:00","updated_at":"2026-01-06T16:29:18.728097676-08:00","closed_at":"2026-01-06T16:29:18.728097676-08:00","close_reason":"21 tests added covering idempotency, brace preservation, inject_home_file wrapper, edge cases"} {"id":"skills-sh6","title":"Research: OpenHands iterative refinement pattern","description":"Document OpenHands SDK patterns for our architecture.\n\n## Iterative Refinement Loop\n1. Worker agent does work\n2. Critique agent evaluates (correctness, quality, completeness)\n3. If not good → worker tries again with feedback\n4. Repeat until standard met\n\n## Parallel Agent Orchestration\n- Git-based coordination (not direct communication)\n- Each agent works on own branch\n- PRs to intermediate 'rolling branch'\n- Human reviews and merges\n- Agents pull latest, handle conflicts\n\n## Key Quote\n'Don't expect 100% automation—tasks are 80-90% automatable.\nYou need a human who understands full context.'\n\n## Mapping to Our Architecture\n- Worker = their refactoring agent\n- Reviewer = their critique agent\n- review-gate = their quality threshold\n- Human orchestrator = their human on rolling branch\n\n## Sources\n- https://openhands.dev/blog/automating-massive-refactors-with-parallel-agents\n- https://arxiv.org/abs/2511.03690\n- https://docs.openhands.dev/sdk","status":"open","priority":3,"issue_type":"task","created_at":"2026-01-10T12:24:02.368542878-08:00","created_by":"dan","updated_at":"2026-01-10T12:24:02.368542878-08:00","dependencies":[{"issue_id":"skills-sh6","depends_on_id":"skills-s6y","type":"blocks","created_at":"2026-01-10T12:24:07.013388857-08:00","created_by":"dan"}]} {"id":"skills-sisi","title":"Extract MaxSummaryLen constant for description truncation","description":"[SMELL] LOW worker.nim:101 - Description truncated at magic number 30. Extract: const MaxSummaryLen = 30","status":"open","priority":4,"issue_type":"task","created_at":"2026-01-10T20:12:11.153123047-08:00","created_by":"dan","updated_at":"2026-01-10T20:12:11.153123047-08:00"} @@ -212,6 +213,7 @@ {"id":"skills-x33","title":"Add tests for branch name generation","description":"File: .specify/scripts/bash/create-new-feature.sh (lines 137-181)\n\nCritical logic with NO test coverage:\n- Word filtering with stop-words\n- Acronym detection\n- Unicode/special character handling\n- Max length boundary (244 bytes)\n- Empty/single-word descriptions\n\nRisk: HIGH - affects all branch creation\n\nFix:\n- Create test suite with edge cases\n- Test stop-word filtering accuracy\n- Test boundary conditions\n\nSeverity: HIGH","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-24T02:51:00.311664646-05:00","updated_at":"2026-01-02T00:53:35.147800477-05:00","closed_at":"2026-01-02T00:53:35.147800477-05:00","close_reason":"Created test suite with 27 tests covering stop words, acronyms, word limits, special chars, unicode, edge cases, and fallback logic"} {"id":"skills-xcl","title":"Handle malformed JSON in poll() payload parsing","description":"[ERROR] HIGH db.nim:174 - parseJson() can raise on malformed payload, crashes entire poll(). Wrap in try/except, log warning, skip or set payload to none.","status":"closed","priority":1,"issue_type":"bug","created_at":"2026-01-10T18:52:36.218439497-08:00","created_by":"dan","updated_at":"2026-01-10T20:37:04.74037114-08:00","closed_at":"2026-01-10T20:37:04.74037114-08:00","close_reason":"Implemented consistent error handling strategy"} {"id":"skills-xgh0","title":"Add taskId context to parseState error propagation","description":"[ERROR] LOW state.nim:49,90,138,167 - parseState can raise ValueError without context. Catch and re-raise with taskId for debugging.","status":"closed","priority":4,"issue_type":"task","created_at":"2026-01-10T19:49:55.572986746-08:00","created_by":"dan","updated_at":"2026-01-10T20:37:25.462176005-08:00","closed_at":"2026-01-10T20:37:25.462176005-08:00","close_reason":"P4 low priority - stack trace provides context, error message is clear"} +{"id":"skills-y3f2","title":"Agent commands fail when run from worktree (DB path issue)","description":"Agent commands (fail, done, heartbeat) are designed to run from worktrees but openBusDb() uses relative path .worker-state/bus.db which resolves relative to the worktree, not the main repo. Need to either: (1) store main repo path in context, (2) use git to find main worktree, or (3) use absolute paths.","status":"closed","priority":1,"issue_type":"bug","created_at":"2026-01-11T00:22:52.854016416-08:00","created_by":"dan","updated_at":"2026-01-11T00:25:38.389318829-08:00","closed_at":"2026-01-11T00:25:38.389318829-08:00","close_reason":"Fixed: agent commands now use getMainRepoBusDbPath() to find the DB in the main repo when running from worktrees"} {"id":"skills-y76g","title":"Extract render() from status() proc in worker.nim","description":"[BLOAT] LOW worker.nim:49-119 - status() is 70 lines with nested render(). Extract render() to module level or split table formatting into helper.","status":"open","priority":4,"issue_type":"task","created_at":"2026-01-10T20:12:11.728628111-08:00","created_by":"dan","updated_at":"2026-01-10T20:12:11.728628111-08:00"} {"id":"skills-yak","title":"Design: Human observability (status command)","description":"Design: Human observability (status command)\n\nImplementation: Nim (table formatting, watch mode)\nDesign doc: docs/design/human-observability.md\n\nFeatures:\n- worker status - Dashboard table\n- worker show \u003cid\u003e - Detailed view\n- --watch mode - Refresh every 2s\n- --json output for scripting\n- Stale detection: 30s WARN, 100s STALE, 5m DEAD\n\nSee: skills-q40 for language decision","design":"docs/design/human-observability.md","notes":"Design complete. Kubectl/docker-style CLI observability. Commands: status (dashboard table), show (detail view), logs (message history), stats (metrics). Stale detection: 3x heartbeat=WARN, 10x=STALE. Watch mode with --watch. Color-coded states. MVP: status + show; defer logs/stats/TUI.","status":"closed","priority":1,"issue_type":"task","created_at":"2026-01-10T13:55:23.910743917-08:00","created_by":"dan","updated_at":"2026-01-10T21:29:25.675678164-08:00","closed_at":"2026-01-10T21:29:25.675678164-08:00","close_reason":"Implemented in worker CLI - spawn, status, state machine, branch isolation all working","dependencies":[{"issue_id":"skills-yak","depends_on_id":"skills-s6y","type":"blocks","created_at":"2026-01-10T13:55:23.912386443-08:00","created_by":"dan"}]} {"id":"skills-ybq","title":"Reorganize lens directory structure","description":"Current structure puts ops lenses as subdirectory of code-review lenses:\n\n```\n~/.config/lenses/ \u003c- code-review lenses\n~/.config/lenses/ops/ \u003c- ops-review lenses\n```\n\nThis is asymmetric. Consider:\n\nOption A: Separate top-level directories\n```\n~/.config/lenses/code-review/\n~/.config/lenses/ops-review/\n```\n\nOption B: Keep flat but with prefixes\n```\n~/.config/lenses/code-*.md\n~/.config/lenses/ops-*.md\n```\n\nOption C: Per-skill lens directories\n```\n~/.claude/skills/code-review/lenses/\n~/.claude/skills/ops-review/lenses/\n```\n\nRequires updating:\n- modules/ai-skills.nix (deployment paths)\n- skills/code-review/SKILL.md (expected paths)\n- skills/ops-review/SKILL.md (expected paths)","status":"closed","priority":3,"issue_type":"task","created_at":"2026-01-01T21:57:06.726997606-05:00","created_by":"dan","updated_at":"2026-01-02T00:24:53.647409845-05:00","closed_at":"2026-01-02T00:24:53.647409845-05:00","close_reason":"Reorganized lens directories: code-review → ~/.config/lenses/code/, ops-review → ~/.config/lenses/ops/. Updated ai-skills.nix, SKILL.md, and README references."} diff --git a/src/worker.nim b/src/worker.nim index b919ef6..d8c8287 100644 --- a/src/worker.nim +++ b/src/worker.nim @@ -218,7 +218,9 @@ proc start(task: string = "") = ## Signal ASSIGNED → WORKING (run from worktree) let taskId = if task != "": validateTaskId(task) else: getTaskId() - let db = openBusDb() + # Use main repo DB path (works from worktrees) + let dbPath = getMainRepoBusDbPath() + let db = openBusDb(dbPath) defer: db.close() # Check current state @@ -232,7 +234,7 @@ proc start(task: string = "") = quit(ExitSuccess) # Start heartbeat before transition so we're heartbeating when state changes - startGlobalHeartbeat(BusDbPath, taskId) + startGlobalHeartbeat(dbPath, taskId) db.transition(taskId, wsAssigned, wsWorking) updateGlobalHeartbeat(hsWorking, taskId) @@ -243,7 +245,8 @@ proc done(skipRebase: bool = false) = ## Signal WORKING → IN_REVIEW (includes rebase) let ctx = findContext() - let db = openBusDb() + # Use main repo DB path (works from worktrees) + let db = openBusDb(getMainRepoBusDbPath()) defer: db.close() let st = db.getState(ctx.taskId) @@ -291,7 +294,8 @@ proc fail(reason: string) = ## Signal WORKING → FAILED let ctx = findContext() - let db = openBusDb() + # Use main repo DB path (works from worktrees) + let db = openBusDb(getMainRepoBusDbPath()) defer: db.close() db.transitionToFailed(ctx.taskId, reason) @@ -302,7 +306,8 @@ proc sendHeartbeat(status: string = "working", progress: float = 0.0) = ## Emit a single heartbeat (normally done by background thread) let ctx = findContext() - let db = openBusDb() + # Use main repo DB path (works from worktrees) + let db = openBusDb(getMainRepoBusDbPath()) defer: db.close() let hs = case status diff --git a/src/worker/review.nim b/src/worker/review.nim index 49a083f..573fdcd 100644 --- a/src/worker/review.nim +++ b/src/worker/review.nim @@ -3,8 +3,7 @@ ## Shells out to review-gate CLI for review state management. ## Uses taskId as the session ID for consistent mapping. -import std/[os, osproc, json, strutils, strformat, options, streams] -import ./types +import std/[os, osproc, json, strutils, options, streams] import ./utils const @@ -25,12 +24,18 @@ type issues*: seq[string] proc runReviewGate(args: openArray[string]): tuple[output: string, exitCode: int] = - ## Run review-gate command, return output and exit code - let process = startProcess(ReviewGateCmd, args = @args, - options = {poUsePath, poStdErrToStdOut}) - result.output = process.outputStream.readAll() - result.exitCode = process.waitForExit() - process.close() + ## Run review-gate command, return output and exit code. + ## Returns exitCode=-1 if command not found. + try: + let process = startProcess(ReviewGateCmd, args = @args, + options = {poUsePath, poStdErrToStdOut}) + result.output = process.outputStream.readAll() + result.exitCode = process.waitForExit() + process.close() + except OSError: + # review-gate not installed - graceful degradation + result.output = "review-gate not found" + result.exitCode = -1 proc enableReview*(taskId: string): bool = ## Enable review requirement for a task. Returns true on success. diff --git a/src/worker/tests/test-worker.sh b/src/worker/tests/test-worker.sh new file mode 100755 index 0000000..404cd5a --- /dev/null +++ b/src/worker/tests/test-worker.sh @@ -0,0 +1,536 @@ +#!/usr/bin/env bash +# Tests for worker CLI +# Run: bash src/worker/tests/test-worker.sh +# +# Requires: +# - Compiled worker binary at src/worker.out +# - SQLite library available +# - review-gate in PATH (optional, for integration tests) + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +WORKER_BIN="${WORKER_BIN:-$SCRIPT_DIR/../../worker.out}" +REVIEW_GATE="${REVIEW_GATE:-review-gate}" + +# Check binary exists +if [[ ! -x "$WORKER_BIN" ]]; then + echo "ERROR: Worker binary not found at $WORKER_BIN" + echo "Build with: nix-shell -p nim2 sqlite --run 'nim c -d:release src/worker.nim'" + exit 1 +fi + +PASSED=0 +FAILED=0 + +# Colors +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[0;33m' +NC='\033[0m' + +# Isolated test environment +TEST_DIR=$(mktemp -d) +TEST_REPO="$TEST_DIR/repo" +ORIG_DIR="$(pwd)" + +# Set up SQLite library path +SQLITE_LIB=$(nix-build '' -A sqlite.out --no-out-link 2>/dev/null)/lib +export LD_LIBRARY_PATH="${SQLITE_LIB}:${LD_LIBRARY_PATH:-}" + +cleanup() { + cd "$ORIG_DIR" + rm -rf "$TEST_DIR" +} +trap cleanup EXIT + +# Initialize test git repo +setup_test_repo() { + mkdir -p "$TEST_REPO" + cd "$TEST_REPO" + git init -q + git config user.email "test@test.com" + git config user.name "Test User" + echo "# Test Repo" > README.md + git add README.md + git commit -q -m "Initial commit" + + # Create integration branch (worker uses origin/integration by default) + git checkout -q -b integration + git checkout -q master + + # Set up fake origin for fetch operations + git remote add origin . 2>/dev/null || true + + mkdir -p .worker-state + mkdir -p .review-state +} + +# Helpers +assert_exit_code() { + local description="$1" + local expected="$2" + local actual="$3" + + if [[ "$actual" == "$expected" ]]; then + echo -e "${GREEN}PASS${NC}: $description" + ((PASSED++)) + else + echo -e "${RED}FAIL${NC}: $description" + echo " Expected exit code: $expected, Actual: $actual" + ((FAILED++)) + fi +} + +assert_output_contains() { + local description="$1" + local pattern="$2" + local output="$3" + + if echo "$output" | grep -q "$pattern"; then + echo -e "${GREEN}PASS${NC}: $description" + ((PASSED++)) + else + echo -e "${RED}FAIL${NC}: $description" + echo " Pattern not found: '$pattern'" + echo " Output was:" + echo "$output" | sed 's/^/ /' | head -10 + ((FAILED++)) + fi +} + +assert_output_not_contains() { + local description="$1" + local pattern="$2" + local output="$3" + + if ! echo "$output" | grep -q "$pattern"; then + echo -e "${GREEN}PASS${NC}: $description" + ((PASSED++)) + else + echo -e "${RED}FAIL${NC}: $description" + echo " Pattern found but should not be: '$pattern'" + ((FAILED++)) + fi +} + +assert_file_exists() { + local description="$1" + local file="$2" + + if [[ -f "$file" ]]; then + echo -e "${GREEN}PASS${NC}: $description" + ((PASSED++)) + else + echo -e "${RED}FAIL${NC}: $description" + echo " File does not exist: $file" + ((FAILED++)) + fi +} + +assert_dir_exists() { + local description="$1" + local dir="$2" + + if [[ -d "$dir" ]]; then + echo -e "${GREEN}PASS${NC}: $description" + ((PASSED++)) + else + echo -e "${RED}FAIL${NC}: $description" + echo " Directory does not exist: $dir" + ((FAILED++)) + fi +} + +run_worker() { + "$WORKER_BIN" "$@" 2>&1 +} + +run_worker_exit() { + "$WORKER_BIN" "$@" 2>&1 + echo "EXIT:$?" +} + +get_worker_state() { + local task_id="$1" + sqlite3 .worker-state/bus.db "SELECT state FROM workers WHERE task_id='$task_id'" 2>/dev/null || echo "NOT_FOUND" +} + +set_worker_state() { + local task_id="$1" + local state="$2" + sqlite3 .worker-state/bus.db "UPDATE workers SET state='$state' WHERE task_id='$task_id'" 2>/dev/null +} + +echo "=== Worker CLI Tests ===" +echo "Worker: $WORKER_BIN" +echo "Test dir: $TEST_DIR" +echo "" + +# Set up test repo +setup_test_repo + +# --- Spawn Command --- +echo "## Spawn Command" + +output=$(run_worker spawn --taskId=test-001 --description="Test task" --fromBranch=master) +exit_code=$? +assert_exit_code "Spawn succeeds" "0" "$exit_code" +assert_output_contains "Spawn shows task ID" "test-001" "$output" +assert_output_contains "Spawn shows branch" "feat/test-001" "$output" +assert_output_contains "Spawn shows state" "ASSIGNED" "$output" +assert_dir_exists "Worktree created" "worktrees/test-001" +assert_file_exists "Context file created" "worktrees/test-001/.worker-ctx.json" + +# Check DB state +state=$(get_worker_state "test-001") +if [[ "$state" == "ASSIGNED" ]]; then + echo -e "${GREEN}PASS${NC}: DB state is ASSIGNED" + ((PASSED++)) +else + echo -e "${RED}FAIL${NC}: DB state should be ASSIGNED, got: $state" + ((FAILED++)) +fi + +# --- Spawn Duplicate --- +echo "" +echo "## Spawn Duplicate (Idempotent)" + +output=$(run_worker spawn --taskId=test-001) +exit_code=$? +assert_exit_code "Spawn duplicate returns 0" "0" "$exit_code" +assert_output_contains "Shows already exists" "already exists" "$output" + +# --- Spawn Invalid Task ID --- +echo "" +echo "## Spawn Invalid Task ID" + +output=$(run_worker spawn --taskId="../etc/passwd" 2>&1) +exit_code=$? +assert_exit_code "Spawn rejects path traversal" "1" "$exit_code" +assert_output_contains "Shows invalid character error" "invalid character" "$output" + +output=$(run_worker spawn --taskId="" 2>&1) +exit_code=$? +assert_exit_code "Spawn rejects empty ID" "1" "$exit_code" + +# --- Status Command --- +echo "" +echo "## Status Command" + +output=$(run_worker status) +exit_code=$? +assert_exit_code "Status succeeds" "0" "$exit_code" +assert_output_contains "Status shows task" "test-001" "$output" +assert_output_contains "Status shows state" "ASSIGNED" "$output" + +# JSON output +output=$(run_worker status --json) +assert_output_contains "JSON output has task_id" '"task_id"' "$output" +assert_output_contains "JSON output has state" '"state"' "$output" + +# --- Show Command --- +echo "" +echo "## Show Command" + +output=$(run_worker show --taskId=test-001) +exit_code=$? +assert_exit_code "Show succeeds" "0" "$exit_code" +assert_output_contains "Show displays task" "Task: test-001" "$output" +assert_output_contains "Show displays description" "Test task" "$output" +assert_output_contains "Show displays state" "State: ASSIGNED" "$output" +assert_output_contains "Show displays branch" "Branch: feat/test-001" "$output" + +# --- Show Non-existent --- +echo "" +echo "## Show Non-existent Task" + +output=$(run_worker show --taskId=nonexistent 2>&1) +exit_code=$? +assert_exit_code "Show returns 7 for not found" "7" "$exit_code" +assert_output_contains "Shows not found" "not found" "$output" + +# --- Start Command --- +echo "" +echo "## Start Command" + +output=$(run_worker start --task=test-001) +exit_code=$? +assert_exit_code "Start succeeds" "0" "$exit_code" +assert_output_contains "Start confirms" "Started work" "$output" + +state=$(get_worker_state "test-001") +if [[ "$state" == "WORKING" ]]; then + echo -e "${GREEN}PASS${NC}: State transitioned to WORKING" + ((PASSED++)) +else + echo -e "${RED}FAIL${NC}: State should be WORKING, got: $state" + ((FAILED++)) +fi + +# --- Start Already Working --- +echo "" +echo "## Start Already Working (Idempotent)" + +output=$(run_worker start --task=test-001) +exit_code=$? +assert_exit_code "Start when working returns 0" "0" "$exit_code" +assert_output_contains "Shows already working" "Already WORKING" "$output" + +# --- Cancel Command --- +echo "" +echo "## Cancel Command" + +# Create a new task to cancel +run_worker spawn --taskId=test-cancel --fromBranch=master > /dev/null + +output=$(run_worker cancel --taskId=test-cancel --reason="Testing cancel") +exit_code=$? +assert_exit_code "Cancel succeeds" "0" "$exit_code" +assert_output_contains "Cancel confirms" "Cancelled" "$output" + +state=$(get_worker_state "test-cancel") +if [[ "$state" == "FAILED" ]]; then + echo -e "${GREEN}PASS${NC}: State transitioned to FAILED" + ((PASSED++)) +else + echo -e "${RED}FAIL${NC}: State should be FAILED, got: $state" + ((FAILED++)) +fi + +# Cancel with cleanup +run_worker spawn --taskId=test-cleanup --fromBranch=master > /dev/null +output=$(run_worker cancel --taskId=test-cleanup --cleanup) +assert_output_contains "Cleanup confirmed" "Worktree removed" "$output" + +if [[ ! -d "worktrees/test-cleanup" ]]; then + echo -e "${GREEN}PASS${NC}: Worktree removed by cleanup" + ((PASSED++)) +else + echo -e "${RED}FAIL${NC}: Worktree should be removed" + ((FAILED++)) +fi + +# --- Full Lifecycle --- +echo "" +echo "## Full Lifecycle: spawn → start → (simulate done) → approve" + +run_worker spawn --taskId=lifecycle-001 --fromBranch=master > /dev/null + +# Start +run_worker start --task=lifecycle-001 > /dev/null +state=$(get_worker_state "lifecycle-001") +assert_output_contains "After start: WORKING" "WORKING" "$state" + +# Simulate done (manually set to IN_REVIEW since done requires actual git work) +set_worker_state "lifecycle-001" "IN_REVIEW" +state=$(get_worker_state "lifecycle-001") +assert_output_contains "After done: IN_REVIEW" "IN_REVIEW" "$state" + +# Approve +output=$(run_worker approve --taskId=lifecycle-001) +assert_output_contains "Approve confirms" "Approved" "$output" +state=$(get_worker_state "lifecycle-001") +if [[ "$state" == "APPROVED" ]]; then + echo -e "${GREEN}PASS${NC}: After approve: APPROVED" + ((PASSED++)) +else + echo -e "${RED}FAIL${NC}: State should be APPROVED, got: $state" + ((FAILED++)) +fi + +# --- Request Changes --- +echo "" +echo "## Request Changes (Reject)" + +run_worker spawn --taskId=test-reject --fromBranch=master > /dev/null +set_worker_state "test-reject" "IN_REVIEW" + +output=$(run_worker request-changes --taskId=test-reject --comment="Needs more tests") +exit_code=$? +assert_exit_code "Request-changes succeeds" "0" "$exit_code" +assert_output_contains "Shows changes requested" "Changes requested" "$output" +assert_output_contains "Shows comment" "Needs more tests" "$output" + +state=$(get_worker_state "test-reject") +if [[ "$state" == "WORKING" ]]; then + echo -e "${GREEN}PASS${NC}: State returned to WORKING" + ((PASSED++)) +else + echo -e "${RED}FAIL${NC}: State should be WORKING, got: $state" + ((FAILED++)) +fi + +# --- Invalid State Transitions --- +echo "" +echo "## Invalid State Transitions" + +# Try to approve from WORKING state +run_worker spawn --taskId=test-invalid --fromBranch=master > /dev/null +run_worker start --task=test-invalid > /dev/null + +# NOTE: Should return exit code 3 (ExitInvalidTransition) but currently returns 1 +# See skills-lxb9 for fix +output=$(run_worker approve --taskId=test-invalid 2>&1) +exit_code=$? +if [[ "$exit_code" != "0" ]]; then + echo -e "${GREEN}PASS${NC}: Approve from WORKING fails (exit code: $exit_code)" + ((PASSED++)) +else + echo -e "${RED}FAIL${NC}: Approve from WORKING should fail" + ((FAILED++)) +fi + +# --- Review-Gate Integration --- +echo "" +echo "## Review-Gate Integration" + +if command -v "$REVIEW_GATE" &> /dev/null; then + # Spawn creates review state + run_worker spawn --taskId=test-review --fromBranch=master > /dev/null + + if [[ -f ".review-state/test-review.json" ]]; then + echo -e "${GREEN}PASS${NC}: Spawn creates review-gate state" + ((PASSED++)) + + # Check status is pending + review_status=$(jq -r '.status' .review-state/test-review.json) + if [[ "$review_status" == "pending" ]]; then + echo -e "${GREEN}PASS${NC}: Review status is pending" + ((PASSED++)) + else + echo -e "${RED}FAIL${NC}: Review status should be pending, got: $review_status" + ((FAILED++)) + fi + else + echo -e "${YELLOW}SKIP${NC}: Review state file not created (review-gate integration may not be active)" + fi + + # Approve updates review-gate + set_worker_state "test-review" "IN_REVIEW" + run_worker approve --taskId=test-review > /dev/null + + if [[ -f ".review-state/test-review.json" ]]; then + review_status=$(jq -r '.status' .review-state/test-review.json) + if [[ "$review_status" == "approved" ]]; then + echo -e "${GREEN}PASS${NC}: Approve updates review-gate to approved" + ((PASSED++)) + else + echo -e "${RED}FAIL${NC}: Review status should be approved, got: $review_status" + ((FAILED++)) + fi + fi + + # Cancel cleans up review state + run_worker spawn --taskId=test-review-clean --fromBranch=master > /dev/null + run_worker cancel --taskId=test-review-clean > /dev/null + + if [[ ! -f ".review-state/test-review-clean.json" ]]; then + echo -e "${GREEN}PASS${NC}: Cancel cleans up review-gate state" + ((PASSED++)) + else + echo -e "${RED}FAIL${NC}: Review state should be cleaned up after cancel" + ((FAILED++)) + fi +else + echo -e "${YELLOW}SKIP${NC}: review-gate not in PATH, skipping integration tests" +fi + +# --- Heartbeat Command --- +echo "" +echo "## Heartbeat Command" + +run_worker spawn --taskId=test-heartbeat --fromBranch=master > /dev/null +cd worktrees/test-heartbeat + +output=$(run_worker heartbeat --status=working 2>&1) +exit_code=$? +cd "$TEST_REPO" + +assert_exit_code "Heartbeat succeeds" "0" "$exit_code" +assert_output_contains "Heartbeat confirms" "Heartbeat" "$output" + +# --- Fail Command --- +echo "" +echo "## Fail Command" + +run_worker spawn --taskId=test-fail --fromBranch=master > /dev/null +run_worker start --task=test-fail > /dev/null +cd worktrees/test-fail + +output=$(run_worker fail --reason="Test failure reason" 2>&1) +exit_code=$? +cd "$TEST_REPO" + +assert_exit_code "Fail succeeds" "0" "$exit_code" +assert_output_contains "Fail shows task" "test-fail" "$output" +assert_output_contains "Fail shows reason" "Test failure reason" "$output" + +state=$(get_worker_state "test-fail") +if [[ "$state" == "FAILED" ]]; then + echo -e "${GREEN}PASS${NC}: State transitioned to FAILED" + ((PASSED++)) +else + echo -e "${RED}FAIL${NC}: State should be FAILED, got: $state" + ((FAILED++)) +fi + +# --- Status Filtering --- +echo "" +echo "## Status Filtering" + +# Create workers in different states +run_worker spawn --taskId=filter-assigned --fromBranch=master > /dev/null +run_worker spawn --taskId=filter-working --fromBranch=master > /dev/null +run_worker start --task=filter-working > /dev/null + +output=$(run_worker status --state=ASSIGNED) +assert_output_contains "Filter by ASSIGNED shows assigned worker" "filter-assigned" "$output" +assert_output_not_contains "Filter by ASSIGNED hides working worker" "filter-working" "$output" + +output=$(run_worker status --state=WORKING) +assert_output_contains "Filter by WORKING shows working worker" "filter-working" "$output" +assert_output_not_contains "Filter by WORKING hides assigned worker" "filter-assigned" "$output" + +# --- Context File --- +echo "" +echo "## Context File Validation" + +# Check context file content +if [[ -f "worktrees/test-001/.worker-ctx.json" ]]; then + ctx_task=$(jq -r '.task_id' worktrees/test-001/.worker-ctx.json) + ctx_branch=$(jq -r '.branch' worktrees/test-001/.worker-ctx.json) + + if [[ "$ctx_task" == "test-001" ]]; then + echo -e "${GREEN}PASS${NC}: Context file has correct task_id" + ((PASSED++)) + else + echo -e "${RED}FAIL${NC}: Context task_id should be test-001, got: $ctx_task" + ((FAILED++)) + fi + + if [[ "$ctx_branch" == "feat/test-001" ]]; then + echo -e "${GREEN}PASS${NC}: Context file has correct branch" + ((PASSED++)) + else + echo -e "${RED}FAIL${NC}: Context branch should be feat/test-001, got: $ctx_branch" + ((FAILED++)) + fi +else + echo -e "${RED}FAIL${NC}: Context file missing" + ((FAILED++)) + ((FAILED++)) +fi + +# --- Summary --- +echo "" +echo "=== Summary ===" +echo -e "Passed: ${GREEN}$PASSED${NC}" +echo -e "Failed: ${RED}$FAILED${NC}" + +if [[ $FAILED -gt 0 ]]; then + exit 1 +fi + +echo "" +echo -e "${GREEN}All tests passed!${NC}" diff --git a/src/worker/utils.nim b/src/worker/utils.nim index b04876c..ca0a1bd 100644 --- a/src/worker/utils.nim +++ b/src/worker/utils.nim @@ -2,7 +2,7 @@ ## ## Consolidates repeated patterns across modules. -import std/[strformat, times, options] +import std/[strformat, strutils, times, options, os, osproc] import tiny_sqlite import ./types @@ -15,6 +15,30 @@ proc worktreePath*(taskId: string): string = ## Get the worktree path for a task &"{WorktreesDir}/{taskId}" +proc findMainRepoDir*(): string = + ## Find the main repository directory, even when running from a worktree. + ## Uses git to find the common git directory, then derives the main repo. + ## Returns current directory if git fails or we're already in main repo. + let (output, exitCode) = execCmdEx("git rev-parse --git-common-dir") + if exitCode != 0: + return getCurrentDir() + + let gitCommonDir = output.strip() + if gitCommonDir == ".git": + # Already in main repo + return getCurrentDir() + + # gitCommonDir is something like "/path/to/main-repo/.git" + # The main repo is its parent directory + let mainGitDir = if gitCommonDir.isAbsolute: gitCommonDir + else: getCurrentDir() / gitCommonDir + result = parentDir(mainGitDir) + +proc getMainRepoBusDbPath*(): string = + ## Get the absolute path to bus.db in the main repository. + ## Works correctly even when called from a worktree. + findMainRepoDir() / BusDbPath + # Time conversion proc msToTime*(ms: int64): Time = ## Convert epoch milliseconds to Time