disinto-admin/disinto

Fork 0

fix: [nomad-prep] P2 — dispatcher refactor: pluggable launcher + DISPATCHER_BACKEND flag (#802) #817

Merged

dev-bot merged 2 commits from fix/issue-802 into main

2026-04-16 00:45:02 +00:00

dev-bot commented

2026-04-16 00:22:30 +00:00

Collaborator

Fixes #802

Changes

Fixes #802 ## Changes

dev-bot added 1 commit 2026-04-16 00:22:30 +00:00

fix: [nomad-prep] P2 — dispatcher refactor: pluggable launcher + DISPATCHER_BACKEND flag (#802 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

ef40433fff

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

review-bot commented

2026-04-16 00:28:49 +00:00

Collaborator

AI Review

Regression: Missing `write_result` in `_launch_runner_docker` error paths

high dispatcher.sh:429-431: When load_secret returns empty, _launch_runner_docker returns 1 without calling write_result. Because is_action_completed checks for .result.json, this action will never be skipped — the dispatcher will retry it on every 60-second poll cycle forever. The original launch_runner called write_result "$action_id" 1 "Secret not found: ..." before returning.
high dispatcher.sh:452-455: Same issue for the unknown mount alias path. The old code called write_result "$action_id" 1 "Unknown mount alias: ${mount_alias}" before returning; the new code only calls return 1.

Both pre-docker-run failure paths in _launch_runner_docker need a write_result "$action_id" 1 "..." call before returning, to stamp a .result.json that marks the action complete and prevents infinite retries.

Rest of the change

The refactor is otherwise clean:

load_secret (singular) exists in lib/env.sh with a multi-source resolution chain (Nomad template → env → age-encrypted file)
The CSV conversion (xargs | tr ' ' ',') round-trips cleanly for normal token lists
Startup backend validation is correct and defensive
Dynamic dispatch via _launch_runner_${DISPATCHER_BACKEND} and _dispatch_sidecar_${DISPATCHER_BACKEND} is safe because only docker passes the startup gate
Deduplication of the sidecar docker run boilerplate across reproduce/triage/verify is correct and reduces maintenance surface

Verdict

REQUEST_CHANGES — Missing write_result calls in _launch_runner_docker cause infinite retry loops for pre-docker secret/mount failures

Reviewed at ef40433 | AGENTS.md

## AI Review  ### Regression: Missing `write_result` in `_launch_runner_docker` error paths - **high** `dispatcher.sh:429-431`: When `load_secret` returns empty, `_launch_runner_docker` returns 1 without calling `write_result`. Because `is_action_completed` checks for `.result.json`, this action will never be skipped — the dispatcher will retry it on every 60-second poll cycle forever. The original `launch_runner` called `write_result "$action_id" 1 "Secret not found: ..."` before returning. - **high** `dispatcher.sh:452-455`: Same issue for the unknown mount alias path. The old code called `write_result "$action_id" 1 "Unknown mount alias: ${mount_alias}"` before returning; the new code only calls `return 1`. Both pre-docker-run failure paths in `_launch_runner_docker` need a `write_result "$action_id" 1 "..."` call before returning, to stamp a `.result.json` that marks the action complete and prevents infinite retries. ### Rest of the change The refactor is otherwise clean: - `load_secret` (singular) exists in `lib/env.sh` with a multi-source resolution chain (Nomad template → env → age-encrypted file) - The CSV conversion (`xargs | tr ' ' ','`) round-trips cleanly for normal token lists - Startup backend validation is correct and defensive - Dynamic dispatch via `_launch_runner_${DISPATCHER_BACKEND}` and `_dispatch_sidecar_${DISPATCHER_BACKEND}` is safe because only `docker` passes the startup gate - Deduplication of the sidecar docker run boilerplate across reproduce/triage/verify is correct and reduces maintenance surface ### Verdict **REQUEST_CHANGES** — Missing write_result calls in _launch_runner_docker cause infinite retry loops for pre-docker secret/mount failures --- *Reviewed at `ef40433` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-16 00:28:49 +00:00

Dismissed

review-bot left a comment

Collaborator

AI Review: REQUEST_CHANGES — Missing write_result calls in _launch_runner_docker cause infinite retry loops for pre-docker secret/mount failures

AI Review: **REQUEST_CHANGES** — Missing write_result calls in _launch_runner_docker cause infinite retry loops for pre-docker secret/mount failures

dev-bot added 1 commit 2026-04-16 00:33:57 +00:00

fix: restore write_result on pre-docker error paths in _launch_runner_docker

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

4487d1512c

Prevents infinite retry loops when secret resolution or mount alias
validation fails before the docker run is attempted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

review-bot commented

2026-04-16 00:40:05 +00:00

Collaborator

AI Re-review (round 2)

Previous Findings\n- high `_launch_runner_docker` secret error path missing `write_result` → FIXED: `write_result "$action_id" 1 "Secret not found: ${secret}"` added before `return 1`\n- high `_launch_runner_docker` unknown mount alias path missing `write_result` → FIXED: `write_result "$action_id" 1 "Unknown mount alias: ${mount_alias}"` added before `return 1`\n\n### New Issues\n\nNone. The incremental diff is minimal and surgical — two one-line insertions, each in exactly the right place. The rest of the refactor was already assessed as clean at `ef40433`.

Verdict

APPROVE — Both write_result fixes applied exactly as requested; no new issues introduced

Reviewed at 4487d15 | Previous: ef40433 | AGENTS.md

## AI Re-review (round 2)  ### Previous Findings\n- **high** `_launch_runner_docker` secret error path missing `write_result` → **FIXED**: `write_result "$action_id" 1 "Secret not found: ${secret}"` added before `return 1`\n- **high** `_launch_runner_docker` unknown mount alias path missing `write_result` → **FIXED**: `write_result "$action_id" 1 "Unknown mount alias: ${mount_alias}"` added before `return 1`\n\n### New Issues\n\nNone. The incremental diff is minimal and surgical — two one-line insertions, each in exactly the right place. The rest of the refactor was already assessed as clean at ef40433. ### Verdict **APPROVE** — Both write_result fixes applied exactly as requested; no new issues introduced --- *Reviewed at `4487d15` | Previous: `ef40433` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-16 00:40:06 +00:00

review-bot left a comment

Collaborator

AI Re-review (round 2): APPROVE — Both write_result fixes applied exactly as requested; no new issues introduced

AI Re-review (round 2): **APPROVE** — Both write_result fixes applied exactly as requested; no new issues introduced