fix(loop): repair review-diff/verdict-parsing bugs, deduplicate session lookups
Applies the P0-P3 fixes from the multi-agent-mux-loop audit (.mam/jobs/ab686e47/claude-reports/report-final.md): - P0-1: capture BASE_COMMIT before Phase 2 and diff against it, so reviewer diffs stay non-empty and cumulative even after the Creator commits per the documented DoD (bare `git diff` alone showed nothing once committed). - P0-2: has_verdict now matches only the report's last non-blank line, so a stray [VERDICT: ...] token quoted mid-report as a formatting example can no longer flip the outcome. - P1-1: replace the English-only refactor/complex/design/architect keyword sniff (dead code against Korean-language reviewer reports) with an explicit [ESCALATE: PLANNER] tag the reviewer prompt now asks for. - P1-2: resolve_all_reviewers/resolve_agent_type/resolve_planner_session now read through lib.sh's load_state_json single source of truth instead of each hand-rolling its own SQLite+YAML lookup; resolve_agent_type's name fallback matches exact hyphen segments instead of a substring `in` check. - P2-1: warn when --all-reviewer and --reviewer are both given, since the latter is silently discarded. - P2-2: correct the SKILL.md CLI-mapping table row that overstated an automated lint gate and an unconditional Planner feedback loop. - P3: fix lib.sh shellcheck SC2164 (unguarded cd in start_watchdog) and annotate the intentional SC2317 dual source/exec guard. Verified: shellcheck clean on both scripts, bash -n syntax OK, and the rewritten has_verdict/resolve_* functions were unit-tested against this repo's live .mam/agent-sessions state. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Claude Sonnet 5
@@ -84,10 +84,20 @@ log_error() {
|
||||
echo -e "\033[1;31m[✗]\033[0m $1"
|
||||
}
|
||||
|
||||
# Verdict must occupy its own line — quoted/diff-embedded tokens ('+', '>' prefixed) never match.
|
||||
# --all-reviewer silently takes precedence over an explicit --reviewer list;
|
||||
# warn so the discarded list isn't mistaken for having been honored (P2-1).
|
||||
if [ "$ALL_REVIEWERS" = true ] && [ -n "$REVIEWER_LIST" ]; then
|
||||
log_warn "--all-reviewer takes precedence; ignoring --reviewer list ('$REVIEWER_LIST')."
|
||||
fi
|
||||
|
||||
# Verdict must occupy the report's last non-blank line — a standalone token
|
||||
# quoted mid-report (e.g. as a formatting example) never matches (P0-2).
|
||||
has_verdict() {
|
||||
local file="$1" verdict="$2"
|
||||
grep -qE "^\[VERDICT: ${verdict}\][[:space:]]*\r?$" "$file"
|
||||
local last_line pattern
|
||||
last_line=$(grep -v '^[[:space:]]*$' "$file" 2>/dev/null | tail -n 1)
|
||||
pattern="^\[VERDICT: ${verdict}\][[:space:]]*\r?\$"
|
||||
[[ "$last_line" =~ $pattern ]]
|
||||
}
|
||||
|
||||
# Helper: Blocking wait for a delegate job's completion or error state (with safety timeout)
|
||||
@@ -128,100 +138,58 @@ except Exception:
|
||||
return 1
|
||||
}
|
||||
|
||||
# Resolve active reviewers from SQL DB or YAML (excluding $TARGET_AGENT)
|
||||
# Resolve active reviewers (excluding $TARGET_AGENT) via lib.sh's
|
||||
# load_state_json — the single source of truth for the merged DB/YAML
|
||||
# session state, instead of hand-rolling a 4th copy of that lookup (P1-2).
|
||||
resolve_all_reviewers() {
|
||||
TARGET_AGENT="$TARGET_AGENT" python3 -c "
|
||||
import sqlite3, os, yaml, json
|
||||
yaml_path = '.mam/agent-sessions.yaml'
|
||||
db_path = '.mam/agent-sessions.db'
|
||||
TARGET_AGENT="$TARGET_AGENT" MAM_STATE_JSON="$(load_state_json)" python3 -c "
|
||||
import os, json
|
||||
d = json.loads(os.environ.get('MAM_STATE_JSON', '{}'))
|
||||
target_agent = os.environ.get('TARGET_AGENT')
|
||||
reviewers = []
|
||||
if os.path.exists(db_path):
|
||||
try:
|
||||
conn = sqlite3.connect(db_path)
|
||||
cursor = conn.execute('SELECT data FROM sessions')
|
||||
for r in cursor.fetchall():
|
||||
s = json.loads(r[0])
|
||||
if s.get('role') == 'reviewer' and s.get('name') != target_agent:
|
||||
reviewers.append(s.get('name'))
|
||||
conn.close()
|
||||
except Exception:
|
||||
pass
|
||||
if not reviewers and os.path.exists(yaml_path):
|
||||
try:
|
||||
with open(yaml_path) as f:
|
||||
d = yaml.safe_load(f) or {}
|
||||
for s in d.get('tmux_sessions', []):
|
||||
if s.get('role') == 'reviewer' and s.get('name') != target_agent:
|
||||
reviewers.append(s.get('name'))
|
||||
except Exception:
|
||||
pass
|
||||
reviewers = [s.get('name') for s in d.get('tmux_sessions', [])
|
||||
if s.get('role') == 'reviewer' and s.get('name') != target_agent]
|
||||
print(','.join(reviewers))
|
||||
"
|
||||
}
|
||||
|
||||
# Resolve target agent type (claude, cline, agy)
|
||||
# Resolve target agent type (claude, cline, agy) via load_state_json.
|
||||
resolve_agent_type() {
|
||||
local name="$1"
|
||||
NAME="$name" python3 -c "
|
||||
import sqlite3, os, yaml, json
|
||||
yaml_path = '.mam/agent-sessions.yaml'
|
||||
db_path = '.mam/agent-sessions.db'
|
||||
NAME="$name" MAM_STATE_JSON="$(load_state_json)" python3 -c "
|
||||
import os, json
|
||||
name = os.environ.get('NAME')
|
||||
d = json.loads(os.environ.get('MAM_STATE_JSON', '{}'))
|
||||
agent = None
|
||||
if os.path.exists(db_path):
|
||||
try:
|
||||
conn = sqlite3.connect(db_path)
|
||||
row = conn.execute('SELECT data FROM sessions WHERE name=?', (name,)).fetchone()
|
||||
if row:
|
||||
agent = json.loads(row[0]).get('agent')
|
||||
conn.close()
|
||||
except Exception:
|
||||
pass
|
||||
if not agent and os.path.exists(yaml_path):
|
||||
try:
|
||||
with open(yaml_path) as f:
|
||||
d = yaml.safe_load(f) or {}
|
||||
for s in d.get('tmux_sessions', []):
|
||||
if s.get('name') == name:
|
||||
agent = s.get('agent') or s.get('pane', {}).get('cmd')
|
||||
break
|
||||
except Exception:
|
||||
pass
|
||||
for s in d.get('tmux_sessions', []):
|
||||
if s.get('name') == name:
|
||||
agent = s.get('agent') or s.get('pane', {}).get('cmd')
|
||||
break
|
||||
if not agent:
|
||||
agent = 'agy' if 'agy' in name else 'cline' if 'cline' in name else 'hermes' if 'hermes' in name else 'claude'
|
||||
# Exact hyphen-segment match, not a naive substring 'in' check, so a
|
||||
# decoy substring inside an unrelated segment can't misclassify.
|
||||
segments = name.split('-')
|
||||
if 'agy' in segments:
|
||||
agent = 'agy'
|
||||
elif 'cline' in segments:
|
||||
agent = 'cline'
|
||||
elif 'hermes' in segments:
|
||||
agent = 'hermes'
|
||||
else:
|
||||
agent = 'claude'
|
||||
print(agent)
|
||||
"
|
||||
}
|
||||
|
||||
# Resolve planner session dynamically
|
||||
# Resolve planner session dynamically via load_state_json.
|
||||
resolve_planner_session() {
|
||||
python3 -c "
|
||||
import sqlite3, os, yaml, json
|
||||
yaml_path = '.mam/agent-sessions.yaml'
|
||||
db_path = '.mam/agent-sessions.db'
|
||||
MAM_STATE_JSON="$(load_state_json)" python3 -c "
|
||||
import os, json
|
||||
d = json.loads(os.environ.get('MAM_STATE_JSON', '{}'))
|
||||
planner = 'canary-projects-multi-agent-mux-planner-claude'
|
||||
if os.path.exists(db_path):
|
||||
try:
|
||||
conn = sqlite3.connect(db_path)
|
||||
row = conn.execute('SELECT name FROM sessions WHERE role=\"planner\" LIMIT 1').fetchone()
|
||||
if row:
|
||||
planner = row[0]
|
||||
conn.close()
|
||||
print(planner)
|
||||
raise SystemExit(0)
|
||||
except Exception:
|
||||
pass
|
||||
if os.path.exists(yaml_path):
|
||||
try:
|
||||
with open(yaml_path) as f:
|
||||
d = yaml.safe_load(f) or {}
|
||||
for s in d.get('tmux_sessions', []):
|
||||
if s.get('role') == 'planner':
|
||||
print(s.get('name'))
|
||||
raise SystemExit(0)
|
||||
except Exception:
|
||||
pass
|
||||
for s in d.get('tmux_sessions', []):
|
||||
if s.get('role') == 'planner':
|
||||
planner = s.get('name')
|
||||
break
|
||||
print(planner)
|
||||
"
|
||||
}
|
||||
@@ -350,6 +318,11 @@ fi
|
||||
# PHASE 2: IMPLEMENTATION (CREATION)
|
||||
# ===========================================================================
|
||||
log_info "=== Phase 2: Code Implementation ==="
|
||||
|
||||
# Fix the pre-implementation commit as the diff baseline so review diffs stay
|
||||
# cumulative and non-empty even after the Creator commits per DoD (P0-1).
|
||||
BASE_COMMIT=$(git rev-parse HEAD 2>/dev/null || echo "")
|
||||
|
||||
EXECUTION_PROMPT="다음 작업 목표를 완성해주세요: $TASK"
|
||||
if [ "$PLAN_MODE" = true ]; then
|
||||
EXECUTION_PROMPT="최종 합의된 다음 계획서에 입각하여 코드를 수정 및 구현해주세요. 계획서:\n$CURRENT_PLAN"
|
||||
@@ -432,14 +405,20 @@ while [ "$loop_count" -le "$MAX_LOOP" ]; do
|
||||
|
||||
for rev in "${REVIEWERS[@]}"; do
|
||||
log_info "Requesting code review from Reviewer '$rev'..."
|
||||
# Cumulative working tree diff (M-6)
|
||||
CHANGES_DIFF=$(git diff 2>/dev/null || echo "No git diff available")
|
||||
# Cumulative diff since BASE_COMMIT (M-6): includes committed AND
|
||||
# uncommitted changes, so it stays non-empty even after the Creator
|
||||
# commits per the documented DoD (bare `git diff` alone would not).
|
||||
if [ -n "$BASE_COMMIT" ]; then
|
||||
CHANGES_DIFF=$(git diff "$BASE_COMMIT" 2>/dev/null || echo "No git diff available")
|
||||
else
|
||||
CHANGES_DIFF=$(git diff 2>/dev/null || echo "No git diff available")
|
||||
fi
|
||||
|
||||
REV_OUTPUT=$(bash .agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job submit \
|
||||
--agent-session "tmux:$rev" \
|
||||
--agent "$(resolve_agent_type "$rev")" \
|
||||
--type "direct" \
|
||||
--prompt "다음 구현 사항(작업 목표: $TASK) 및 누적 변경분(git diff)에 대해 린트, 동작성, 유실 등의 관점에서 교차 코드 리뷰를 수행해주세요. 확인 후 최종 Verdict로 '[VERDICT: PASS]' 혹은 '[VERDICT: NOT PASS]' 태그를 리뷰 리포트 마지막에 단독 행으로 명시적으로 작성해주세요. 변경분:\n$CHANGES_DIFF")
|
||||
--prompt "다음 구현 사항(작업 목표: $TASK) 및 누적 변경분(git diff)에 대해 린트, 동작성, 유실 등의 관점에서 교차 코드 리뷰를 수행해주세요. 확인 후 최종 Verdict로 '[VERDICT: PASS]' 혹은 '[VERDICT: NOT PASS]' 태그를 리뷰 리포트 마지막에 단독 행으로 명시적으로 작성해주세요. 만약 단순 버그 수정으로는 부족하고 설계 변경/재작업 수준의 재계획이 필요하다고 판단되면, 리포트 아무 곳에나 단독 행으로 '[ESCALATE: PLANNER]' 태그도 함께 남겨주세요. 변경분:\n$CHANGES_DIFF")
|
||||
|
||||
REV_JOB_ID=$(extract_job_id "$REV_OUTPUT")
|
||||
if [ -z "$REV_JOB_ID" ]; then
|
||||
@@ -494,9 +473,11 @@ while [ "$loop_count" -le "$MAX_LOOP" ]; do
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Re-planning check for complex refactoring feedback
|
||||
# Re-planning check: rely on the explicit '[ESCALATE: PLANNER]' tag a
|
||||
# reviewer is instructed to emit, rather than sniffing English keywords
|
||||
# (reviewers report in Korean, so keyword matching never fired) (P1-1).
|
||||
COMPLEX_FIX=false
|
||||
if echo "$FEEDBACK_AGGREGATE" | grep -Eiq "refactor|complex|design|architect"; then
|
||||
if echo "$FEEDBACK_AGGREGATE" | grep -qE '^\[ESCALATE: PLANNER\][[:space:]]*\r?$'; then
|
||||
COMPLEX_FIX=true
|
||||
fi
|
||||
|
||||
|
||||
Reference in New Issue
Block a user