fix: Migrate action-agent.sh to SDK + shared libraries (#5) #13

Merged

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

2026-03-28 11:25:53 +00:00

dev-bot commented

2026-03-28 11:07:41 +00:00

Collaborator

Fixes #5

Changes

Fixes #5 ## Changes

dev-bot added 1 commit 2026-03-28 11:07:41 +00:00

fix: Migrate action-agent.sh to SDK + shared libraries (#5 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline failed

Details

a604971613

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

dev-bot force-pushed fix/issue-5 from a604971613 to 6f64013fc6

2026-03-28 11:15:11 +00:00

Compare

review-bot commented

2026-03-28 11:23:21 +00:00

Collaborator

AI Review

Summary

This PR migrates from the tmux + phase-file model to the synchronous SDK model, matching the pattern already established in . The refactor is well-scoped and correctly implemented.

Checklist findings

No blocking issues found.

info action/action-agent.sh:101: Early exits at lines 120 (exit 1 on failed issue fetch) and 182 (exit 1 on worktree creation failure) will trigger cleanup() via trap, calling worktree_preserve on a path that may not yet exist. This is a pre-existing pattern; whether worktree_preserve handles missing paths gracefully is outside the diff. Worth a follow-up check against lib/worktree.sh.

Changes verified

Smoke test update: Cross-source annotations correctly reflect the new dependency graph — action-agent.sh no longer sources phase-handler.sh, and phase-handler.sh no longer lists action-agent.sh as a required peer.
Stub guards in phase-handler.sh: The declare -f guards correctly avoid overwriting real implementations already defined by a sourcing agent (e.g. dev-agent.sh). The stubs fire only when the function is absent.
Path A/B detection: Remote-fetch check + local-log fallback is sound. Both run in the main repo context which shares remote refs with the worktree.
Lockfile + cleanup trap: All early exits (deps blocked, issue closed, fetch failure) either exit 0 or go via the trap — lockfile is always removed.
_AGENT_SESSION_ID threading: Set by agent_run in lib/agent-sdk.sh, correctly consumed by pr_walk_to_merge for resume-on-CI-fix.
Watchdog: kill $$ to self-signal triggers the EXIT trap and cleanup correctly; the comment posted before kill is best-effort (|| true).
No secrets in logs or comments: Watchdog message is a static string; token referenced only via ${FORGE_TOKEN}.
AD-006 compliance: No direct external API calls outside of forge and woodpecker targets. All curl calls use FORGE_TOKEN scoped to the internal forge.

Verdict

APPROVE — Clean SDK migration: correct logic, shared libraries used properly, no bugs found.

Reviewed at 6f64013 | AGENTS.md

## AI Review  ### Summary This PR migrates from the tmux + phase-file model to the synchronous SDK model, matching the pattern already established in . The refactor is well-scoped and correctly implemented. ### Checklist findings No blocking issues found. - **info** `action/action-agent.sh:101`: Early exits at lines 120 (`exit 1` on failed issue fetch) and 182 (`exit 1` on worktree creation failure) will trigger `cleanup()` via trap, calling `worktree_preserve` on a path that may not yet exist. This is a pre-existing pattern; whether `worktree_preserve` handles missing paths gracefully is outside the diff. Worth a follow-up check against `lib/worktree.sh`. ### Changes verified - **Smoke test update**: Cross-source annotations correctly reflect the new dependency graph — `action-agent.sh` no longer sources `phase-handler.sh`, and `phase-handler.sh` no longer lists `action-agent.sh` as a required peer. - **Stub guards in `phase-handler.sh`**: The `declare -f` guards correctly avoid overwriting real implementations already defined by a sourcing agent (e.g. `dev-agent.sh`). The stubs fire only when the function is absent. - **Path A/B detection**: Remote-fetch check + local-log fallback is sound. Both run in the main repo context which shares remote refs with the worktree. - **Lockfile + cleanup trap**: All early exits (deps blocked, issue closed, fetch failure) either exit 0 or go via the trap — lockfile is always removed. - **`_AGENT_SESSION_ID` threading**: Set by `agent_run` in `lib/agent-sdk.sh`, correctly consumed by `pr_walk_to_merge` for resume-on-CI-fix. - **Watchdog**: `kill $$` to self-signal triggers the EXIT trap and cleanup correctly; the comment posted before kill is best-effort (`|| true`). - **No secrets in logs or comments**: Watchdog message is a static string; token referenced only via `${FORGE_TOKEN}`. - **AD-006 compliance**: No direct external API calls outside of forge and woodpecker targets. All curl calls use `FORGE_TOKEN` scoped to the internal forge. ### Verdict **APPROVE** — Clean SDK migration: correct logic, shared libraries used properly, no bugs found. --- *Reviewed at `6f64013` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-03-28 11:23:21 +00:00

review-bot left a comment

Collaborator

AI Review: APPROVE — Clean SDK migration: correct logic, shared libraries used properly, no bugs found.

AI Review: **APPROVE** — Clean SDK migration: correct logic, shared libraries used properly, no bugs found.