fix: address review findings from issue #76
- Fix double-injection bug: flat-file write only when direct tmux inject didn't happen - Fix ci_exhausted href='#' fallback to use CODEBERG_WEB/pulls/N - Remove duplicate $THREAD_FILE in rm command - HTML-escape CI snippet before embedding in <pre> block - notify_ctx falls back to plain matrix_send when no thread exists - Thread root uses HTML-formatted message for consistency - Deduplicate _ci_pipeline_url variable Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
814706bf90
commit
8034b50315
2 changed files with 23 additions and 10 deletions
|
|
@ -81,11 +81,17 @@ notify() {
|
||||||
}
|
}
|
||||||
|
|
||||||
# notify_ctx — Send rich notification with HTML links/context into the issue thread
|
# notify_ctx — Send rich notification with HTML links/context into the issue thread
|
||||||
|
# Falls back to plain matrix_send (which registers a thread root) when no thread exists.
|
||||||
notify_ctx() {
|
notify_ctx() {
|
||||||
local plain="$1" html="$2"
|
local plain="$1" html="$2"
|
||||||
local thread_id=""
|
local thread_id=""
|
||||||
[ -f "${THREAD_FILE}" ] && thread_id=$(cat "$THREAD_FILE" 2>/dev/null || true)
|
[ -f "${THREAD_FILE}" ] && thread_id=$(cat "$THREAD_FILE" 2>/dev/null || true)
|
||||||
matrix_send_ctx "dev" "🔧 #${ISSUE}: ${plain}" "🔧 #${ISSUE}: ${html}" "${thread_id}" 2>/dev/null || true
|
if [ -n "$thread_id" ]; then
|
||||||
|
matrix_send_ctx "dev" "🔧 #${ISSUE}: ${plain}" "🔧 #${ISSUE}: ${html}" "${thread_id}" 2>/dev/null || true
|
||||||
|
else
|
||||||
|
# No thread — fall back to plain send so a thread root is registered
|
||||||
|
matrix_send "dev" "🔧 #${ISSUE}: ${plain}" "" "${ISSUE}" 2>/dev/null || true
|
||||||
|
fi
|
||||||
}
|
}
|
||||||
|
|
||||||
# --- Phase helpers ---
|
# --- Phase helpers ---
|
||||||
|
|
@ -265,7 +271,7 @@ do_merge() {
|
||||||
"✅ PR <a href='${CODEBERG_WEB}/pulls/${pr}'>#${pr}</a> merged! <a href='${CODEBERG_WEB}/issues/${ISSUE}'>Issue #${ISSUE}</a> done."
|
"✅ PR <a href='${CODEBERG_WEB}/pulls/${pr}'>#${pr}</a> merged! <a href='${CODEBERG_WEB}/issues/${ISSUE}'>Issue #${ISSUE}</a> done."
|
||||||
kill_tmux_session
|
kill_tmux_session
|
||||||
cleanup_worktree
|
cleanup_worktree
|
||||||
rm -f "$PHASE_FILE" "$IMPL_SUMMARY_FILE" "$THREAD_FILE" "$THREAD_FILE"
|
rm -f "$PHASE_FILE" "$IMPL_SUMMARY_FILE" "$THREAD_FILE"
|
||||||
exit 0
|
exit 0
|
||||||
else
|
else
|
||||||
log "merge failed (HTTP ${http_code}) — attempting rebase and retry"
|
log "merge failed (HTTP ${http_code}) — attempting rebase and retry"
|
||||||
|
|
@ -874,9 +880,13 @@ echo '{"status":"ready"}' > "$PREFLIGHT_RESULT"
|
||||||
# Create Matrix thread for this issue (or reuse existing one)
|
# Create Matrix thread for this issue (or reuse existing one)
|
||||||
if [ ! -f "${THREAD_FILE}" ] || [ -z "$(cat "$THREAD_FILE" 2>/dev/null)" ]; then
|
if [ ! -f "${THREAD_FILE}" ] || [ -z "$(cat "$THREAD_FILE" 2>/dev/null)" ]; then
|
||||||
ISSUE_URL="${CODEBERG_WEB}/issues/${ISSUE}"
|
ISSUE_URL="${CODEBERG_WEB}/issues/${ISSUE}"
|
||||||
_thread_id=$(matrix_send "dev" "🔧 Issue #${ISSUE}: ${ISSUE_TITLE} — ${ISSUE_URL}" "" "${ISSUE}") || true
|
_thread_id=$(matrix_send_ctx "dev" \
|
||||||
|
"🔧 Issue #${ISSUE}: ${ISSUE_TITLE} — ${ISSUE_URL}" \
|
||||||
|
"🔧 <a href='${ISSUE_URL}'>Issue #${ISSUE}</a>: ${ISSUE_TITLE}") || true
|
||||||
if [ -n "${_thread_id:-}" ]; then
|
if [ -n "${_thread_id:-}" ]; then
|
||||||
printf '%s' "$_thread_id" > "$THREAD_FILE"
|
printf '%s' "$_thread_id" > "$THREAD_FILE"
|
||||||
|
# Register thread root in map for listener dispatch
|
||||||
|
printf '%s\t%s\t%s\t%s\n' "$_thread_id" "dev" "$(date +%s)" "${ISSUE}" >> "${MATRIX_THREAD_MAP:-/tmp/matrix-thread-map}" 2>/dev/null || true
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
notify "tmux session ${SESSION_NAME} started for issue #${ISSUE}: ${ISSUE_TITLE}"
|
notify "tmux session ${SESSION_NAME} started for issue #${ISSUE}: ${ISSUE_TITLE}"
|
||||||
|
|
@ -1110,14 +1120,14 @@ Write PHASE:awaiting_review to the phase file, then stop and wait for review fee
|
||||||
fi
|
fi
|
||||||
|
|
||||||
CI_FIX_COUNT=$(( CI_FIX_COUNT + 1 ))
|
CI_FIX_COUNT=$(( CI_FIX_COUNT + 1 ))
|
||||||
|
_ci_pipeline_url="${WOODPECKER_SERVER}/repos/${WOODPECKER_REPO_ID}/pipeline/${PIPELINE_NUM:-0}"
|
||||||
if [ "$CI_FIX_COUNT" -gt "$MAX_CI_FIXES" ]; then
|
if [ "$CI_FIX_COUNT" -gt "$MAX_CI_FIXES" ]; then
|
||||||
log "CI failure not recoverable after ${CI_FIX_COUNT} fix attempts — escalating"
|
log "CI failure not recoverable after ${CI_FIX_COUNT} fix attempts — escalating"
|
||||||
echo "{\"issue\":${ISSUE},\"pr\":${PR_NUMBER},\"reason\":\"ci_exhausted\",\"step\":\"${FAILED_STEP:-unknown}\",\"attempts\":${CI_FIX_COUNT},\"ts\":\"$(date -u +%Y-%m-%dT%H:%M:%SZ)\"}" \
|
echo "{\"issue\":${ISSUE},\"pr\":${PR_NUMBER},\"reason\":\"ci_exhausted\",\"step\":\"${FAILED_STEP:-unknown}\",\"attempts\":${CI_FIX_COUNT},\"ts\":\"$(date -u +%Y-%m-%dT%H:%M:%SZ)\"}" \
|
||||||
>> "${FACTORY_ROOT}/supervisor/escalations.jsonl"
|
>> "${FACTORY_ROOT}/supervisor/escalations.jsonl"
|
||||||
PIPELINE_URL="${WOODPECKER_SERVER}/repos/${WOODPECKER_REPO_ID}/pipeline/${PIPELINE_NUM:-0}"
|
|
||||||
notify_ctx \
|
notify_ctx \
|
||||||
"CI exhausted after ${CI_FIX_COUNT} attempts — escalated to supervisor" \
|
"CI exhausted after ${CI_FIX_COUNT} attempts — escalated to supervisor" \
|
||||||
"CI exhausted after ${CI_FIX_COUNT} attempts on PR <a href='${PR_URL:-#}'>#${PR_NUMBER}</a> | <a href='${PIPELINE_URL}'>Pipeline</a><br>Step: <code>${FAILED_STEP:-unknown}</code> — escalated to supervisor"
|
"CI exhausted after ${CI_FIX_COUNT} attempts on PR <a href='${PR_URL:-${CODEBERG_WEB}/pulls/${PR_NUMBER}}'>#${PR_NUMBER}</a> | <a href='${_ci_pipeline_url}'>Pipeline</a><br>Step: <code>${FAILED_STEP:-unknown}</code> — escalated to supervisor"
|
||||||
printf 'PHASE:failed\nReason: ci_exhausted after %d attempts\n' "$CI_FIX_COUNT" > "$PHASE_FILE"
|
printf 'PHASE:failed\nReason: ci_exhausted after %d attempts\n' "$CI_FIX_COUNT" > "$PHASE_FILE"
|
||||||
LAST_PHASE_MTIME=$(stat -c %Y "$PHASE_FILE" 2>/dev/null || echo 0)
|
LAST_PHASE_MTIME=$(stat -c %Y "$PHASE_FILE" 2>/dev/null || echo 0)
|
||||||
continue
|
continue
|
||||||
|
|
@ -1134,8 +1144,7 @@ Write PHASE:awaiting_review to the phase file, then stop and wait for review fee
|
||||||
> "/tmp/ci-result-${PROJECT_NAME}-${ISSUE}.txt" 2>/dev/null || true
|
> "/tmp/ci-result-${PROJECT_NAME}-${ISSUE}.txt" 2>/dev/null || true
|
||||||
|
|
||||||
# Notify Matrix with rich CI failure context
|
# Notify Matrix with rich CI failure context
|
||||||
_ci_pipeline_url="${WOODPECKER_SERVER}/repos/${WOODPECKER_REPO_ID}/pipeline/${PIPELINE_NUM:-0}"
|
_ci_snippet=$(printf '%s' "${CI_ERROR_LOG:-}" | tail -5 | head -c 500 | sed 's/&/\&/g; s/</\</g; s/>/\>/g')
|
||||||
_ci_snippet=$(printf '%s' "${CI_ERROR_LOG:-}" | tail -5 | head -c 500)
|
|
||||||
notify_ctx \
|
notify_ctx \
|
||||||
"CI failed on PR #${PR_NUMBER}: step=${FAILED_STEP:-unknown} (attempt ${CI_FIX_COUNT}/${MAX_CI_FIXES})" \
|
"CI failed on PR #${PR_NUMBER}: step=${FAILED_STEP:-unknown} (attempt ${CI_FIX_COUNT}/${MAX_CI_FIXES})" \
|
||||||
"CI failed on PR <a href='${PR_URL:-${CODEBERG_WEB}/pulls/${PR_NUMBER}}'>#${PR_NUMBER}</a> | <a href='${_ci_pipeline_url}'>Pipeline #${PIPELINE_NUM:-?}</a><br>Step: <code>${FAILED_STEP:-unknown}</code> (exit ${FAILED_EXIT:-?})<br>Attempt ${CI_FIX_COUNT}/${MAX_CI_FIXES}<br><pre>${_ci_snippet:-no logs}</pre>"
|
"CI failed on PR <a href='${PR_URL:-${CODEBERG_WEB}/pulls/${PR_NUMBER}}'>#${PR_NUMBER}</a> | <a href='${_ci_pipeline_url}'>Pipeline #${PIPELINE_NUM:-?}</a><br>Step: <code>${FAILED_STEP:-unknown}</code> (exit ${FAILED_EXIT:-?})<br>Attempt ${CI_FIX_COUNT}/${MAX_CI_FIXES}<br><pre>${_ci_snippet:-no logs}</pre>"
|
||||||
|
|
|
||||||
|
|
@ -142,11 +142,9 @@ while true; do
|
||||||
matrix_send "gardener" "✓ received, will act on next poll" "$THREAD_ROOT" >/dev/null 2>&1 || true
|
matrix_send "gardener" "✓ received, will act on next poll" "$THREAD_ROOT" >/dev/null 2>&1 || true
|
||||||
;;
|
;;
|
||||||
dev)
|
dev)
|
||||||
# Write to flat file for backward compat
|
|
||||||
printf '%s\t%s\t%s\n' "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "$SENDER" "$BODY" >> /tmp/dev-escalation-reply
|
|
||||||
|
|
||||||
# Route reply into the dev tmux session using context_tag (issue number)
|
# Route reply into the dev tmux session using context_tag (issue number)
|
||||||
DEV_ISSUE=$(awk -F'\t' -v id="$THREAD_ROOT" '$1 == id {print $4}' "$THREAD_MAP" 2>/dev/null || true)
|
DEV_ISSUE=$(awk -F'\t' -v id="$THREAD_ROOT" '$1 == id {print $4}' "$THREAD_MAP" 2>/dev/null || true)
|
||||||
|
DEV_INJECTED=false
|
||||||
if [ -n "$DEV_ISSUE" ]; then
|
if [ -n "$DEV_ISSUE" ]; then
|
||||||
DEV_SESSION="dev-${PROJECT_NAME}-${DEV_ISSUE}"
|
DEV_SESSION="dev-${PROJECT_NAME}-${DEV_ISSUE}"
|
||||||
DEV_PHASE_FILE="/tmp/dev-session-${PROJECT_NAME}-${DEV_ISSUE}.phase"
|
DEV_PHASE_FILE="/tmp/dev-session-${PROJECT_NAME}-${DEV_ISSUE}.phase"
|
||||||
|
|
@ -166,6 +164,7 @@ Consider this guidance for your current work."
|
||||||
tmux send-keys -t "$DEV_SESSION" "" Enter || true
|
tmux send-keys -t "$DEV_SESSION" "" Enter || true
|
||||||
tmux delete-buffer -b "dev-q-${DEV_ISSUE}" 2>/dev/null || true
|
tmux delete-buffer -b "dev-q-${DEV_ISSUE}" 2>/dev/null || true
|
||||||
rm -f "$DEV_INJECT_TMP"
|
rm -f "$DEV_INJECT_TMP"
|
||||||
|
DEV_INJECTED=true
|
||||||
log "human guidance from ${SENDER} injected into ${DEV_SESSION}"
|
log "human guidance from ${SENDER} injected into ${DEV_SESSION}"
|
||||||
matrix_send "dev" "✓ guidance forwarded to dev session for issue #${DEV_ISSUE}" "$THREAD_ROOT" >/dev/null 2>&1 || true
|
matrix_send "dev" "✓ guidance forwarded to dev session for issue #${DEV_ISSUE}" "$THREAD_ROOT" >/dev/null 2>&1 || true
|
||||||
else
|
else
|
||||||
|
|
@ -180,6 +179,11 @@ Consider this guidance for your current work."
|
||||||
log "dev thread ${THREAD_ROOT:0:20} has no issue mapping"
|
log "dev thread ${THREAD_ROOT:0:20} has no issue mapping"
|
||||||
matrix_send "dev" "✓ received, will act on next poll" "$THREAD_ROOT" >/dev/null 2>&1 || true
|
matrix_send "dev" "✓ received, will act on next poll" "$THREAD_ROOT" >/dev/null 2>&1 || true
|
||||||
fi
|
fi
|
||||||
|
# Only write to flat file when direct injection didn't happen,
|
||||||
|
# to avoid supervisor/gardener poll re-injecting the same message.
|
||||||
|
if [ "$DEV_INJECTED" = false ]; then
|
||||||
|
printf '%s\t%s\t%s\n' "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "$SENDER" "$BODY" >> /tmp/dev-escalation-reply
|
||||||
|
fi
|
||||||
;;
|
;;
|
||||||
review)
|
review)
|
||||||
# Route human questions to persistent review tmux session
|
# Route human questions to persistent review tmux session
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue