fix(seedgo,ai_mail): close CLI help-checker loophole + render ai_mail --help in Rich
seedgo's cli/help_text/introspection standards are static source scans — they confirm a print_help function, console.print, and --help wiring exist, but never execute --help. So a module could score 100% while rendering raw argparse. ai_mail did exactly that via console.print(parser.format_help()), laundering argparse plain text through the approved console API and dodging the existing parser.print_help() ban. - seedgo: cli_check now flags .format_help(); cli.md/cli_content.py name it alongside print_help(); +2 regression tests (1095 pass, self-audit 100%) - ai_mail: rewrote print_help() to hand-rolled Rich (737 tests pass); --help now renders Rich with no raw argparse, Cli back to 100% legitimately - behavioral --help check (run it, assert not raw argparse) noted as a follow-up DPLAN-0217. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QEQZXCtgnF3NQtcttTErpq
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
4af5b8bf63
commit
4d4106505f
@@ -13,6 +13,18 @@ PyPI version — not the changelog header.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **seedgo CLI help checkers green-lit non-compliant `--help` output** — the
|
||||
`cli`/`help_text`/`introspection` standards are static source scans (they
|
||||
confirm a `print_help` function, `console.print`, and `--help` wiring exist)
|
||||
but never execute `--help`, so a module could score 100% while rendering raw
|
||||
argparse. `@ai_mail` did exactly that via `console.print(parser.format_help())`,
|
||||
laundering argparse's plain text through the approved console API and dodging
|
||||
the existing `parser.print_help()` ban. Closed the loophole: `cli_check` now
|
||||
flags `.format_help()`, `cli.md`/`cli_content.py` name it alongside
|
||||
`print_help()`, +2 regression tests. Also rewrote `@ai_mail`'s `print_help()`
|
||||
to render hand-rolled Rich (the `--help` content was complete, just unstyled).
|
||||
A behavioral `--help` check (run it, assert not raw argparse) is noted as a
|
||||
follow-up. (DPLAN-0217)
|
||||
- **seedgo `readme_check` ignored the `(disabled)` marker in self-scans** — its
|
||||
module-list and test-count scans now skip `foo(disabled).py`, matching the
|
||||
central audit collector. An in-place disabled module no longer trips a false
|
||||
|
||||
@@ -16,7 +16,6 @@ Main handles routing, modules implement functionality.
|
||||
# Standard library imports
|
||||
import sys
|
||||
import importlib
|
||||
import argparse
|
||||
import signal
|
||||
from pathlib import Path
|
||||
from typing import Any, List
|
||||
@@ -51,52 +50,47 @@ MODULES_DIR = MODULE_ROOT / "modules"
|
||||
|
||||
|
||||
def print_help():
|
||||
"""Print drone-compliant help output"""
|
||||
parser = argparse.ArgumentParser(
|
||||
description="AI_MAIL Branch Operations - Email system for branch communication",
|
||||
formatter_class=argparse.RawDescriptionHelpFormatter,
|
||||
epilog="""
|
||||
COMMANDS:
|
||||
dispatch - Send dispatch email + wake target (one step)
|
||||
email - Send email to a branch
|
||||
send - Send email (alias for email)
|
||||
inbox - List emails (new + opened)
|
||||
view - View email content (marks as opened)
|
||||
reply - Reply to email (closes + archives)
|
||||
close - Close email(s) without reply (archives)
|
||||
sent - View sent messages
|
||||
contacts - Manage contacts
|
||||
EMAIL LIFECYCLE (v2):
|
||||
new → opened → closed
|
||||
- new: Just arrived, never viewed
|
||||
- opened: You've viewed it, not yet resolved
|
||||
- closed: Resolved (replied or dismissed), auto-archived
|
||||
"""Print drone-compliant help output with Rich markup"""
|
||||
console.print()
|
||||
console.print("[bold cyan]AI_MAIL — Email system for branch communication[/bold cyan]")
|
||||
console.print()
|
||||
|
||||
USAGE:
|
||||
drone @ai_mail <command> [args]
|
||||
drone @ai_mail --help
|
||||
console.print("[yellow]COMMANDS:[/yellow]")
|
||||
console.print(" [cyan]dispatch[/cyan] [dim]Send dispatch email + wake target (one step)[/dim]")
|
||||
console.print(" [cyan]email[/cyan] [dim]Send email to a branch[/dim]")
|
||||
console.print(" [cyan]send[/cyan] [dim]Send email (alias for email)[/dim]")
|
||||
console.print(" [cyan]inbox[/cyan] [dim]List emails (new + opened)[/dim]")
|
||||
console.print(" [cyan]view[/cyan] [dim]View email content (marks as opened)[/dim]")
|
||||
console.print(" [cyan]reply[/cyan] [dim]Reply to email (closes + archives)[/dim]")
|
||||
console.print(" [cyan]close[/cyan] [dim]Close email(s) without reply (archives)[/dim]")
|
||||
console.print(" [cyan]sent[/cyan] [dim]View sent messages[/dim]")
|
||||
console.print(" [cyan]contacts[/cyan] [dim]Manage contacts[/dim]")
|
||||
console.print()
|
||||
|
||||
EXAMPLES:
|
||||
# Dispatch (send + wake in one command)
|
||||
drone @ai_mail dispatch @branch "Subject" "Body"
|
||||
drone @ai_mail dispatch @branch "Subject" "Body" --fresh
|
||||
console.print("[yellow]EMAIL LIFECYCLE (v2):[/yellow]")
|
||||
console.print(" new → opened → closed")
|
||||
console.print(" [dim]new: Just arrived, never viewed[/dim]")
|
||||
console.print(" [dim]opened: You've viewed it, not yet resolved[/dim]")
|
||||
console.print(" [dim]closed: Resolved (replied or dismissed), auto-archived[/dim]")
|
||||
console.print()
|
||||
|
||||
# Send mail (no wake)
|
||||
drone @ai_mail email @seedgo "Subject" "Msg" # Send to branch
|
||||
drone @ai_mail email @all "Subject" "Msg" # Broadcast to all
|
||||
console.print("[yellow]USAGE:[/yellow]")
|
||||
console.print(" [cyan]drone @ai_mail[/cyan] <command> [args]")
|
||||
console.print(" [cyan]drone @ai_mail --help[/cyan]")
|
||||
console.print()
|
||||
|
||||
# Check mail
|
||||
drone @ai_mail inbox # List all emails
|
||||
drone @ai_mail view abc123 # View email (marks as opened)
|
||||
|
||||
# Resolve emails
|
||||
drone @ai_mail reply abc123 "Thanks!" # Reply + close + archive
|
||||
drone @ai_mail close abc123 # Close single email
|
||||
drone @ai_mail close abc123 def456 ghi789 # Close multiple emails
|
||||
drone @ai_mail close all # Close ALL emails
|
||||
""",
|
||||
)
|
||||
console.print(parser.format_help())
|
||||
console.print("[yellow]EXAMPLES:[/yellow]")
|
||||
console.print(' [cyan]drone @ai_mail dispatch @branch "Subject" "Body"[/cyan]')
|
||||
console.print(' [cyan]drone @ai_mail dispatch @branch "Subject" "Body" --fresh[/cyan]')
|
||||
console.print(' [cyan]drone @ai_mail email @seedgo "Subject" "Msg"[/cyan] [dim]Send to branch[/dim]')
|
||||
console.print(' [cyan]drone @ai_mail email @all "Subject" "Msg"[/cyan] [dim]Broadcast to all[/dim]')
|
||||
console.print(" [cyan]drone @ai_mail inbox[/cyan] [dim]List all emails[/dim]")
|
||||
console.print(" [cyan]drone @ai_mail view abc123[/cyan] [dim]View email[/dim]")
|
||||
console.print(' [cyan]drone @ai_mail reply abc123 "Thanks!"[/cyan] [dim]Reply + close + archive[/dim]')
|
||||
console.print(" [cyan]drone @ai_mail close abc123[/cyan] [dim]Close single email[/dim]")
|
||||
console.print(" [cyan]drone @ai_mail close abc123 def456 ghi789[/cyan] [dim]Close multiple[/dim]")
|
||||
console.print(" [cyan]drone @ai_mail close all[/cyan] [dim]Close ALL emails[/dim]")
|
||||
console.print()
|
||||
|
||||
|
||||
# =============================================================================
|
||||
|
||||
@@ -135,7 +135,9 @@ console.print("[cyan]This is the ONLY approved way to output text[/cyan]")
|
||||
|
||||
### Help Output: Manual Rich Formatting
|
||||
|
||||
**DO NOT use `parser.print_help()`** - it outputs plain text.
|
||||
**DO NOT use `parser.print_help()` or `parser.format_help()`** - both produce plain argparse text.
|
||||
|
||||
Wrapping `format_help()` in `console.print()` still renders plain text — it launders argparse output through the approved API without adding Rich formatting.
|
||||
|
||||
**Argparse is for PARSING arguments only, NOT for help output.**
|
||||
|
||||
@@ -161,6 +163,9 @@ def print_help():
|
||||
# DON'T DO THIS - outputs plain text
|
||||
parser = argparse.ArgumentParser(...)
|
||||
parser.print_help()
|
||||
|
||||
# DON'T DO THIS EITHER - format_help() returns plain text
|
||||
console.print(parser.format_help())
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
@@ -333,6 +333,7 @@ def check_print_usage(
|
||||
# Find print() statements and raw stdout/stderr writes
|
||||
print_lines = []
|
||||
parser_print_help_lines = []
|
||||
format_help_lines = []
|
||||
raw_write_lines = []
|
||||
|
||||
# Track if we're inside an if __name__ == '__main__': block
|
||||
@@ -376,6 +377,15 @@ def check_print_usage(
|
||||
else:
|
||||
parser_print_help_lines.append(i)
|
||||
|
||||
# Check for .format_help() - returns plain argparse text
|
||||
if ".format_help()" in stripped:
|
||||
if "#" in line:
|
||||
code_part = line.split("#")[0]
|
||||
if ".format_help()" in code_part:
|
||||
format_help_lines.append(i)
|
||||
else:
|
||||
format_help_lines.append(i)
|
||||
|
||||
# Check for raw sys.stdout.write() / sys.stderr.write() (bypasses Rich)
|
||||
if "sys.stdout.write(" in stripped or "sys.stderr.write(" in stripped:
|
||||
if "#" in line:
|
||||
@@ -412,6 +422,16 @@ def check_print_usage(
|
||||
),
|
||||
}
|
||||
|
||||
if format_help_lines:
|
||||
return {
|
||||
"name": "print() usage",
|
||||
"passed": False,
|
||||
"message": (
|
||||
f"Found .format_help() in {filename} on lines "
|
||||
f"{format_help_lines[:3]} (returns plain argparse text - write Rich help instead)"
|
||||
),
|
||||
}
|
||||
|
||||
if raw_write_lines:
|
||||
return {
|
||||
"name": "print() usage",
|
||||
|
||||
@@ -35,10 +35,11 @@ def get_cli_standards() -> str:
|
||||
" • Only in test/temp code",
|
||||
" • Remove before commit",
|
||||
"",
|
||||
"[red]✗ Never use:[/red] parser.print_help()",
|
||||
" • Outputs plain text (violates standard)",
|
||||
"[red]✗ Never use:[/red] parser.print_help() or parser.format_help()",
|
||||
" • Both produce plain argparse text (violates standard)",
|
||||
" • console.print(parser.format_help()) still renders plain text",
|
||||
" • Argparse is for PARSING only, not help output",
|
||||
" • Write custom print_help() with console.print()",
|
||||
" • Write custom print_help() with console.print() and Rich markup",
|
||||
"",
|
||||
"─" * 70,
|
||||
"",
|
||||
@@ -84,7 +85,8 @@ def get_cli_standards() -> str:
|
||||
"[bold cyan]RICH FORMATTING QUICK REFERENCE:[/bold cyan]",
|
||||
"",
|
||||
"[bold]Colors:[/bold]",
|
||||
" [red]red[/red], [green]green[/green], [yellow]yellow[/yellow], [blue]blue[/blue], [cyan]cyan[/cyan], [magenta]magenta[/magenta]",
|
||||
" [red]red[/red], [green]green[/green], [yellow]yellow[/yellow],"
|
||||
" [blue]blue[/blue], [cyan]cyan[/cyan], [magenta]magenta[/magenta]",
|
||||
"",
|
||||
"[bold]Styles:[/bold]",
|
||||
" [bold]bold[/bold], [italic]italic[/italic], [dim]dim[/dim], [underline]underline[/underline]",
|
||||
|
||||
@@ -50,17 +50,27 @@ def _mock_infrastructure(monkeypatch):
|
||||
json_mod,
|
||||
)
|
||||
|
||||
# -- bypass handler (used by architecture_check) ------------------------
|
||||
# -- bypass handler (used by architecture_check + cli_check) -------------
|
||||
bypass_pkg = MagicMock()
|
||||
bypass_ignore = MagicMock()
|
||||
bypass_ignore.get_template_ignore_patterns = MagicMock(return_value=[])
|
||||
bypass_pkg.ignore_handler = bypass_ignore
|
||||
from aipass.seedgo.apps.handlers.bypass.utils import is_bypassed as _real_is_bypassed
|
||||
|
||||
bypass_utils = MagicMock()
|
||||
bypass_utils.is_bypassed = _real_is_bypassed
|
||||
bypass_pkg.utils = bypass_utils
|
||||
monkeypatch.setitem(sys.modules, "aipass.seedgo.apps.handlers.bypass", bypass_pkg)
|
||||
monkeypatch.setitem(
|
||||
sys.modules,
|
||||
"aipass.seedgo.apps.handlers.bypass.ignore_handler",
|
||||
bypass_ignore,
|
||||
)
|
||||
monkeypatch.setitem(
|
||||
sys.modules,
|
||||
"aipass.seedgo.apps.handlers.bypass.utils",
|
||||
bypass_utils,
|
||||
)
|
||||
|
||||
# Force re-imports so checkers pick up fresh mocks
|
||||
for mod_name in [
|
||||
@@ -545,6 +555,30 @@ class TestCheckPrintUsage:
|
||||
assert result["passed"] is False
|
||||
assert "parser.print_help()" in result["message"]
|
||||
|
||||
def test_format_help_fails(self):
|
||||
"""console.print(parser.format_help()) fails — plain argparse text."""
|
||||
from aipass.seedgo.apps.handlers.aipass_standards.cli_check import (
|
||||
check_print_usage,
|
||||
)
|
||||
|
||||
content = "console.print(parser.format_help())\n"
|
||||
lines = _lines(content)
|
||||
result = check_print_usage(content, lines, "/module.py")
|
||||
assert result is not None
|
||||
assert result["passed"] is False
|
||||
assert ".format_help()" in result["message"]
|
||||
|
||||
def test_format_help_in_comment_ignored(self):
|
||||
""".format_help() inside a comment is not flagged."""
|
||||
from aipass.seedgo.apps.handlers.aipass_standards.cli_check import (
|
||||
check_print_usage,
|
||||
)
|
||||
|
||||
content = "# parser.format_help() should not be used\n"
|
||||
lines = _lines(content)
|
||||
result = check_print_usage(content, lines, "/module.py")
|
||||
assert result is None
|
||||
|
||||
def test_sys_stdout_write_fails(self):
|
||||
"""sys.stdout.write() usage fails."""
|
||||
from aipass.seedgo.apps.handlers.aipass_standards.cli_check import (
|
||||
|
||||
Reference in New Issue
Block a user