fix(refactor): address reviewer findings R-1..R-8, achieve 31/31 test pass

This commit is contained in:
2026-08-05 10:08:59 +09:00
parent 8dcb2b2d9e
commit ddd43ecbea
12 changed files with 684 additions and 58 deletions
+45 -28
View File
@@ -127,7 +127,7 @@ except Exception:
fi
_real_herdr() {
local session="${HERDR_SERVER_NAME:-}"
local session="${HERDR_SESSION_NAME:-}"
if [ "$session" = "default" ]; then
session=""
fi
@@ -547,25 +547,44 @@ PYEOF
}
# Despite the name (kept for caller compatibility — resume/stop/update_yaml_resumed
# all do `HERDR_SERVER_NAME="$(resolve_herdr_workspace "$SESSION_NAME")"`), this
# all do `HERDR_SESSION_NAME="$(resolve_herdr_workspace "$SESSION_NAME")"`), this
# returns the isolated herdr *session* name to use for this MAM session row, not
# a workspace id. Real isolation is `--session <name>` (see `_MAM_SESSION` in the
# generated wrapper) — a workspace label match provides no actual isolation
# since agent/pane commands are server-global regardless of workspace.
resolve_herdr_session() {
local session_name="$1"
MAM_STATE_JSON="$(load_state_json)" SESSION_NAME="$session_name" python3 -c "
local workspace="${2:-}"
MAM_STATE_JSON="$(load_state_json)" SESSION_NAME="$session_name" TARGET_WS="$workspace" python3 -c "
import sys, os, json
name = os.environ['SESSION_NAME']
ws = os.environ.get('TARGET_WS', '').strip()
d = json.loads(os.environ.get('MAM_STATE_JSON', '{}'))
for s in d.get('herdr_sessions', []):
if s.get('name') == name:
print(s.get('herdr_session') or 'default')
print(s.get('herdr_session') or s.get('herdr_server') or s.get('herdr_workspace') or 'default')
sys.exit(0)
fallback = os.environ.get('HERDR_SESSION_NAME', '')
if not fallback or fallback == 'default':
pwd = os.path.abspath(os.getcwd())
fallback = 'mam-' + os.path.basename(pwd).lower().replace('_', '-')
legacy = os.environ.get('HERDR_SERVER_NAME', '')
if legacy and legacy != 'default':
fallback = legacy
if not fallback or fallback == 'default':
if ws:
# derive_workspace_slug Equivalent in python
abs_ws = os.path.abspath(ws)
parent = os.path.basename(os.path.dirname(abs_ws)) or 'workspace'
work = os.path.basename(abs_ws) or 'root'
if parent in ('/', '.'): parent = 'workspace'
if work in ('/', '.'): work = 'root'
slug = f'{parent}-{work}'.lower().replace('_', '-')
import re
slug = re.sub(r'[^a-zA-Z0-9-]', '', slug).lstrip('-')
fallback = f'mam-{slug}' if slug else 'mam-ws'
else:
fallback = 'default'
print('WARN: resolve_herdr_session called without workspace parameter for unregistered session; falling back to default', file=sys.stderr)
print(fallback or 'default')
"
}
@@ -575,24 +594,10 @@ resolve_herdr_workspace() {
}
# ---------------------------------------------------------------------------
# derive_session_name <workspace> <agent>
#
# THE single source of truth for the herdr session name. Rule:
# slug = the two trailing path components of the absolute workspace,
# '_' -> '-', lowercased, joined with '-'
# name = "<slug>-creator-<agent>"
#
# Workspace root 기준 상대 해석. 예:
# $WORKSPACE_ROOT/landing_page/refer_landing_page + claude
# -> landing-page-refer-landing-page-creator-claude
#
# Decision (REVIEW P0-A): the actual workspace basename (refer_landing_page)
# IS included. The hand-written historical entry that dropped it
# (lab-landing-page-creator-claude) was the bug, not the convention.
# Every script and SKILL.md must use exactly this rule.
# derive_workspace_slug <workspace>
# ---------------------------------------------------------------------------
derive_session_name() {
local workspace="$1" agent="$2"
derive_workspace_slug() {
local workspace="${1:-$PWD}"
local abs parent work slug
abs="$(cd "$workspace" 2>/dev/null && pwd)" || abs="$workspace"
parent="$(basename "$(dirname "$abs")" 2>/dev/null || echo "")"
@@ -609,7 +614,22 @@ derive_session_name() {
if [ -z "$slug" ]; then
slug="ws"
fi
printf '%s-creator-%s' "$slug" "$agent"
printf 'mam-%s' "$slug"
}
# ---------------------------------------------------------------------------
# derive_session_name <workspace> <agent>
# ---------------------------------------------------------------------------
derive_session_name() {
local workspace="${1:-$PWD}" agent="${2:-}"
local base_slug
base_slug="$(derive_workspace_slug "$workspace")"
local slug="${base_slug#mam-}"
if [ -n "$agent" ]; then
printf '%s-creator-%s' "$slug" "$agent"
else
printf '%s-creator-' "$slug"
fi
}
# ---------------------------------------------------------------------------
@@ -1494,10 +1514,7 @@ start_watchdog() {
# Waits up to 15 seconds for the agent's TUI to render its welcome screen.
wait_for_tui_ready() {
local sess="$1" agent="$2"
local local_herdr="herdr" i
if [ -n "${HERDR_SERVER_NAME:-}" ] && [ "$HERDR_SERVER_NAME" != "default" ]; then
local_herdr="herdr -L $HERDR_SERVER_NAME"
fi
local i
for i in {1..30}; do
if _pane_dialog_open "$sess"; then