From d34306643540427226b39314ee390dbbb901db3a Mon Sep 17 00:00:00 2001 From: Ed_ Date: Thu, 26 Feb 2026 08:35:50 -0500 Subject: [PATCH] fix(conductor): Apply review suggestions for track 'mma_utilization_refinement_20260226' --- .editorconfig | 2 +- coverage_report.txt | Bin 27150 -> 0 bytes scripts/mma_exec.py | 19 +++++++-------- tests/test_mma_exec.py | 52 +++++++++++++++++++++++------------------ 4 files changed, 39 insertions(+), 34 deletions(-) delete mode 100644 coverage_report.txt diff --git a/.editorconfig b/.editorconfig index 56f90a9..572197b 100644 --- a/.editorconfig +++ b/.editorconfig @@ -2,7 +2,7 @@ root = true [*.py] indent_style = space -indent_size = 2 +indent_size = 1 [*.s] indent_style = tab diff --git a/coverage_report.txt b/coverage_report.txt deleted file mode 100644 index e85738c65aac5a2e020946575a31f7c4a3b9cfc7..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 27150 zcmeI5TW=i45ryYDK>owN7$8W|v)sAtg@TX-3u*l(7 z*<3bfapj~rZJswzn>WpQa}(#!g1(8ev*uNE6}3*I?j&kl#rZ{07jgYE%Ac1jwD_>p zzm9*en+NeP_0FS|exJngS>bdV)Ne{{dR7ltrLUK9{W9A6I@m4>9+2eKZ<}8>zi2*= zvPtlV%W3qn9cRy)7s2VOoKff3QKqq9#ql!EUN>)}-HX!GBx+K6Tu>M{H73ng{O*nr zw65pzsV-ytv3VThuFtokhtud234-=P`5D1hqRH%s%sy$&{oCe`A*JUr3fHNR z%0nSc&a)al5m?M!S>v_i1>M%Rr0myo@cMqbTiFMCEqj-%|f`Jo_$x`@B!N5*JqeI38!qWgDyT1HZDg2&0s@|Ka%&dRex z%k${*ZPBEg*1Qf&m(~|Co3C5#`e%eTucJq7`6l`uFCVpBG@E01yommoFYMuM!OiN0 zUgqqN<8vCVPG%Oyyk7^0tFR69`m{BkJ_&Z|CBBSRRY=DmqUQI}Gwax!K%+5@X<5&_ zECT7A1qTrP97p?6bJNP(_Cm|}x$YJJ-TA$44CDMDu>%r6j`=3~kms<36xl5Hi1wt( z;WSn(>Js%hz8S;G9NJ!@Pv5RX>n{WGafyAv-)`GwwX*N@Zp2EA|NK_d*VT!xRsY>- zL>Gd7eNZdbM^-t!z)i6iqUmw6HHYtl2kpnqyxS0ODtbnPmT8I35v9E;XyH<~?$;sW z@zj8Eqx>n5UWL9r51o0{{8VHGpSssB?MwS$cXD#uc3jVRLwqxHgr7%VV|vA^iFJ+x zZ5w;tx=&KsA6^2xf7*N(T;YsQC1$93b&bE~W4mL``6+lf8a5x<;g>P5m+@1i?EQ{* zEtArY-OnWO;OXB~kx#$2+jn)vVqGI}9Cm+E;84$9EA5yFuy2k%vj6TVuwS~1Bfw#) z{VzpXYU$Tc;K zBnv_!n|>L-2pb;0-!YHQC^@b!R}k(+9MO+(@!WD}>h7Hs;anZjd`l4S4|?pry*Y^H zxo+}Y2!|}C_JU*X1m%3Mk5veV{Gs!YzVor@?n|OtPw(c(1j6ays&^M*X0O=`p^eV^ z)=1}jTjg8`hhiJcZXU%dJuce@ga?B%?7mBN-*&PZghQ5MgxXWHAI!UK_a!@z1cZl!Yl;ze<=^fTr4SCS zDMmP-@H0D`k>7T#2M05mkY_{d&3UgxraIe=J?B=;zwSBgMH}+;hvhEA{;YIbN)MwP z&0(eCe#B8ZM zP{OtoypN)nom;d!F_P_aS3uF~-Fw9z|9VH{kg}W>MCP~?=suXqdGB^)+qaY@yB%0} zij4L{ni}gta6)SAY+NE49Y$?SMq@^zl#sVNoJm2_LkhIoiGLr?>L6oE4&qO(4rYju zA-!^xTzBJ~_PfDZ9HCSK2ih~!px-T$P|0EGV>kX)D`d1AbvOz=T461l2F*3bOnWF_ z6DQ_UXps~cIa)7w%c%FGOo$jWazMYt5ehx!C{5ijW88|G4`-aGabz4EC3AM>Dgigh zrlk++&|W3ez)Oke12HN-=uoY5~w^w*{6mrCi)rPz#0EhSnV1U5?11fxX5ggtdlUPyKKqPFBjX?m%{ zXs6!jOUTH?xfB`;lGPr~e2m^?a2{lF4#|FD*(2kchUEPNxoNb`K1P;-7K5KL$xb+v zA4si5is^09Mn>ee^4}h#q~Y1MzA#txz)>=%fB6qvIVFoFl$j!rWSt|p_itrFuX)B^ z!xBPQG)|96W0Z9;qv#yo(j!8?)I}SS86Hvch0|Sk zPu|dHXg--sUE3=f@3=)C;c#XTUiIYLu`6N++YXk(itUo4;yZ{mq$#p1*_U($k1V@n zU35G^lmSn?8@`64R#LFxkBBa{ei29b8tAx2ghTuNsDpfIquRs|z9v6_&-67#sFb@7 zD=6)~HWOP=LPR00hf-`{J0P}DiBaQekPSzTk)H5U9J2)U_#)xKbHfKT>I+IGcrivu zjKK(rAu`s050QqX&bkR^j&7xRhEcf=^|wmj>I)QL#{a7o6r5QJ)E6@iXI6hn)9B>^Ccd6t@X+|;7$eBL9#)#I~m~-g^rHq-n zNY{L!kvXNw7g`)Z>$=d?1)=w9qO}4N%}HjIN&}b)evCpic%jfnm%^D+ELD;ejU=IL zWuk>4cM6o+H$Z8`DwSqoL5gBUX*CvYU#|BqEvjit@| zN{xU~<1Hi+`4`6)8Rbhq9ACmaHHLY$M4_ZtmL^uOS@yh13uuMKiI3-r8Kb@C)-#7y zq3518Mc|xEYoIjaj>wTZtEhI{;v+4_*UL6&4QGzR!H&mkuWTBv01t7lTcN9z;G5tK zh0f!*g~yu0QFZXNzK)WVy?uGE1=2WdM%oR9G#QKZ3knU!mejjEwn{WE_@d9u ztLxwqDX}IqD#pc8n!$Xj1WRJ1=mV{=P;t(^A#ze3))MM?jl#P!I~>i~IEYB3W8xZ_w%u+tWN#Lz)LC^g_twhvAU+2@N8Z=4u2b49r?`(7H}5e2w|CvWNrv zoTWmBy&D-c8UqJwk!WUHl;;hv$Ba{&l7tpA^ufrW#Q{nlof5_Y2RwkZPqaDYt`nP3 zZ$yCR3oX8Ajz^3JYQ#|H3oX9*2H7_r)t*Ll)-XzZu24dG^ys#FM*fx`wB)CZm1GU= zQoNQ+!OZCKTUknNl{z-02EIw9cmQ)?bSljeAqST_R)T}~>!LjlA~$o+S)^2wK$4aq zbe0-1$b?w_Q`n$g*J}%_DDe)6p^O1vCJa!?S6-dEspHTZ9rkZ+hL$#Sw0xjlHzk1< z2pRstb)i)koA!J{yEN?qt<>B@*$Y9-Tj1}KFH&X1w6|}9mK@}fB{OL81+8@)+8n$i zf*Y15zM0W-jL!9p8la65+7d=1q?dSh^{2QF`6wpP+>53^=F%&cXmN(didDn2NU3sc zP~Ig#%Qr(YYQ`@bJfhq)HG~JvmR#lE)ElL;d`jWQ&fd?}+|L~V>^ZY>SX7=dHb;Ys zW>TLN1>hI0VW|_gt@N6fGxjCD$W^G$T-qDhPSX=w;;dBih`By`b+q)*8T1UzlZMd0 zHNho}jymk8G;4~qki8>Ftu3TvSxXCKW}iD%yPj$Nr0c10a~Dk!v4 z>kD@6c(Cwk+9G3G}aq%GMbF?z-{f9 z?Cg*5K==W4N+YP>@!`4E{4&XU*({LK&*3d*^cI?>zL4B6_L#jfv(Z(AIuVOz;rgQz-2Mcp&2sSka5aSb#oU^n`yqm|N6`5VEFx~xv+&3X{eaE8z3CZlM zo5DlZSJOKnL1V(2hSAJF4O#OYM-v@oO)T1H_$8z9nq;5qoT6hZ#Z_K+SZ!qG!y{Py zw4^bNIkxlf%(7eIQ#=C444Oy2SD0Ta%GvFHGSWPUj;xDyowOrjaHCNy%&LW5cwOqp zmd{YF>IGT9YizZoQuA2ryXUR%*}aTOCUY2zYY@$t z22)Ci$R%fwD8}oZ;%o2Yhy!yKClX+WwSJQ)&?xwZ3TrmsWBEBe;Hop4Sd&sD>2t=A zqd>~-Q8-hU*$R7a{1o2Xc`_(PA&vs^uDM5&hj*mn{5g@PB2I8H`ua^-S^x9fvQ5YV z?eXZ_dc}UqqbGs{G0`PQTclz}?KBK0MOa;icUx6|yf&}@*AhwpOCGRWBF^(WZ<}8e zU3SGrvuiZtCE~v4{oCR%2Go7b)2sWrP3EB5G1PcD7{mIRr}=Ee@)*|79nHs*%VStS zV>G|!)E1Iinf`dmvy!{LWi7*6%D&h%$7Lka^|k1l-x!*EH+8+VS+W$4+0Y;E6HR+O z2lD9p&UxL#7<$!Jd2<=F)%O~k%aF~laEtYWNQ8F;SJlG)`9KS@1{pV>)meS=SH7p{ zJRJ8TeIMD+>g1h9?f@u`Asa-_hYYRrLwY)cy_W7>5~q>tLAv1FpX{e~!P8o6jq$rTo^T*hliTnz)cVK^o@Cxw3d; z-b^Nk%*?Vo=bc!-m&m-J``6*)Ud9z-6uzR!+ZW1SPea4FA9o$+9|iR&{L KIAeN7O8)_Q`!SmU diff --git a/scripts/mma_exec.py b/scripts/mma_exec.py index 62a2101..9c8cc1a 100644 --- a/scripts/mma_exec.py +++ b/scripts/mma_exec.py @@ -81,7 +81,7 @@ def get_role_documents(role: str) -> list[str]: return ['conductor/workflow.md'] return [] -def log_delegation(role, prompt, result=None): +def log_delegation(role, full_prompt, result=None, summary_prompt=None): os.makedirs('logs/agents', exist_ok=True) timestamp = datetime.datetime.now().strftime('%Y%m%d_%H%M%S') log_file = f'logs/agents/mma_{role}_task_{timestamp}.log' @@ -91,7 +91,7 @@ def log_delegation(role, prompt, result=None): f.write(f"ROLE: {role}\n") f.write(f"TIMESTAMP: {datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}\n") f.write("--------------------------------------------------\n") - f.write(f"PROMPT:\n{prompt}\n") + f.write(f"FULL PROMPT:\n{full_prompt}\n") f.write("--------------------------------------------------\n") if result: f.write(f"RESULT:\n{result}\n") @@ -99,15 +99,13 @@ def log_delegation(role, prompt, result=None): # Also keep the master log os.makedirs(os.path.dirname(LOG_FILE), exist_ok=True) + display_prompt = summary_prompt if summary_prompt else full_prompt with open(LOG_FILE, 'a', encoding='utf-8') as f: - f.write(f"[{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}] {role}: {prompt[:100]}... (Log: {log_file})\n") + f.write(f"[{datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')}] {role}: {display_prompt[:100]}... (Log: {log_file})\n") return log_file -def execute_agent(role: str, prompt: str, docs: list[str]) -> str: - model = get_model_for_role(role) - - # Advanced Context: Dependency skeletons for Tier 3 +def get_dependencies(filepath: str) -> list[str]: """Identify top-level module imports from a Python file.""" try: with open(filepath, 'r', encoding='utf-8') as f: @@ -200,12 +198,13 @@ def execute_agent(role: str, prompt: str, docs: list[str]) -> str: command_text += f"\n\nTASK: {prompt}\n\n" # Use subprocess with input to pipe the prompt via stdin, avoiding WinError 206. - # We use -p 'task' to ensure non-interactive (headless) mode and valid parsing. + # We use -p '-' to read from stdin if supported, otherwise just a placeholder that stdin appends to. ps_command = ( f"if (Test-Path 'C:\\projects\\misc\\setup_gemini.ps1') {{ . 'C:\\projects\\misc\\setup_gemini.ps1' }}; " - f"gemini -p 'task' --output-format json --model {model}" + f"gemini -p '---' --output-format json --model {model}" ) cmd = ['powershell.exe', '-NoProfile', '-Command', ps_command] + try: process = subprocess.run(cmd, input=command_text, capture_output=True, text=True, encoding='utf-8') @@ -215,7 +214,7 @@ def execute_agent(role: str, prompt: str, docs: list[str]) -> str: result = f"Error: {process.stderr}" # Log the attempt and result - log_file = log_delegation(role, command_text, result) + log_file = log_delegation(role, command_text, result, summary_prompt=prompt) print(f"Sub-agent log created: {log_file}") stdout = process.stdout diff --git a/tests/test_mma_exec.py b/tests/test_mma_exec.py index 03522a0..39ed108 100644 --- a/tests/test_mma_exec.py +++ b/tests/test_mma_exec.py @@ -20,11 +20,13 @@ def test_parser_invalid_role(): with pytest.raises(SystemExit): parser.parse_args(['--role', 'tier5', 'Some prompt']) -def test_parser_prompt_required(): - """Test that the prompt argument is mandatory.""" +def test_parser_prompt_optional(): + """Test that the prompt argument is optional if role is provided (or handled in main).""" parser = create_parser() - with pytest.raises(SystemExit): - parser.parse_args(['--role', 'tier3']) + # Prompt is now optional (nargs='?') + args = parser.parse_args(['--role', 'tier3']) + assert args.role == 'tier3' + assert args.prompt is None def test_parser_help(): """Test that the help flag works without raising errors (exits with 0).""" @@ -49,14 +51,13 @@ def test_get_model_for_role(): def test_execute_agent(): """ - Test that execute_agent calls subprocess.run with the correct gemini CLI arguments + Test that execute_agent calls subprocess.run with powershell and the correct gemini CLI arguments including the model specified for the role. """ role = "tier3-worker" prompt = "Write a unit test." docs = ["file1.py", "docs/spec.md"] - expected_gemini_arg = "Use the mma-tier3-worker skill. Write a unit test. @file1.py @docs/spec.md" expected_model = "gemini-2.5-flash-lite" mock_stdout = "Mocked AI Response" @@ -72,14 +73,16 @@ def test_execute_agent(): args, kwargs = mock_run.call_args cmd_list = args[0] - assert cmd_list[0] == "gemini" - assert cmd_list[1] == "-p" - assert cmd_list[2] == expected_gemini_arg - - # Verify the correct model is passed via --model flag - assert "--model" in cmd_list - model_idx = cmd_list.index("--model") - assert cmd_list[model_idx + 1] == expected_model + assert cmd_list[0] == "powershell.exe" + assert "-Command" in cmd_list + ps_cmd = cmd_list[cmd_list.index("-Command") + 1] + assert "gemini" in ps_cmd + assert f"--model {expected_model}" in ps_cmd + + # Verify input contains the prompt and system directive + input_text = kwargs.get("input") + assert "STRICT SYSTEM DIRECTIVE" in input_text + assert "TASK: Write a unit test." in input_text assert kwargs.get("capture_output") is True assert kwargs.get("text") is True @@ -95,13 +98,15 @@ def test_get_dependencies(tmp_path): ) filepath = tmp_path / "mock_script.py" filepath.write_text(content) - dependencies = get_dependencies(filepath) + dependencies = get_dependencies(str(filepath)) assert dependencies == ['os', 'sys', 'file_cache', 'mcp_client'] import re def test_execute_agent_logging(tmp_path): log_file = tmp_path / "mma_delegation.log" + # mma_exec now uses logs/agents/ for individual logs and logs/mma_delegation.log for master + # We will patch LOG_FILE to point to our temp location with patch("scripts.mma_exec.LOG_FILE", str(log_file)), \ patch("subprocess.run") as mock_run: mock_process = MagicMock() @@ -114,10 +119,9 @@ def test_execute_agent_logging(tmp_path): assert log_file.exists() log_content = log_file.read_text() assert test_role in log_content - assert test_prompt in log_content + assert test_prompt in log_content # Master log should now have the summary prompt assert re.search(r"\d{4}-\d{2}-\d{2}", log_content) - - + def test_execute_agent_tier3_injection(tmp_path): main_content = "import dependency\n\ndef run():\n dependency.do_work()\n" main_file = tmp_path / "main.py" @@ -125,6 +129,8 @@ def test_execute_agent_tier3_injection(tmp_path): dep_content = "def do_work():\n pass\n\ndef other_func():\n print('hello')\n" dep_file = tmp_path / "dependency.py" dep_file.write_text(dep_content) + + # We need to ensure generate_skeleton is mockable or working old_cwd = os.getcwd() os.chdir(tmp_path) try: @@ -135,10 +141,10 @@ def test_execute_agent_tier3_injection(tmp_path): mock_run.return_value = mock_process execute_agent('tier3-worker', 'Modify main.py', ['main.py']) assert mock_run.called - cmd_list = mock_run.call_args[0][0] - full_command = " ".join(str(arg) for arg in cmd_list) - assert "DEPENDENCY SKELETON: dependency.py" in full_command - assert "def do_work():" in full_command - assert "Modify main.py" in full_command + input_text = mock_run.call_args[1].get("input") + assert "DEPENDENCY SKELETON: dependency.py" in input_text + assert "def do_work():" in input_text + assert "Modify main.py" in input_text finally: os.chdir(old_cwd) +