diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index acb8878..ca62ae1 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -450,13 +450,40 @@ except Exception: exit 1 fi buf="$(echo "$2" | tr -cd 'A-Za-z0-9_.-')" - if [ -z "$buf" ]; then buf="tmp_buffer"; else buf="buf_$buf"; fi + if [ -z "$buf" ]; then + echo "Error: -b name contains no usable characters" >&2 + exit 1 + fi + buf="buf_$buf" shift 2 ;; *) text="$1"; shift ;; esac done - echo -n "$text" > "$wrapper_dir/$buf" + # A-3 GC: unique buffer names mean an abandoned buffer (SIGINT/SIGTERM/ + # timeout between set-buffer and delete-buffer) is never overwritten, so + # it would leak forever. Sweep here -- at creation time, the moment new + # garbage can appear -- rather than from a separate reaper. + # + # SAFETY: the age threshold must comfortably exceed the lifetime of a LIVE + # buffer, or the sweep would delete a buffer awaiting its paste and + # re-create the silent loss A-3 exists to remove. A live buffer lives from + # set-buffer to delete-buffer (sub-second in practice, bounded by the + # blocking `agent send`), so 60 minutes is ~4 orders of magnitude of slack. + # Failures are ignored: garbage collection must never break an injection. + # + # The '.buf_sks_*' arm is not redundant: F2's atomic write stages through + # a DOT-prefixed temp ('.buf_sks_....tmp'), which 'buf_sks_*' cannot match. + # Without it a temp orphaned by SIGKILL between write and rename would leak + # forever, exactly like the buffers this sweep exists to reclaim. + find "$wrapper_dir" \( -name 'buf_sks_*' -o -name '.buf_sks_*' \) \ + -mmin +${MAM_BUFFER_GC_MINUTES:-60} -delete 2>/dev/null || true + _tmp="$wrapper_dir/.$buf.$$.tmp" + if ! { echo -n "$text" > "$_tmp" && mv -f "$_tmp" "$wrapper_dir/$buf"; }; then + rm -f "$_tmp" + echo "Error: failed to write buffer $buf" >&2 + exit 1 + fi ;; paste-buffer) buf="tmp_buffer" @@ -465,7 +492,11 @@ except Exception: case "$1" in -b) buf="$(echo "$2" | tr -cd 'A-Za-z0-9_.-')" - if [ -z "$buf" ]; then buf="tmp_buffer"; else buf="buf_$buf"; fi + if [ -z "$buf" ]; then + echo "Error: -b name contains no usable characters" >&2 + exit 1 + fi + buf="buf_$buf" shift 2 ;; -t) @@ -492,7 +523,11 @@ except Exception: case "$1" in -b) buf="$(echo "$2" | tr -cd 'A-Za-z0-9_.-')" - if [ -z "$buf" ]; then buf="tmp_buffer"; else buf="buf_$buf"; fi + if [ -z "$buf" ]; then + echo "Error: -b name contains no usable characters" >&2 + exit 1 + fi + buf="buf_$buf" shift 2 ;; *) shift ;; @@ -1706,9 +1741,10 @@ send_keys_safe() { sleep 2 done - _sks_herdr set-buffer -b "sks_$job_id" "$text" - _sks_herdr paste-buffer -b "sks_$job_id" -t "$sess" - _sks_herdr delete-buffer -b "sks_$job_id" 2>/dev/null || true + local sks_buf="sks_${sess}_${job_id}_$$_${RANDOM}_$(date +%s%N 2>/dev/null || date +%s)" + _sks_herdr set-buffer -b "$sks_buf" "$text" + _sks_herdr paste-buffer -b "$sks_buf" -t "$sess" + _sks_herdr delete-buffer -b "$sks_buf" 2>/dev/null || true if [[ "$sess" =~ "agy" ]]; then _sks_herdr send-keys -t "$sess" C-m return 0 diff --git a/IMPROVEMENTS.md b/IMPROVEMENTS.md index bf0ff05..f4d60d2 100644 --- a/IMPROVEMENTS.md +++ b/IMPROVEMENTS.md @@ -1,9 +1,9 @@ # πŸ› οΈ Multi-Agent Mux μ’…ν•© κ°œμ„  및 λ―Έν•΄κ²° 과제 백둜그 (`IMPROVEMENTS.md`) -- **μ΅œμ’… 갱신일**: 2026-08-06 (B-3 herdr real binary preflight verification μ™„λ£Œ 반영) +- **μ΅œμ’… 갱신일**: 2026-08-06 (A-3 call-unique buffer isolation & automatic GC μ™„λ£Œ 반영) - **톡합 관리 λŒ€μƒ**: κΈ°μ‘΄ `CODEBASE_REVIEW_REPORT.md` + `OPTIMIZATION.md` -- **총 좔적 λ―Έν•΄κ²° 과제**: **15건** (μ•„ν‚€ν…μ²˜ 2건, μ—£μ§€μΌ€μ΄μŠ€ 7건, μ˜€μΌ€μŠ€νŠΈλ ˆμ΄μ…˜ 2건, λ ˆκ±°μ‹œ μž”μž¬ 4건) -- **μ™„λ£Œλœ 과제**: **6건** (A-1, A-5, B-1, B-3, C-1, O-1) +- **총 좔적 λ―Έν•΄κ²° 과제**: **14건** (μ•„ν‚€ν…μ²˜ 1건, μ—£μ§€μΌ€μ΄μŠ€ 7건, μ˜€μΌ€μŠ€νŠΈλ ˆμ΄μ…˜ 2건, λ ˆκ±°μ‹œ μž”μž¬ 4건) +- **μ™„λ£Œλœ 과제**: **7건** (A-1, A-3, A-5, B-1, B-3, C-1, O-1) --- @@ -13,16 +13,12 @@ --- -## 1. πŸ”΄ μ•„ν‚€ν…μ²˜ 결함 (Architecture Flaws β€” 2건) +## 1. πŸ”΄ μ•„ν‚€ν…μ²˜ 결함 (Architecture Flaws β€” 1건) ### **A-2: 곡개 브둜컀 + HMAC 인증 Off + μ™€μΌλ“œμΉ΄λ“œ μ „νŒŒ** - **ν˜„μƒ**: `mqtt_common.py`의 κΈ°λ³Έ λΈŒλ‘œμ»€κ°€ 곡개 μ„œλ²„(`broker.hivemq.com`), HMAC 무쑰건 True λ°˜ν™˜μœΌλ‘œ μ„€μ •λ˜μ–΄ μžˆμŠ΅λ‹ˆλ‹€. - **νŒŒκΈ‰ 효과**: μ™ΈλΆ€μ—μ„œ μœ μž…λ˜λŠ” malicious `error` 이벀트 μˆ˜μ‹  μ‹œ `reconcile.sh`κ°€ 라이브 μ—μ΄μ „νŠΈ pane을 `kill-session`으둜 κ°•μ œ νŒŒκ΄΄ν•˜λŠ” 치λͺ…적 λ³΄μ•ˆ/μ•ˆμ •μ„± μœ„ν—˜μ΄ μ‘΄μž¬ν•©λ‹ˆλ‹€. -### **A-3: μ‹œν”„νŠΈ 버퍼 단일 파일 곡유 및 λ™μ‹œ μ£Όμž… μ˜€μ—Ό** -- **ν˜„μƒ**: `send_keys_safe` μ‹œν”„νŠΈμ˜ `set/paste/delete-buffer`κ°€ `-b` μ„Έμ…˜ 버퍼 이름을 λ¬΄μ‹œν•˜κ³  단일 `.mam/shim/tmp_buffer` 파일 ν•˜λ‚˜λ§Œμ„ κ³΅μœ ν•©λ‹ˆλ‹€. -- **νŒŒκΈ‰ 효과**: 닀쀑 μ—μ΄μ „νŠΈ λ™μ‹œ μ£Όμž… μ‹œ λŒ€ν™” ν…μŠ€νŠΈ ꡐ차 μ˜€μ—Ό 및 무음 μœ μ‹€(t5 silent loss)이 λ°œμƒν•©λ‹ˆλ‹€. - --- ## 2. 🟠 μ—£μ§€ μΌ€μ΄μŠ€ 및 λŸ°νƒ€μž„ 버그 (Edge-case Bugs β€” 7건) @@ -84,12 +80,18 @@ --- -## 5. πŸŽ‰ μ™„λ£Œλœ 과제 (Completed Tasks β€” 6건) +## 5. πŸŽ‰ μ™„λ£Œλœ 과제 (Completed Tasks β€” 7건) ### **A-1: μ›Œν¬μŠ€νŽ˜μ΄μŠ€ μ„Έμ…˜ 격리 & drift-B μ˜€λ“±λ‘ λ°©μ§€** β€” βœ… μ™„λ£Œ - `derive_workspace_slug` 헬퍼 ν•¨μˆ˜λ₯Ό μΆ”κ°€ν•˜μ—¬ μ›Œν¬μŠ€νŽ˜μ΄μŠ€ 경둜 기반 단일 μ†ŒμΌ“ 슬러그(`mam--`) λ„μΆœ 체계λ₯Ό κ΅¬μΆ•ν–ˆμŠ΅λ‹ˆλ‹€. - `reconcile.sh` drift-B μžλ™ 등둝 μ‹œ foreign cwd 차단 게이트λ₯Ό κ΅¬μΆ•ν•˜μ—¬ μ„Έμ…˜ μ˜€λ“±λ‘μ„ λ°©μ§€ν–ˆμŠ΅λ‹ˆλ‹€. +### **A-3: μ‹œν”„νŠΈ 버퍼 λ™μ‹œ μ£Όμž… μ˜€μ—Ό & μžλ™ GC 체계 ꡬ좕** β€” βœ… μ™„λ£Œ +- `lib.sh` λ‚΄ `send_keys_safe` μ‹œν”„νŠΈ 버퍼 λͺ…λ Ήμ˜ μž„μ‹œ 파일λͺ…을 `sks_${sess}_${job_id}_$$_${RANDOM}_$(date +%s%N)` 식 호좜 λ‹¨μœ„ 독립 ν† ν°μœΌλ‘œ λ³€ν™˜ν•˜μ—¬ 닀쀑 μ—μ΄μ „νŠΈ λ™μ‹œ μ£Όμž… μ‹œ λŒ€ν™” ꡐ차 μ˜€μ—Ό 및 무음 μœ μ‹€(T5)을 μ›μ²œ μ°¨λ‹¨ν–ˆμŠ΅λ‹ˆλ‹€. +- `set-buffer` μ‹œ μ›μžμ  μž„μ‹œ μ“°κΈ°(`.$buf.$$.tmp`) 및 rename(`mv -f`) ꡬ쑰λ₯Ό κ΅¬ν˜„ν•˜μ—¬ νŒŒν‹°μ…œ λ ˆμ½”λ“œ 관츑을 λ°©μ§€ν–ˆμŠ΅λ‹ˆλ‹€. +- μΈν„°λŸ½νŠΈ/μ˜ˆμ™Έ μ’…λ£Œ μ‹œ λ‚¨λŠ” stale 버퍼 νŒŒμΌμ„ μžλ™μœΌλ‘œ μ •λ¦¬ν•˜λŠ” 60λΆ„ λ‚΄μž₯ GC(`find -mmin +60 -delete`)λ₯Ό λ‚΄μž₯ν–ˆμŠ΅λ‹ˆλ‹€. +- μ „μš© νšŒκ·€ ν…ŒμŠ€νŠΈ μŠ€μœ„νŠΈ `tests/test_a3_buffer_isolation.py` (12/12 PASS)λ₯Ό μž‘μ„±ν•˜μ—¬ μž…μ¦ν–ˆμŠ΅λ‹ˆλ‹€. + ### **A-5: `HERDR_SESSION_NAME` λ„€μ΄ν‹°λΈŒ μ „ν™˜** β€” βœ… μ™„λ£Œ - κΈ°μ‘΄ `HERDR_SERVER_NAME` ν™˜κ²½λ³€μˆ˜λ₯Ό herdr μ‹œν”„νŠΈκ°€ 직접 μ½λŠ” λ„€μ΄ν‹°λΈŒ **`HERDR_SESSION_NAME`** 및 YAML λ ˆμ§€μŠ€νŠΈλ¦¬ ν‚€ **`herdr_session`**으둜 μ „μˆ˜ μ „ν™˜ λ‹¨μΌν™”ν–ˆμŠ΅λ‹ˆλ‹€. diff --git a/tests/test_a3_buffer_isolation.py b/tests/test_a3_buffer_isolation.py new file mode 100644 index 0000000..48a9d8e --- /dev/null +++ b/tests/test_a3_buffer_isolation.py @@ -0,0 +1,276 @@ +"""A-3 β€” shift buffer isolation & garbage collection. + +Defects addressed: + β‘  Colliding buffer names (e.g. constant "sks_adhoc" or "sks_onboard") caused + cross-agent prompt contamination and silent buffer deletion loss. + β‘‘ Call-unique buffer names (sks___) fixed β‘  but introduced a + permanent leak of abandoned buffers (SIGINT/SIGTERM/timeout between + set-buffer and delete-buffer). Fixed with automatic GC (-mmin +60). + β‘’ F2's atomic write created DOT-prefixed temp files ('.buf_sks_....tmp') that + F4's initial GC pattern ('buf_sks_*') missed. Fixed by widening the GC + pattern to include '.buf_sks_*' and cleaning up on write/rename failure. +""" +import os +import subprocess +import time + +import pytest + +BARE_PATH = "/usr/bin:/bin:/usr/sbin:/sbin" + + +def _bash(sandbox, snippet, extra_env=None): + env = dict(os.environ) + env["PATH"] = BARE_PATH + env["WORKSPACE_ROOT"] = str(sandbox) + if extra_env: + env.update(extra_env) + script = f'source "{sandbox}/.agents/skills/lib.sh" >/dev/null 2>&1\n{snippet}' + return subprocess.run(["bash", "-c", script], capture_output=True, + text=True, cwd=str(sandbox), env=env) + + +# Harness to invoke send_keys_safe without a running herdr daemon or tmux +def _run_send_keys_safe(sandbox, sess, text, job_id=None): + buflog = sandbox / "buflog.txt" + job_arg = f' "{job_id}"' if job_id is not None else "" + snippet = f""" +_pane_quiescent() {{ return 0; }} +_pane_dialog_open() {{ return 1; }} +_pane_capture() {{ printf '%s' "$SKS_FAKE_PANE"; }} +_sks_herdr() {{ + case "$1" in + set-buffer|paste-buffer|delete-buffer) + [ "$2" = "-b" ] && printf '%s %s\\n' "$1" "$3" >> "{buflog}" ;; + *) + return 0 ;; + esac + return 0 +}} +export SKS_FAKE_PANE="● $text" +send_keys_safe "{sess}" "{text}"{job_arg} +""" + res = _bash(sandbox, snippet) + log_lines = buflog.read_text().splitlines() if buflog.exists() else [] + return res, log_lines + + +# X-1 β€” Two calls with the same job_id (e.g. "onboard") get DIFFERENT buffer names +def test_a3_same_job_id_gets_different_buffer_names(mam_sandbox): + res1, lines1 = _run_send_keys_safe(mam_sandbox, "claude_sess1", "hello", "onboard") + res2, lines2 = _run_send_keys_safe(mam_sandbox, "claude_sess2", "world", "onboard") + + assert res1.returncode == 0 and res2.returncode == 0 + buf1 = lines1[0].split()[1] + buf2 = lines2[-3].split()[1] + assert buf1 != buf2, f"buffer names collided: {buf1} == {buf2}" + + +# X-2 β€” Within one call, set-buffer, paste-buffer, and delete-buffer use the SAME name +def test_a3_triple_uses_same_buffer_name(mam_sandbox): + res, lines = _run_send_keys_safe(mam_sandbox, "claude_sess1", "test payload", "job1") + assert res.returncode == 0 + assert len(lines) == 3 + cmd0, name0 = lines[0].split() + cmd1, name1 = lines[1].split() + cmd2, name2 = lines[2].split() + + assert cmd0 == "set-buffer" and cmd1 == "paste-buffer" and cmd2 == "delete-buffer" + assert name0 == name1 == name2 + + +# X-3 β€” Different buffer names map to different files; delete does not touch another's file +def test_a3_different_names_do_not_interfere(mam_sandbox): + res = _bash(mam_sandbox, """ +SHIM="$WORKSPACE_ROOT/.mam/shim/herdr" +DIR="$WORKSPACE_ROOT/.mam/shim" +"$SHIM" set-buffer -b "buf_A" "PAYLOAD_A" +"$SHIM" set-buffer -b "buf_B" "PAYLOAD_B" + +assert_file() { [ -f "$DIR/$1" ] && echo "$1:EXISTS" || echo "$1:MISSING"; } +assert_file "buf_buf_A" +assert_file "buf_buf_B" + +"$SHIM" delete-buffer -b "buf_A" +assert_file "buf_buf_A" +assert_file "buf_buf_B" +""") + assert "buf_buf_A:EXISTS" in res.stdout + assert "buf_buf_B:EXISTS" in res.stdout + assert "buf_buf_A:MISSING" in res.stdout + assert "buf_buf_B:EXISTS" in res.stdout + + +# X-4 β€” Invalid/empty -b names are rejected explicitly, not falling back to tmp_buffer +def test_a3_unusable_buffer_name_is_rejected(mam_sandbox): + res = _bash(mam_sandbox, """ +SHIM="$WORKSPACE_ROOT/.mam/shim/herdr" +"$SHIM" set-buffer -b "!!!" "PAYLOAD" 2>&1 +echo "RC:$?" +""") + assert "RC:1" in res.stdout + assert "contains no usable characters" in res.stdout + res.stderr + + +# X-5 β€” Parallel execution test: 12 callers get isolated buffer text +def test_a3_parallel_calls_are_isolated(mam_sandbox): + res = _bash(mam_sandbox, """ +SHIM="$WORKSPACE_ROOT/.mam/shim/herdr" +DIR="$WORKSPACE_ROOT/.mam/shim" + +for i in $(seq 1 12); do + "$SHIM" set-buffer -b "par_$i" "DATA_$i" & +done +wait + +for i in $(seq 1 12); do + content=$(cat "$DIR/buf_par_$i" 2>/dev/null) + if [ "$content" != "DATA_$i" ]; then + echo "CORRUPTION in par_$i: got '$content'" + fi +done +echo "PARALLEL:OK" +""") + assert "PARALLEL:OK" in res.stdout + assert "CORRUPTION" not in res.stdout + + +# X-6 β€” Atomic write does not leave .tmp files behind on success +def test_a3_atomic_write_leaves_no_tmp_on_success(mam_sandbox): + res = _bash(mam_sandbox, """ +SHIM="$WORKSPACE_ROOT/.mam/shim/herdr" +DIR="$WORKSPACE_ROOT/.mam/shim" +"$SHIM" set-buffer -b "atomic_test" "ATOMIC_PAYLOAD" +ls -a "$DIR" +""") + assert "buf_atomic_test" in res.stdout + assert not any(f.endswith(".tmp") for f in res.stdout.split()) + + +# X-7 β€” Abandoned buffers (older than GC threshold) are reclaimed +def test_a3_abandoned_buffers_are_collected(mam_sandbox): + res = _bash(mam_sandbox, """ +DIR="$WORKSPACE_ROOT/.mam/shim" +mkdir -p "$DIR" +ABANDONED="$DIR/buf_sks_OLD_1_1" +: > "$ABANDONED" +touch -t 202501010000 "$ABANDONED" + +SHIM="$DIR/herdr" +"$SHIM" set-buffer -b "trigger_gc" "NEW_DATA" + +if [ -f "$ABANDONED" ]; then + echo "GC:LEAK" +else + echo "GC:SWEPT" +fi +""") + assert "GC:SWEPT" in res.stdout + + +# X-8 β€” Live buffers (newer than GC threshold) are preserved +def test_a3_live_buffers_are_preserved(mam_sandbox): + res = _bash(mam_sandbox, """ +DIR="$WORKSPACE_ROOT/.mam/shim" +mkdir -p "$DIR" +LIVE="$DIR/buf_sks_LIVE_2_2" +echo "LIVE_CONTENT" > "$LIVE" + +SHIM="$DIR/herdr" +"$SHIM" set-buffer -b "trigger_gc" "NEW_DATA" + +if [ -f "$LIVE" ]; then + echo "LIVE:PRESERVED" +else + echo "LIVE:DELETED" +fi +""") + assert "LIVE:PRESERVED" in res.stdout + + +# X-9 β€” Sweep is scoped to buf_sks_* and does not touch non-matching files +def test_a3_sweep_is_scoped(mam_sandbox): + res = _bash(mam_sandbox, """ +DIR="$WORKSPACE_ROOT/.mam/shim" +mkdir -p "$DIR" +OTHER="$DIR/other_old_file" +: > "$OTHER" +touch -t 202501010000 "$OTHER" + +SHIM="$DIR/herdr" +"$SHIM" set-buffer -b "trigger_gc" "NEW_DATA" + +if [ -f "$OTHER" ]; then + echo "OTHER:PRESERVED" +else + echo "OTHER:DELETED" +fi +""") + assert "OTHER:PRESERVED" in res.stdout + + +# X-10 β€” Abandoned atomic write DOT-temp files (.buf_sks_*.tmp) are collected +def test_a3_abandoned_dot_temps_are_collected(mam_sandbox): + res = _bash(mam_sandbox, """ +DIR="$WORKSPACE_ROOT/.mam/shim" +mkdir -p "$DIR" +DOT_TEMP="$DIR/.buf_sks_ORPHAN_1_1.99.tmp" +: > "$DOT_TEMP" +touch -t 202501010000 "$DOT_TEMP" + +SHIM="$DIR/herdr" +"$SHIM" set-buffer -b "trigger_gc" "NEW_DATA" + +if [ -f "$DOT_TEMP" ]; then + echo "DOT_TEMP:LEAK" +else + echo "DOT_TEMP:SWEPT" +fi +""") + assert "DOT_TEMP:SWEPT" in res.stdout + + +# X-11 β€” Failed rename cleans up temp file immediately and fails with exit code 1 +def test_a3_failed_rename_cleans_up_and_fails(mam_sandbox): + res = _bash(mam_sandbox, """ +DIR="$WORKSPACE_ROOT/.mam/shim" +mkdir -p "$DIR" +# Block creation of buf_sks_blocked by placing a read-only directory in its place +BLOCK_DIR="$DIR/buf_sks_blocked" +mkdir -p "$BLOCK_DIR" +chmod 500 "$BLOCK_DIR" + +SHIM="$DIR/herdr" +"$SHIM" set-buffer -b "sks_blocked" "PAYLOAD" 2>&1 +RC=$? +echo "RC:$RC" + +# Clean up permissions so test sandbox can clean up +chmod 700 "$BLOCK_DIR" + +# Check if any .tmp files were left behind +ls -a "$DIR" | grep -q '\.tmp$' && echo "TEMP:LEAKED" || echo "TEMP:CLEAN" +""") + assert "RC:1" in res.stdout + assert "TEMP:CLEAN" in res.stdout + + +# X-12 β€” Scoped GC preserves unrelated dot-files +def test_a3_unrelated_dot_files_preserved(mam_sandbox): + res = _bash(mam_sandbox, """ +DIR="$WORKSPACE_ROOT/.mam/shim" +mkdir -p "$DIR" +DOT_OTHER="$DIR/.some_other_dot_file" +: > "$DOT_OTHER" +touch -t 202501010000 "$DOT_OTHER" + +SHIM="$DIR/herdr" +"$SHIM" set-buffer -b "trigger_gc" "NEW_DATA" + +if [ -f "$DOT_OTHER" ]; then + echo "DOT_OTHER:PRESERVED" +else + echo "DOT_OTHER:DELETED" +fi +""") + assert "DOT_OTHER:PRESERVED" in res.stdout