fix: proper exit codes for state transition errors
- Add try/except blocks in worker commands to catch WorkerNotFound, InvalidTransition, and StaleState exceptions - Return ExitInvalidTransition (3) for state transition errors - Return ExitNotFound (7) for missing workers - Fix double ROLLBACK bug in state.nim by removing inline ROLLBACKs and letting the except block handle transaction cleanup Closes: skills-lxb9 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
4d7a1d96b0
commit
de84648563
|
|
@ -160,7 +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-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":"in_progress","priority":2,"issue_type":"bug","created_at":"2026-01-11T00:17:51.270060721-08:00","created_by":"dan","updated_at":"2026-01-11T00:26:56.34243995-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"}
|
||||
|
|
|
|||
|
|
@ -147,8 +147,18 @@ proc approve(taskId: string) =
|
|||
# Update review-gate state
|
||||
discard approveReview(taskId)
|
||||
|
||||
db.transition(taskId, wsInReview, wsApproved)
|
||||
echo "Approved: ", taskId
|
||||
try:
|
||||
db.transition(taskId, wsInReview, wsApproved)
|
||||
echo "Approved: ", taskId
|
||||
except WorkerNotFound:
|
||||
echo "Worker not found: ", taskId
|
||||
quit(ExitNotFound)
|
||||
except InvalidTransition as e:
|
||||
echo "Cannot approve: ", e.msg
|
||||
quit(ExitInvalidTransition)
|
||||
except StaleState as e:
|
||||
echo "Cannot approve: ", e.msg
|
||||
quit(ExitInvalidTransition)
|
||||
|
||||
proc requestChanges(taskId: string, comment: string = "") =
|
||||
## Request revisions (IN_REVIEW → WORKING)
|
||||
|
|
@ -159,10 +169,20 @@ proc requestChanges(taskId: string, comment: string = "") =
|
|||
# Update review-gate state
|
||||
discard rejectReview(taskId, comment)
|
||||
|
||||
db.transition(taskId, wsInReview, wsWorking)
|
||||
echo "Changes requested: ", taskId
|
||||
if comment != "":
|
||||
echo " Comment: ", comment
|
||||
try:
|
||||
db.transition(taskId, wsInReview, wsWorking)
|
||||
echo "Changes requested: ", taskId
|
||||
if comment != "":
|
||||
echo " Comment: ", comment
|
||||
except WorkerNotFound:
|
||||
echo "Worker not found: ", taskId
|
||||
quit(ExitNotFound)
|
||||
except InvalidTransition as e:
|
||||
echo "Cannot request changes: ", e.msg
|
||||
quit(ExitInvalidTransition)
|
||||
except StaleState as e:
|
||||
echo "Cannot request changes: ", e.msg
|
||||
quit(ExitInvalidTransition)
|
||||
|
||||
proc merge(taskId: string, deleteBranch: bool = false) =
|
||||
## Merge approved work (APPROVED → COMPLETED)
|
||||
|
|
@ -236,10 +256,18 @@ proc start(task: string = "") =
|
|||
# Start heartbeat before transition so we're heartbeating when state changes
|
||||
startGlobalHeartbeat(dbPath, taskId)
|
||||
|
||||
db.transition(taskId, wsAssigned, wsWorking)
|
||||
updateGlobalHeartbeat(hsWorking, taskId)
|
||||
|
||||
echo "Started work on ", taskId
|
||||
try:
|
||||
db.transition(taskId, wsAssigned, wsWorking)
|
||||
updateGlobalHeartbeat(hsWorking, taskId)
|
||||
echo "Started work on ", taskId
|
||||
except InvalidTransition as e:
|
||||
stopGlobalHeartbeat()
|
||||
echo "Cannot start: ", e.msg
|
||||
quit(ExitInvalidTransition)
|
||||
except StaleState as e:
|
||||
stopGlobalHeartbeat()
|
||||
echo "Cannot start: ", e.msg
|
||||
quit(ExitInvalidTransition)
|
||||
|
||||
proc done(skipRebase: bool = false) =
|
||||
## Signal WORKING → IN_REVIEW (includes rebase)
|
||||
|
|
@ -298,9 +326,19 @@ proc fail(reason: string) =
|
|||
let db = openBusDb(getMainRepoBusDbPath())
|
||||
defer: db.close()
|
||||
|
||||
db.transitionToFailed(ctx.taskId, reason)
|
||||
stopGlobalHeartbeat()
|
||||
echo "Failed: ", ctx.taskId, " - ", reason
|
||||
try:
|
||||
db.transitionToFailed(ctx.taskId, reason)
|
||||
stopGlobalHeartbeat()
|
||||
echo "Failed: ", ctx.taskId, " - ", reason
|
||||
except WorkerNotFound:
|
||||
echo "Worker not found: ", ctx.taskId
|
||||
quit(ExitNotFound)
|
||||
except InvalidTransition as e:
|
||||
echo "Cannot fail: ", e.msg
|
||||
quit(ExitInvalidTransition)
|
||||
except StaleState as e:
|
||||
echo "Cannot fail: ", e.msg
|
||||
quit(ExitInvalidTransition)
|
||||
|
||||
proc sendHeartbeat(status: string = "working", progress: float = 0.0) =
|
||||
## Emit a single heartbeat (normally done by background thread)
|
||||
|
|
|
|||
|
|
@ -43,14 +43,12 @@ proc transition*(db: DbConn, taskId: string, fromState, toState: WorkerState) =
|
|||
)
|
||||
|
||||
if row.isNone:
|
||||
db.exec("ROLLBACK")
|
||||
raise newException(WorkerNotFound, &"Worker not found: {taskId}")
|
||||
|
||||
let currentStr = row.get[0].fromDbValue(string)
|
||||
let current = parseState(currentStr)
|
||||
|
||||
if current != fromState:
|
||||
db.exec("ROLLBACK")
|
||||
raise newException(StaleState, &"Expected {fromState}, got {current}")
|
||||
|
||||
let tsMs = epochMs()
|
||||
|
|
@ -84,14 +82,12 @@ proc transitionToFailed*(db: DbConn, taskId: string, reason: string = "") =
|
|||
)
|
||||
|
||||
if row.isNone:
|
||||
db.exec("ROLLBACK")
|
||||
raise newException(WorkerNotFound, &"Worker not found: {taskId}")
|
||||
|
||||
let currentStr = row.get[0].fromDbValue(string)
|
||||
let current = parseState(currentStr)
|
||||
|
||||
if current == wsCompleted:
|
||||
db.exec("ROLLBACK")
|
||||
raise newException(InvalidTransition, "Cannot fail a completed worker")
|
||||
|
||||
let tsMs = epochMs()
|
||||
|
|
|
|||
Loading…
Reference in a new issue