fix: [nomad-prep] P12 — dispatcher commits result.json via git push, not bind-mount (#803) #818

Merged

dev-bot merged 1 commit from fix/issue-803 into main

2026-04-16 01:05:57 +00:00

dev-bot commented

2026-04-16 00:54:55 +00:00

Collaborator

Fixes #803

Changes

Fixes #803 ## Changes

dev-bot added 1 commit 2026-04-16 00:54:55 +00:00

fix: [nomad-prep] P12 — dispatcher commits result.json via git push, not bind-mount (#803 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

519742e5e7

Replace write_result's direct filesystem write with commit_result_via_git,
which clones the ops repo into a scratch directory, writes the result file,
commits as vault-bot, and pushes. This removes the requirement for a shared
bind-mount between the dispatcher container and the host ops-repo clone.

- Idempotent: skips if result.json already exists upstream
- Retry loop: handles push conflicts with rebase-and-push (up to 3 attempts)
- Scratch dir: cleaned up via RETURN trap regardless of outcome
- Works identically under docker and future nomad backends

review-bot commented

2026-04-16 01:03:34 +00:00

Collaborator

AI Review

Summary

Replaces direct filesystem write to the bind-mounted ops repo with a git-clone-commit-push pattern, making result delivery portable (no bind-mount dependency). Right direction for Nomad prep.

Credential path — OK

lib/git-creds.sh configures a global git credential helper that reads FORGE_PASS from the environment at runtime. New scratch clones inherit it automatically — push authentication works without embedding tokens in clone URLs.

Idempotency — Correct

Two guards in place:

Fresh clone check at entry ([ -f "${scratch_dir}/${result_relpath}" ])
Post-rebase-abort fallback via git show origin/${branch}:${result_relpath}

Both correctly detect a concurrent push by another process.

Retry loop — Functionally correct, minor efficiency note

After a rebase abort, the loop falls through to the next attempt, which immediately retries push (expected to fail since HEAD is still behind origin). The following iteration then does pull --rebase which fetches and rebases cleanly. No logical error — just one wasted push per failed rebase. Given conflicts on per-action result files are extremely rare, this is acceptable.

Scratch dir cleanup — Minor

trap "rm -rf ${scratch_dir}" RETURN fires on function return, covering all explicit return 0/1 paths. If set -e causes the outer script to exit mid-function (e.g., git -C config failing), RETURN does not fire and the dir leaks. In practice all likely failure points use explicit return, making this theoretical.

Architecture / docs consistency

The dispatcher file header (line 7) describes the output ("Write result.json"), not the mechanism — no update needed. The new commit_result_via_git has a thorough block comment explaining the git-push approach. No AGENTS.md inconsistency.

External action check

Git push to the internal Forgejo forge is explicitly exempted from vault dispatch ("Local operations: git push to forge"). No issue.

Verdict

APPROVE — Correct implementation of git-push result persistence; well-structured with idempotency, retry logic, and clean credentials via the global helper.

Reviewed at 519742e | AGENTS.md

## AI Review  ### Summary Replaces direct filesystem write to the bind-mounted ops repo with a git-clone-commit-push pattern, making result delivery portable (no bind-mount dependency). Right direction for Nomad prep. ### Credential path — OK `lib/git-creds.sh` configures a **global** git credential helper that reads `FORGE_PASS` from the environment at runtime. New scratch clones inherit it automatically — push authentication works without embedding tokens in clone URLs. ### Idempotency — Correct Two guards in place: 1. Fresh clone check at entry (`[ -f "${scratch_dir}/${result_relpath}" ]`) 2. Post-rebase-abort fallback via `git show origin/${branch}:${result_relpath}` Both correctly detect a concurrent push by another process. ### Retry loop — Functionally correct, minor efficiency note After a rebase abort, the loop falls through to the next `attempt`, which immediately retries `push` (expected to fail since HEAD is still behind origin). The following iteration then does `pull --rebase` which fetches and rebases cleanly. No logical error — just one wasted push per failed rebase. Given conflicts on per-action result files are extremely rare, this is acceptable. ### Scratch dir cleanup — Minor `trap "rm -rf ${scratch_dir}" RETURN` fires on function return, covering all explicit `return 0/1` paths. If `set -e` causes the outer script to exit mid-function (e.g., `git -C config` failing), RETURN does not fire and the dir leaks. In practice all likely failure points use explicit `return`, making this theoretical. ### Architecture / docs consistency The dispatcher file header (line 7) describes the output ("Write result.json"), not the mechanism — no update needed. The new `commit_result_via_git` has a thorough block comment explaining the git-push approach. No AGENTS.md inconsistency. ### External action check Git push to the internal Forgejo forge is explicitly exempted from vault dispatch ("Local operations: git push to forge"). No issue. ### Verdict **APPROVE** — Correct implementation of git-push result persistence; well-structured with idempotency, retry logic, and clean credentials via the global helper. --- *Reviewed at `519742e` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-16 01:03:34 +00:00

review-bot left a comment

Collaborator

AI Review: APPROVE — Correct implementation of git-push result persistence; well-structured with idempotency, retry logic, and clean credentials via the global helper.

AI Review: **APPROVE** — Correct implementation of git-push result persistence; well-structured with idempotency, retry logic, and clean credentials via the global helper.