feat(worker): integrate review-gate with worker CLI
- Add review.nim module for review-gate integration - spawn: enables review-gate automatically - status: shows review state column (pending/approved/REJECTED) - show: displays review status in detailed view - approve: calls review-gate approve before state transition - request-changes: calls review-gate reject with comment - cancel/merge: clean up review state files Closes skills-byq Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
dan
parent
f9ac03a8a8
commit
caff76f618
|
|
@ -103,7 +103,7 @@
|
|||
{"id":"skills-buh","title":"Document SQLite compile flags in config.nims","description":"[EVOLVE] LOW - SQLite compile flags (SQLITE_THREADSAFE, SQLITE_ENABLE_JSON1, SQLITE_OMIT_LOAD_EXTENSION) are hardcoded. Add comments explaining purpose.","status":"open","priority":4,"issue_type":"task","created_at":"2026-01-10T18:50:54.19875394-08:00","created_by":"dan","updated_at":"2026-01-10T18:50:54.19875394-08:00"}
|
||||
{"id":"skills-bvz","title":"spec-review: Add Definition of Ready checklists for each phase","description":"'Ready for /speckit.plan' and similar are underspecified.\n\nAdd concrete checklists:\n- Spec ready for planning: problem statement, goals, constraints, acceptance criteria, etc.\n- Plan ready for tasks: milestones, risks, dependencies, test strategy, etc.\n- Tasks ready for bd: each task has acceptance criteria, dependencies explicit, etc.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-12-15T00:23:24.877531852-08:00","updated_at":"2025-12-15T14:05:26.880419097-08:00","closed_at":"2025-12-15T14:05:26.880419097-08:00"}
|
||||
{"id":"skills-bww","title":"Benchmark AT-SPI overhead and coverage","description":"## Goal\nMeasure AT-SPI's runtime overhead and coverage across apps.\n\n## Prerequisites\n- Enable `services.gnome.at-spi2-core.enable = true` in NixOS\n- Set `QT_LINUX_ACCESSIBILITY_ALWAYS_ON=1` for Qt apps\n- Rebuild and re-login\n\n## Overhead benchmarks\n1. **Startup time**: App launch with/without AT-SPI\n2. **Memory**: RSS delta with AT-SPI enabled\n3. **CPU**: Idle CPU with AT-SPI bus running\n4. **UI latency**: Input-to-paint latency (if measurable)\n\n## Coverage audit\nFor each app, document:\n- Does it expose accessibility tree?\n- How complete is the tree? (all elements vs partial)\n- Are coordinates accurate?\n- Are element types/roles correct?\n\n### Apps to test\n- [ ] Firefox\n- [ ] Ghostty terminal\n- [ ] Nautilus/file manager\n- [ ] VS Code / Electron app\n- [ ] A Qt app (if any installed)\n\n## Query benchmarks\n- Time to enumerate all elements in a window\n- Time to find element by role/name\n- Memory overhead of pyatspi queries\n\n## Depends on\n- skills-pdg (Enable AT-SPI for UI tree access)","status":"open","priority":2,"issue_type":"task","created_at":"2025-12-17T14:13:21.599259773-08:00","updated_at":"2025-12-17T14:13:21.599259773-08:00","dependencies":[{"issue_id":"skills-bww","depends_on_id":"skills-pdg","type":"blocks","created_at":"2025-12-17T14:13:41.633210539-08:00","created_by":"daemon","metadata":"{}"}]}
|
||||
{"id":"skills-byq","title":"Integrate: review-gate with worker primitives","description":"Connect existing review-gate CLI with new worker system.\n\n## Current state\nreview-gate CLI exists with:\n- check/enable/approve/reject\n- Circuit breaker (3 strikes)\n- Stop hook integration (for Claude)\n\n## Integration needed\n- worker spawn enables review-gate automatically\n- worker status shows review state\n- worker approve/reject wraps review-gate\n- Evidence artifacts feed into review-gate\n\n## File coordination\n.worker-state/X.json includes:\n - review_session_id (links to .review-state/)\n - needs_review: true/false\n - review_status: pending/approved/rejected","notes":"MVP Tier 1: Wire review-gate to worker state machine","status":"open","priority":1,"issue_type":"task","created_at":"2026-01-10T12:15:04.625083755-08:00","created_by":"dan","updated_at":"2026-01-10T15:44:17.993809509-08:00","dependencies":[{"issue_id":"skills-byq","depends_on_id":"skills-s6y","type":"blocks","created_at":"2026-01-10T12:15:10.376067847-08:00","created_by":"dan"}]}
|
||||
{"id":"skills-byq","title":"Integrate: review-gate with worker primitives","description":"Connect existing review-gate CLI with new worker system.\n\n## Current state\nreview-gate CLI exists with:\n- check/enable/approve/reject\n- Circuit breaker (3 strikes)\n- Stop hook integration (for Claude)\n\n## Integration needed\n- worker spawn enables review-gate automatically\n- worker status shows review state\n- worker approve/reject wraps review-gate\n- Evidence artifacts feed into review-gate\n\n## File coordination\n.worker-state/X.json includes:\n - review_session_id (links to .review-state/)\n - needs_review: true/false\n - review_status: pending/approved/rejected","notes":"MVP Tier 1: Wire review-gate to worker state machine","status":"closed","priority":1,"issue_type":"task","created_at":"2026-01-10T12:15:04.625083755-08:00","created_by":"dan","updated_at":"2026-01-10T23:24:21.172713875-08:00","closed_at":"2026-01-10T23:24:21.172713875-08:00","close_reason":"Integrated review-gate with worker: spawn enables review, status/show display review state, approve/reject update review-gate, cancel/merge clean up review state","dependencies":[{"issue_id":"skills-byq","depends_on_id":"skills-s6y","type":"blocks","created_at":"2026-01-10T12:15:10.376067847-08:00","created_by":"dan"}]}
|
||||
{"id":"skills-cc0","title":"spec-review: Add anti-hallucination constraints to prompts","description":"Models may paraphrase and present as quotes, or invent requirements/risks not in the doc.\n\nAdd:\n- 'Quotes must be verbatim'\n- 'Do not assume technologies/constraints not stated'\n- 'If missing info, list as open questions rather than speculating'","status":"closed","priority":3,"issue_type":"task","created_at":"2025-12-15T00:23:26.045478292-08:00","updated_at":"2025-12-15T14:07:19.556888057-08:00","closed_at":"2025-12-15T14:07:19.556888057-08:00"}
|
||||
{"id":"skills-cjx","title":"Create spec-review skill for orch + spec-kit integration","description":"A new skill that integrates orch multi-model consensus with spec-kit workflows.\n\n**Purpose**: Use different models/temps/stances to review spec-kit artifacts before phase transitions.\n\n**Proposed commands**:\n- /spec-review.spec - Critique current spec for completeness, ambiguity, gaps\n- /spec-review.plan - Evaluate architecture decisions in plan\n- /spec-review.gate - Go/no-go consensus before phase transition\n\n**Structure**:\n```\nskills/spec-review/\n├── SKILL.md\n├── commands/\n│ ├── spec.md\n│ ├── plan.md\n│ └── gate.md\n└── prompts/\n └── ...\n```\n\n**Key design points**:\n- Finds spec/plan files from current branch or specs/ directory\n- Invokes orch with appropriate prompt, models, stances\n- Presents consensus/critique results\n- AI reviewing AI is valuable redundancy (different models/temps/stances)\n\n**Dependencies**:\n- orch CLI must be available (blocked on dotfiles-3to)\n- spec-kit project structure conventions","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-12-14T17:50:13.22879874-08:00","updated_at":"2025-12-15T00:10:23.122342449-08:00","closed_at":"2025-12-15T00:10:23.122342449-08:00"}
|
||||
{"id":"skills-cnc","title":"Add direnv helper for per-repo skill deployment","description":"Create sourceable helper script and documentation for the standard per-repo skill deployment pattern using direnv + nix build.","status":"closed","priority":2,"issue_type":"feature","created_at":"2025-11-30T12:19:20.71056749-08:00","updated_at":"2025-11-30T12:37:47.22638278-08:00","closed_at":"2025-11-30T12:37:47.22638278-08:00"}
|
||||
|
|
@ -162,6 +162,7 @@
|
|||
{"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-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"}
|
||||
{"id":"skills-m21","title":"Apply niri-window-capture code review recommendations","description":"CODE-REVIEW-niri-window-capture.md identifies action items: add dependency checks to scripts, improve error handling for niri failures, add screenshot directory validation, implement rate limiting. See High/Medium priority sections.","status":"closed","priority":2,"issue_type":"task","created_at":"2025-11-30T11:58:24.648846875-08:00","updated_at":"2025-12-28T20:16:53.914141949-05:00","closed_at":"2025-12-28T20:16:53.914141949-05:00","close_reason":"Implemented all 4 high-priority recommendations from code review: dependency checks, directory validation, error handling, audit logging"}
|
||||
{"id":"skills-mjf","title":"Design: Portable adversarial reviewer","description":"Design a reviewer agent/skill that can run on any capable model.\n\nalice is Claude Opus with specific tools. We need:\n- Model-agnostic reviewer prompt/instructions\n- Tool requirements (read-only: Read, Grep, Glob, Bash)\n- Integration with orch for multi-model consensus\n- Decision format (APPROVED/ISSUES)\n- Issue filing (beads or tissue)\n\nKey principles from alice:\n- Work for the USER, not the agent\n- Assume errors exist, find them\n- Steel-man then attack\n- Seek second opinions\n\nOutput: Reviewer skill spec that works across agents","status":"closed","priority":3,"issue_type":"task","created_at":"2026-01-09T17:14:20.778647076-08:00","created_by":"dan","updated_at":"2026-01-09T19:59:37.80146821-08:00","closed_at":"2026-01-09T19:59:37.80146821-08:00","close_reason":"Covered in architecture design doc - adversarial reviewer section"}
|
||||
{"id":"skills-ms5","title":"Design: Local message passing layer","description":"Design: Local message passing layer\n\nImplementation: Nim (tiny_sqlite, channels)\nDesign doc: docs/design/message-passing-layer.md (v4)\n\nKey components:\n- SQLite WAL mode as source of truth\n- Heartbeat thread with own connection + channel control\n- Task claims with lease expiry\n- At-least-once delivery with explicit ack\n- JSONL export for debugging\n\nSee: skills-q40 for language decision","design":"docs/design/message-passing-layer.md","notes":"DESIGN UPDATED v2: SQLite as primary storage (not JSONL+flock). After orch consensus (3 models unanimous), aligned with Beads approach. Key change: SQLite with BEGIN IMMEDIATE for atomic writes, WAL mode for concurrency. JSONL becomes read-only export for debugging. See docs/design/message-passing-layer.md and message-passing-comparison.md.","status":"open","priority":2,"issue_type":"task","created_at":"2026-01-10T13:24:21.597509853-08:00","created_by":"dan","updated_at":"2026-01-10T18:04:18.389932002-08:00","dependencies":[{"issue_id":"skills-ms5","depends_on_id":"skills-s6y","type":"blocks","created_at":"2026-01-10T13:24:36.033492407-08:00","created_by":"dan"}]}
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@
|
|||
import std/[os, strformat, json, times, terminal, strutils, sequtils]
|
||||
import cligen
|
||||
import tiny_sqlite
|
||||
import worker/[types, db, state, git, context, heartbeat, utils]
|
||||
import worker/[types, db, state, git, context, heartbeat, utils, review]
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
# Orchestrator Commands
|
||||
|
|
@ -42,10 +42,19 @@ proc spawn(taskId: string, description: string = "",
|
|||
# Create worker in DB
|
||||
discard db.createWorker(taskId, branch, worktree, description)
|
||||
|
||||
echo "Created worker: ", taskId
|
||||
echo " Branch: ", branch
|
||||
echo " Worktree: ", worktree
|
||||
echo " State: ASSIGNED"
|
||||
# Enable review-gate for this task
|
||||
if enableReview(taskId):
|
||||
echo "Created worker: ", taskId
|
||||
echo " Branch: ", branch
|
||||
echo " Worktree: ", worktree
|
||||
echo " State: ASSIGNED"
|
||||
echo " Review: enabled"
|
||||
else:
|
||||
echo "Created worker: ", taskId
|
||||
echo " Branch: ", branch
|
||||
echo " Worktree: ", worktree
|
||||
echo " State: ASSIGNED"
|
||||
echo " Review: (review-gate not available)"
|
||||
|
||||
proc status(state: string = "", stale: bool = false,
|
||||
json: bool = false, watch: bool = false) =
|
||||
|
|
@ -59,12 +68,14 @@ proc status(state: string = "", stale: bool = false,
|
|||
if json:
|
||||
var arr = newJArray()
|
||||
for w in workers:
|
||||
let rs = getReviewState(w.taskId)
|
||||
arr.add(%*{
|
||||
"task_id": w.taskId,
|
||||
"state": $w.state,
|
||||
"branch": w.branch,
|
||||
"stale": w.staleLevel,
|
||||
"age": $(getTime() - w.createdAt)
|
||||
"age": $(getTime() - w.createdAt),
|
||||
"review": $rs.status
|
||||
})
|
||||
echo arr.pretty
|
||||
return
|
||||
|
|
@ -82,9 +93,9 @@ proc status(state: string = "", stale: bool = false,
|
|||
|
||||
# Table header
|
||||
echo ""
|
||||
echo "TASK".alignLeft(14), "STATE".alignLeft(12), "AGE".alignLeft(8),
|
||||
"HEARTBEAT".alignLeft(12), "STATUS".alignLeft(8), "SUMMARY"
|
||||
echo "-".repeat(70)
|
||||
echo "TASK".alignLeft(14), "STATE".alignLeft(12), "REVIEW".alignLeft(10),
|
||||
"AGE".alignLeft(8), "HEARTBEAT".alignLeft(12), "STATUS".alignLeft(8), "SUMMARY"
|
||||
echo "-".repeat(80)
|
||||
|
||||
for w in filtered:
|
||||
let age = getTime() - w.createdAt
|
||||
|
|
@ -98,12 +109,20 @@ proc status(state: string = "", stale: bool = false,
|
|||
hbStr = if hbAge.inMinutes > 0: &"{hbAge.inMinutes}m ago"
|
||||
else: &"{hbAge.inSeconds}s ago"
|
||||
|
||||
let rs = getReviewState(w.taskId)
|
||||
let reviewStr = case rs.status
|
||||
of rsNone: "--"
|
||||
of rsPending: "pending"
|
||||
of rsApproved: "approved"
|
||||
of rsRejected: "REJECTED"
|
||||
|
||||
let staleStr = w.staleLevel
|
||||
let summary = if w.description.len > 30: w.description[0..29] & "..."
|
||||
else: w.description
|
||||
|
||||
echo w.taskId.alignLeft(14),
|
||||
($w.state).alignLeft(12),
|
||||
reviewStr.alignLeft(10),
|
||||
ageStr.alignLeft(8),
|
||||
hbStr.alignLeft(12),
|
||||
staleStr.alignLeft(8),
|
||||
|
|
@ -125,6 +144,9 @@ proc approve(taskId: string) =
|
|||
let db = openBusDb()
|
||||
defer: db.close()
|
||||
|
||||
# Update review-gate state
|
||||
discard approveReview(taskId)
|
||||
|
||||
db.transition(taskId, wsInReview, wsApproved)
|
||||
echo "Approved: ", taskId
|
||||
|
||||
|
|
@ -134,6 +156,9 @@ proc requestChanges(taskId: string, comment: string = "") =
|
|||
let db = openBusDb()
|
||||
defer: db.close()
|
||||
|
||||
# Update review-gate state
|
||||
discard rejectReview(taskId, comment)
|
||||
|
||||
db.transition(taskId, wsInReview, wsWorking)
|
||||
echo "Changes requested: ", taskId
|
||||
if comment != "":
|
||||
|
|
@ -163,6 +188,7 @@ proc merge(taskId: string, deleteBranch: bool = false) =
|
|||
|
||||
# Success
|
||||
db.transition(taskId, wsApproved, wsCompleted)
|
||||
cleanReviewState(taskId)
|
||||
|
||||
if deleteBranch:
|
||||
removeBranch(taskId)
|
||||
|
|
@ -177,6 +203,7 @@ proc cancel(taskId: string, reason: string = "", cleanup: bool = false) =
|
|||
defer: db.close()
|
||||
|
||||
db.transitionToFailed(taskId, reason)
|
||||
cleanReviewState(taskId)
|
||||
echo "Cancelled: ", taskId
|
||||
|
||||
if cleanup:
|
||||
|
|
@ -299,10 +326,13 @@ proc show(taskId: string) =
|
|||
quit(ExitNotFound)
|
||||
|
||||
let w = info.get
|
||||
let rs = getReviewState(w.taskId)
|
||||
|
||||
echo ""
|
||||
echo "Task: ", w.taskId
|
||||
echo "Description: ", w.description
|
||||
echo "State: ", w.state
|
||||
echo "Review: ", reviewStatusStr(rs)
|
||||
echo "Branch: ", w.branch
|
||||
echo "Worktree: ", w.worktree
|
||||
echo ""
|
||||
|
|
|
|||
111
src/worker/review.nim
Normal file
111
src/worker/review.nim
Normal file
|
|
@ -0,0 +1,111 @@
|
|||
## Review-gate integration for worker CLI
|
||||
##
|
||||
## 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 ./utils
|
||||
|
||||
const
|
||||
ReviewGateCmd* = "review-gate"
|
||||
ReviewStateDir* = ".review-state"
|
||||
|
||||
type
|
||||
ReviewStatus* = enum
|
||||
rsNone = "none" # No review registered
|
||||
rsPending = "pending"
|
||||
rsApproved = "approved"
|
||||
rsRejected = "rejected"
|
||||
|
||||
ReviewState* = object
|
||||
sessionId*: string
|
||||
status*: ReviewStatus
|
||||
reason*: Option[string]
|
||||
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()
|
||||
|
||||
proc enableReview*(taskId: string): bool =
|
||||
## Enable review requirement for a task. Returns true on success.
|
||||
let (output, code) = runReviewGate(["enable", taskId])
|
||||
if code != 0:
|
||||
logWarn("enableReview", "failed for " & taskId & ": " & output.strip())
|
||||
return false
|
||||
return true
|
||||
|
||||
proc approveReview*(taskId: string): bool =
|
||||
## Approve review for a task. Returns true on success.
|
||||
let (output, code) = runReviewGate(["approve", taskId])
|
||||
if code != 0:
|
||||
logWarn("approveReview", "failed for " & taskId & ": " & output.strip())
|
||||
return false
|
||||
return true
|
||||
|
||||
proc rejectReview*(taskId: string, reason: string = ""): bool =
|
||||
## Reject review for a task. Returns true on success.
|
||||
var args = @["reject", taskId]
|
||||
if reason != "":
|
||||
args.add(reason)
|
||||
let (output, code) = runReviewGate(args)
|
||||
if code != 0:
|
||||
logWarn("rejectReview", "failed for " & taskId & ": " & output.strip())
|
||||
return false
|
||||
return true
|
||||
|
||||
proc getReviewState*(taskId: string): ReviewState =
|
||||
## Get review state for a task by reading state file directly.
|
||||
## Faster than shelling out for status display.
|
||||
result.sessionId = taskId
|
||||
result.status = rsNone
|
||||
|
||||
let stateFile = ReviewStateDir / (taskId & ".json")
|
||||
if not fileExists(stateFile):
|
||||
return
|
||||
|
||||
try:
|
||||
let content = readFile(stateFile)
|
||||
let j = parseJson(content)
|
||||
|
||||
let statusStr = j{"status"}.getStr("none")
|
||||
result.status = case statusStr
|
||||
of "pending": rsPending
|
||||
of "approved": rsApproved
|
||||
of "rejected": rsRejected
|
||||
else: rsNone
|
||||
|
||||
if j.hasKey("reason"):
|
||||
result.reason = some(j["reason"].getStr())
|
||||
|
||||
if j.hasKey("issues"):
|
||||
for issue in j["issues"]:
|
||||
result.issues.add(issue.getStr())
|
||||
except CatchableError as e:
|
||||
logWarn("getReviewState", "failed to read " & stateFile & ": " & e.msg)
|
||||
|
||||
proc reviewStatusStr*(rs: ReviewState): string =
|
||||
## Format review status for display
|
||||
case rs.status
|
||||
of rsNone: "--"
|
||||
of rsPending: "PENDING"
|
||||
of rsApproved: "APPROVED"
|
||||
of rsRejected:
|
||||
if rs.reason.isSome:
|
||||
"REJECTED: " & rs.reason.get()
|
||||
else:
|
||||
"REJECTED"
|
||||
|
||||
proc cleanReviewState*(taskId: string) =
|
||||
## Remove review state file for a task
|
||||
let stateFile = ReviewStateDir / (taskId & ".json")
|
||||
if fileExists(stateFile):
|
||||
try:
|
||||
removeFile(stateFile)
|
||||
except OSError as e:
|
||||
logWarn("cleanReviewState", "failed to remove " & stateFile & ": " & e.msg)
|
||||
Loading…
Reference in a new issue