From 720f8ad2240c2c24d24972165d8feed1244190c0 Mon Sep 17 00:00:00 2001 From: Godopu Date: Fri, 14 Aug 2026 09:35:17 +0900 Subject: [PATCH] fix(lib): resolve B-8 send_keys_safe agy verification bypass and add submission retry test --- .../report-d629619a.md | 171 ++++++++++++++++++ .agents/skills/lib.sh | 8 +- .agents/skills/lib_py/agents/__main__.py | 15 ++ .agents/skills/lib_py/agents/adapters/agy.py | 12 ++ .../skills/lib_py/agents/adapters/claude.py | 12 ++ .../skills/lib_py/agents/adapters/cline.py | 12 ++ .agents/skills/lib_py/agents/base.py | 12 ++ .agents/skills/lib_py/agents/input_region.py | 40 ++++ IMPROVEMENTS.md | 4 +- tests/test_b8_send_keys_verification.py | 29 +++ 10 files changed, 307 insertions(+), 8 deletions(-) create mode 100644 .agents/reports/canary-projects-multi-agent-mux-creator-cline/report-d629619a.md create mode 100644 .agents/skills/lib_py/agents/input_region.py create mode 100644 tests/test_b8_send_keys_verification.py diff --git a/.agents/reports/canary-projects-multi-agent-mux-creator-cline/report-d629619a.md b/.agents/reports/canary-projects-multi-agent-mux-creator-cline/report-d629619a.md new file mode 100644 index 0000000..3fe101b --- /dev/null +++ b/.agents/reports/canary-projects-multi-agent-mux-creator-cline/report-d629619a.md @@ -0,0 +1,171 @@ +# Cross-Code Review Report: B-8 send_keys_safe agy Path Bypass Removal + +**Job ID**: d629619a +**Reviewer**: cline (session: herdr:canary-projects-multi-agent-mux-creator-cline) +**Review Target**: B-8 `send_keys_safe` agy unconditional `return 0` bypass removal and verification integrity cross-check +**Base Commit**: f8bfa07 (clean tree except working changes) +**Date**: 2026-08-13 + +--- + +## 1. Executive Summary + +This review verifies the removal of the agy early `return 0` bypass in `send_keys_safe()` and the addition of supporting input-region infrastructure (adapter properties, CLI bridge, `input_region.py` module). The change set includes 5 modified files and 2 new files. + +**Verdict: PASS** — The core B-8 fix is correct: the agy unconditional `return 0` is removed, and agy sessions now go through the C-m submission verification loop with hardened checks. The test confirms returncode 4 on submission failure (not 0). However, the supporting `input_region.py` module has a missing `base.py` property declaration that causes `AttributeError` (instead of the intended `ValueError`) for the hermes adapter. This is a latent bug in infrastructure not yet wired into `send_keys_safe`, so it does not affect the B-8 fix itself. + +--- + +## 2. Change Set + +| File | Change | Lines | +|---|---|---| +| `.agents/skills/lib.sh` | Remove agy early return 0; add agy to skip-paste-check group | -6/+2 | +| `.agents/skills/lib_py/agents/__main__.py` | Add `input-region` CLI command + emit INPUT_* facts | +15 | +| `.agents/skills/lib_py/agents/adapters/agy.py` | Add input_prompt/placeholder/rule_pattern | +12 | +| `.agents/skills/lib_py/agents/adapters/claude.py` | Add input_prompt/placeholder/rule_pattern | +12 | +| `.agents/skills/lib_py/agents/adapters/cline.py` | Add input_prompt/placeholder/rule_pattern | +12 | +| `.agents/skills/lib_py/agents/input_region.py` | NEW: rule-based input region extraction | +40 | +| `tests/test_b8_send_keys_verification.py` | NEW: bash mock test for agy submission failure | +29 | + +--- + +## 3. Core B-8 Fix Analysis (lib.sh) + +### Before (lines 1587-1591, removed): +```bash +if [[ "$sess" =~ "agy" ]]; then + _sks_herdr send-keys -t "$sess" C-m + return 0 +fi +``` +This was the B-8 bug: agy sessions blindly returned 0 (success) after sending C-m, without verifying submission. + +### After (line 1588): +```bash +if [[ "$sess" =~ "cline" ]] || [[ "$sess" =~ "claude" ]] || [[ "$sess" =~ "agy" ]]; then + # Skip strict paste check due to scrollout false-positives, proceed to C-m submission loop + true +``` + +agy is now grouped with cline/claude: paste visibility check is skipped (due to scrollout false-positives), but the session goes through the full C-m submission verification loop (lines 1607-1626). If submission is not accepted after 3 tries, `return 4` is executed instead of `return 0`. + +**Verification**: The test `test_send_keys_safe_b8_verification` mocks `_pane_capture` to return static content (no execution token, no pane change) and confirms `send_keys_safe` returns 4 with "Enter not accepted after 3 tries" in stderr. ✅ + +--- + +## 4. Supporting Infrastructure Analysis + +### 4.1 Adapter Properties (agy.py, claude.py, cline.py) + +Each adapter now defines `input_prompt`, `input_placeholder`, and `input_rule_pattern`: + +| Adapter | input_prompt | input_placeholder | input_rule_pattern | +|---|---|---|---| +| agy | `>` | `` | `─{10,}` | +| claude | `❯` | `` | `─{10,}` | +| cline | `❯` | `Ask anything...` | `─{10,}` | + +These are correct and consistent with the agent TUI input area layouts. ✅ + +### 4.2 CLI Bridge (__main__.py) + +New `input-region` command reads pane text from stdin and calls `extract_input_region()`. On success, prints region and exits 0. On exception, prints error to stderr and exits 5. + +The `facts` command now also emits `INPUT_PROMPT`, `INPUT_PLACEHOLDER`, `INPUT_RULE_PATTERN` for shell evaluation. ✅ + +**Manual verification**: +- `input-region agy` with rule-delimited pane → outputs "test prompt", RC=0 ✅ +- `input-region cline` with multi-line + placeholder → outputs "line 1\nline 2\nmarker-xyz", RC=0 ✅ +- `input-region hermes` → outputs `AttributeError` message, RC=5 ⚠️ (see F-1) + +### 4.3 input_region.py + +Rule-based extraction: finds horizontal rule separators (`─{10,}`), extracts text between the last two rules (excluding rule lines themselves via `lines[rule_indices[-2] + 1:rule_indices[-1]]`), strips prompt tokens and placeholders. + +Key design choices (correct): +- Excludes rule lines from region: `+1` offset on start index ✅ +- Falls back to full `lines` when no rules found (supports mocks/unruled panes) ✅ +- Checks `not adapter.input_prompt` for unsupported agent detection ✅ +- Uses `adapter.input_rule_pattern or r'─{10,}'` fallback ✅ + +### 4.4 Test (test_b8_send_keys_verification.py) + +Bash mock test that sources `lib.sh`, mocks `_pane_quiescent`, `_pane_dialog_open`, `_sks_herdr`, and `_pane_capture`, then runs `send_keys_safe` for an agy session. Expects returncode 4 and "Enter not accepted after 3 tries" in stderr. + +Note: `_pane_tail` is NOT mocked — it calls `_pane_capture` internally (line 1508), which IS mocked, so this works correctly. ✅ + +--- + +## 5. Findings + +### F-1 (Medium) — Missing input_prompt/input_placeholder/input_rule_pattern in base.py + +**Location**: `.agents/skills/lib_py/agents/base.py` (not modified) +**Observation**: `BaseAgentAdapter` does not define `input_prompt`, `input_placeholder`, or `input_rule_pattern` properties. Only agy, claude, and cline adapters override them. `HermesAgentAdapter` inherits from `BaseAgentAdapter` without override. +**Impact**: When `input_region.py` line 14 accesses `adapter.input_prompt` on a hermes adapter, it raises `AttributeError: 'HermesAgentAdapter' object has no attribute 'input_prompt'` instead of the intended `ValueError("Agent 'hermes' has no input area facts")`. The CLI bridge catches `Exception` broadly so it still exits with code 5, but the error message is unhelpful. +**Verified**: `hasattr(get_adapter('hermes'), 'input_prompt')` returns `False`. +**Fix**: Add to `base.py`: +```python +@property +def input_prompt(self) -> Optional[str]: + return None +@property +def input_placeholder(self) -> Optional[str]: + return None +@property +def input_rule_pattern(self) -> Optional[str]: + return None +``` +**Severity assessment**: Medium — latent bug in new infrastructure. Does not affect the B-8 fix itself (d629619a's lib.sh does not call `input-region`), but will surface when the infrastructure is wired in. + +### F-2 (Low) — Usage string in __main__.py doesn't mention input-region + +**Location**: `.agents/skills/lib_py/agents/__main__.py:8` +**Observation**: Usage string says `Usage: python -m lib_py.agents [args...]` but `input-region` is now a supported command. +**Fix**: Update to `Usage: python -m lib_py.agents [args...]` + +### F-3 (Info) — input_region.py not wired into send_keys_safe + +**Observation**: The `input_region.py` module and CLI bridge are added but not called from `send_keys_safe()`. The lib.sh change is purely the agy early-return removal. The infrastructure is prepared for future integration (as seen in the alternative 0d0147de implementation). +**Assessment**: Acceptable incremental approach. The B-8 fix is self-contained. + +### F-4 (Info) — [A-Za-z]+ing fast-path regex retained + +**Location**: `.agents/skills/lib.sh:1614` +**Observation**: The submission loop still uses `grep -Eq "● |✽ |[A-Za-z]+ing"` as a fast-path to return 0. This pattern can match false positives (any English word ending in "ing": "Running", "string", "thing", etc.). +**Assessment**: Pre-existing issue, NOT introduced by this change. The 0d0147de alternative removes this fast-path. Non-blocking for d629619a. + +--- + +## 6. Test Results + +| Test | Result | +|---|---| +| `test_b8_send_keys_verification.py::test_send_keys_safe_b8_verification` | PASS | +| `test_a4_adapter_contract.py::test_resolve_home_contract` | PASS | +| `test_a4_adapter_contract.py::test_agent_adapter_registry` | PASS | +| `test_a4_adapter_contract.py::test_agent_of_row_priority` | PASS | + +### Static Analysis + +| Check | Result | +|---|---| +| `bash -n .agents/skills/lib.sh` | PASS | +| `py_compile` all 6 Python files | PASS | +| CLI `input-region agy` manual test | PASS (RC=0, correct output) | +| CLI `input-region cline` manual test | PASS (RC=0, correct output) | +| CLI `input-region hermes` manual test | RC=5 but AttributeError (F-1) | +| `flake8` | Not available (non-blocking) | + +--- + +## 7. Conclusion + +The B-8 fix correctly removes the agy unconditional `return 0` bypass. agy sessions now go through the same C-m submission verification loop as cline and claude, with failure returning 4 instead of 0. The test validates this behavior. + +The supporting input-region infrastructure (adapter properties, CLI bridge, `input_region.py`) is functionally correct for supported agents (agy, claude, cline) but has a missing `base.py` property declaration that causes `AttributeError` for hermes (F-1, Medium). This should be fixed before the infrastructure is wired into `send_keys_safe`. + +No design-level rework or escalation is needed. + +[VERDICT: PASS] diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index b2f86bf..5a83c37 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -1584,13 +1584,9 @@ send_keys_safe() { _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 - fi local was_popup=0 - if [[ "$sess" =~ "cline" ]] || [[ "$sess" =~ "claude" ]]; then - # Skip strict paste check due to scrollout false-positives, proceed to C-m loop + if [[ "$sess" =~ "cline" ]] || [[ "$sess" =~ "claude" ]] || [[ "$sess" =~ "agy" ]]; then + # Skip strict paste check due to scrollout false-positives, proceed to C-m submission loop true else sleep 0.5 diff --git a/.agents/skills/lib_py/agents/__main__.py b/.agents/skills/lib_py/agents/__main__.py index ee7da18..d4f425c 100644 --- a/.agents/skills/lib_py/agents/__main__.py +++ b/.agents/skills/lib_py/agents/__main__.py @@ -18,6 +18,9 @@ def main(): # Emit shell-eval friendly facts print(f"AGENT_NAME={adapter.name}") print(f"OWN_KEY={adapter.own_key}") + print(f"INPUT_PROMPT={adapter.input_prompt or ''}") + print(f"INPUT_PLACEHOLDER={adapter.input_placeholder or ''}") + print(f"INPUT_RULE_PATTERN={adapter.input_rule_pattern or ''}") elif cmd == 'resolve': name = sys.argv[2] if len(sys.argv) > 2 else '' @@ -27,6 +30,18 @@ def main(): sys.exit(0) sys.exit(1) + elif cmd == 'input-region': + agent_name = sys.argv[2] if len(sys.argv) > 2 else '' + pane_text = sys.stdin.read() + from lib_py.agents.input_region import extract_input_region + try: + region = extract_input_region(pane_text, agent_name) + print(region) + sys.exit(0) + except Exception as ex: + print(f"ERROR: {ex}", file=sys.stderr) + sys.exit(5) + else: print(f"ERROR: Unknown CLI command {cmd!r}", file=sys.stderr) sys.exit(1) diff --git a/.agents/skills/lib_py/agents/adapters/agy.py b/.agents/skills/lib_py/agents/adapters/agy.py index b108753..361e882 100644 --- a/.agents/skills/lib_py/agents/adapters/agy.py +++ b/.agents/skills/lib_py/agents/adapters/agy.py @@ -10,3 +10,15 @@ class AgyAgentAdapter(BaseAgentAdapter): @property def own_key(self) -> str: return 'agy_conversation_id_own' + + @property + def input_prompt(self) -> str: + return '>' + + @property + def input_placeholder(self) -> str: + return '' + + @property + def input_rule_pattern(self) -> str: + return '─{10,}' diff --git a/.agents/skills/lib_py/agents/adapters/claude.py b/.agents/skills/lib_py/agents/adapters/claude.py index 1e61a7c..d13cac9 100644 --- a/.agents/skills/lib_py/agents/adapters/claude.py +++ b/.agents/skills/lib_py/agents/adapters/claude.py @@ -10,3 +10,15 @@ class ClaudeAgentAdapter(BaseAgentAdapter): @property def own_key(self) -> str: return 'claude_session_id_own' + + @property + def input_prompt(self) -> str: + return '❯' + + @property + def input_placeholder(self) -> str: + return '' + + @property + def input_rule_pattern(self) -> str: + return '─{10,}' diff --git a/.agents/skills/lib_py/agents/adapters/cline.py b/.agents/skills/lib_py/agents/adapters/cline.py index ad6629c..f021702 100644 --- a/.agents/skills/lib_py/agents/adapters/cline.py +++ b/.agents/skills/lib_py/agents/adapters/cline.py @@ -10,3 +10,15 @@ class ClineAgentAdapter(BaseAgentAdapter): @property def own_key(self) -> str: return 'cline_conversation_id_own' + + @property + def input_prompt(self) -> str: + return '❯' + + @property + def input_placeholder(self) -> str: + return 'Ask anything...' + + @property + def input_rule_pattern(self) -> str: + return '─{10,}' diff --git a/.agents/skills/lib_py/agents/base.py b/.agents/skills/lib_py/agents/base.py index 9088846..d3cab56 100644 --- a/.agents/skills/lib_py/agents/base.py +++ b/.agents/skills/lib_py/agents/base.py @@ -27,6 +27,18 @@ class BaseAgentAdapter: def own_key(self) -> str: raise NotImplementedError + @property + def input_prompt(self) -> Optional[str]: + return None + + @property + def input_placeholder(self) -> Optional[str]: + return None + + @property + def input_rule_pattern(self) -> Optional[str]: + return None + def derive_session_name(self, slug: str, role: str = "creator") -> str: r = role.lower() return f"{slug}-{r}-{self.name}" diff --git a/.agents/skills/lib_py/agents/input_region.py b/.agents/skills/lib_py/agents/input_region.py new file mode 100644 index 0000000..e627bb9 --- /dev/null +++ b/.agents/skills/lib_py/agents/input_region.py @@ -0,0 +1,40 @@ +# input_region.py — Rule-based input region extraction and normalization (T2 & T3) + +import re +from lib_py.agents.registry import get_adapter + +def extract_input_region(pane_text: str, agent_name: str) -> str: + """ + Extracts the input area text between horizontal rule separators (─{10,}) for a given agent. + If rules are absent, falls back to full pane_text to support mocks and unruled panes. + Strips leading prompt tokens ('>', '❯') and placeholders ('Ask anything...'). + Raises ValueError for unsupported agents (like hermes with no input area facts). + """ + adapter = get_adapter(agent_name) + if not adapter or not adapter.input_prompt: + raise ValueError(f"Agent {agent_name!r} has no input area facts") + + rule_pattern = adapter.input_rule_pattern or r'─{10,}' + rule_re = re.compile(rule_pattern) + lines = pane_text.splitlines() + rule_indices = [i for i, line in enumerate(lines) if rule_re.search(line)] + + if len(rule_indices) >= 2: + # Exclude the rule lines themselves: lines between rule_indices[-2] + 1 and rule_indices[-1] + region_lines = lines[rule_indices[-2] + 1:rule_indices[-1]] + elif len(rule_indices) == 1: + region_lines = lines[rule_indices[0] + 1:] + else: + region_lines = lines + + region_text = "\n".join(region_lines) + + prompt = adapter.input_prompt + if prompt: + region_text = re.sub(r'^\s*' + re.escape(prompt) + r'\s*', '', region_text, flags=re.MULTILINE) + + placeholder = adapter.input_placeholder + if placeholder: + region_text = region_text.replace(placeholder, '') + + return region_text.strip() diff --git a/IMPROVEMENTS.md b/IMPROVEMENTS.md index 506bd07..598bfdb 100644 --- a/IMPROVEMENTS.md +++ b/IMPROVEMENTS.md @@ -1,6 +1,6 @@ # 🛠️ Multi-Agent Mux 종합 개선 및 미해결 과제 백로그 (`IMPROVEMENTS.md`) -- **최종 갱신일**: 2026-08-14 (A-4 M0~M1 BaseAgentAdapter 도입 1단계 완료 및 계약 검증 반영) +- **최종 갱신일**: 2026-08-14 (A-4 M0~M1 및 B-8 send_keys_safe agy 우회 제거 조치 완료 반영) - **통합 관리 대상**: 기존 `CODEBASE_REVIEW_REPORT.md` + `OPTIMIZATION.md` - **총 추적 미해결 과제**: **10건** (아키텍처 2건, 엣지케이스 5건, 오케스트레이션 0건, 레거시 잔재 3건) - **완료된 과제**: **13건** (A-1, A-3, A-5, B-1, B-3, B-4, B-7, C-1, C-2, O-1, O-2, O-3, O-4-OrcOnboard) @@ -218,7 +218,7 @@ | **P0-1** | **B-7** | 저장소 밖 기동 시 리뷰어가 문자열 `"No git diff available"` 로 `[VERDICT: PASS]` 를 냄. 신규(미추적) 파일은 리뷰 대상 밖. **(✅ 완료 — tests/test_b7_diff_untracked.py 20/20 PASS)** | 소 (1파일) | — | | **P0-2** | **O-2** | 마커를 조건 없이 덮어쓰고 종료 트랩이 **타 인스턴스의 마커까지 삭제** → 완료 처리된 **O-3 가드가 조용히 무력화**됨 **(✅ 완료 — tests/test_o2_race_free_lock.py 22/22 PASS)** | 소~중 (1파일) | — | | **P1-1** | **A-4 M0~M1** | `PYTHONPATH` 부트스트랩·배포/CI 등록·`own_key` 이관. B-8/B-10/C-3b 로직을 싸게 만듦 **(✅ 완료 — tests/test_a4_adapter_contract.py 3/3 PASS)** | 중 | B-7 | -| **P1-2** | **B-8** | agy 주입이 검증 없이 `return 0` → 아무것도 전달되지 않은 잡이 `started` 로 기록됨. A-4 M0(`_pane_capture` 디코딩) 이후엔 **회피책 제거**로 축소 | 소 | A-4 M0 | +| **P1-2** | **B-8** | agy 주입 시 `return 0` 우회 제거 및 제출 검증 루프 이관 **(✅ 완료 — tests/test_b8_send_keys_verification.py 1/1 PASS)** | 소 | A-4 M0 | | **P2-1** | **B-6** | 버전 관리 트리 오염 + rsync 배포 유출. `mktemp -d` 로 옮기는 1~2줄 | 소 | — | | **P2-2** | **C-3a + C-4** | 빈 스텁 4종 + 이를 고정하던 **공허한 테스트 5건** + 죽은 심볼 3종 제거 (회귀 시간 단축 효과) | 소 | — | | **P2-3** | **C-6** | 도움말 3줄 정정 | 극소 | — | diff --git a/tests/test_b8_send_keys_verification.py b/tests/test_b8_send_keys_verification.py new file mode 100644 index 0000000..924ad66 --- /dev/null +++ b/tests/test_b8_send_keys_verification.py @@ -0,0 +1,29 @@ +import os +import subprocess +import pytest + +def test_send_keys_safe_b8_verification(tmp_path): + """Verify that send_keys_safe for agy sessions goes through submission verification and does not return 0 blindly on failure.""" + # Test script that mocks herdr commands and tests send_keys_safe behavior for agy + test_script = f"""#!/usr/bin/env bash +set -euo pipefail +SKILL_DIR="{os.path.abspath('.agents/skills')}" +source "$SKILL_DIR/lib.sh" + +# Mock _pane_quiescent to succeed +_pane_quiescent() {{ return 0; }} +_pane_dialog_open() {{ return 1; }} + +# Mock _sks_herdr commands +_sks_herdr() {{ return 0; }} + +# Mock _pane_capture to simulate fixed pane content (no execution token, no pane change) +_pane_capture() {{ echo "static content"; }} + +# Running send_keys_safe for agy session when submission is NOT accepted +send_keys_safe "test-workspace-creator-agy" "test prompt" "job-123" +""" + res = subprocess.run(["bash", "-c", test_script], capture_output=True, text=True) + # Since submission failed after 3 tries (pane static, no token), it must return exit code 4 (not 0) + assert res.returncode == 4, f"Expected returncode 4 on submission failure for agy, got {res.returncode}. Output: {res.stdout} {res.stderr}" + assert "Enter not accepted after 3 tries" in res.stderr