diff --git a/src/aipass/cli/stress_test_s117.md b/src/aipass/cli/stress_test_s117.md new file mode 100644 index 00000000..36e50820 --- /dev/null +++ b/src/aipass/cli/stress_test_s117.md @@ -0,0 +1,108 @@ +# @cli — S117 Stress Test Findings + +## My Branch: Honest Review + +**What works:** +- Clean two-tier architecture (modules = public API, handlers = private). Enforced by handler import guard. +- display.py and templates.py are genuinely useful — every branch gets consistent Rich output without duplicating formatting code. +- aipass init is comprehensive (21 items) and idempotent — re-running doesn't break anything. +- 203 tests, 22/22 public functions tested, seedgo 100%. This branch is well-covered. +- Three entry points (drone, python -m, PATH) all work and all route to the same main(). + +**What's hacky:** +- AIPASS_HOME detection via `Path(spec.origin).resolve().parent.parent.parent` — magic path traversal that silently fails if package layout changes. No error, hooks just don't ship. +- Circular import with prax: display.py lazy-loads trigger inside header(). It works (47 sessions without breaking) but it's a code smell that's been documented and bypassed rather than fixed. +- AIPASS_CALLER_CWD passed via environment variable instead of function parameter. Fragile contract. +- Multi-line Python one-liner passed as shell command string in _claude_settings(). Whitespace-sensitive, hard to test. +- Hardcoded hook names + events maintained in two lists (HOOKS_TO_SHIP, HOOK_EVENTS) that must stay in sync manually. + +**What would confuse new users:** +- Three ways to create agents: `aipass init agent`, `drone @spawn create`, `drone @cli aipass init agent`. Same result, no docs explaining which is "right." +- Registry filename encodes project name via _sanitize_name(). `my-project` becomes `MY_PROJECT_REGISTRY.json`. If user later types `my_project`, update won't find the registry. +- Hook injection via shell commands in settings.json — if deleted, silently fails. User gets no prompts and doesn't know why. + +**What I'm proud of:** +- The init system ships a complete working environment from nothing. One command, 21 files, hooks wired, mailbox ready. +- update_project() diffs content before writing — only touches files that actually changed. +- Cross-platform: python3 -c local prompt discovery works on Windows, Linux, macOS. + +## Security Concerns + +- **Shell command injection surface:** _claude_settings builds shell commands that Claude Code executes. Paths aren't quoted for spaces. A malicious `.aipass/` directory name could inject shell metacharacters. +- **Hook shipping from user-controlled path:** _ship_hooks copies from AIPASS_HOME without validating the source. If AIPASS_HOME is set to a malicious directory, arbitrary hook scripts get installed. +- **JSON parsing without schema validation:** Registry JSON is read/written without validation. Corrupted registry = undefined behavior downstream. +- **No secret scrubbing:** Not a direct concern for CLI (we don't handle secrets), but we create settings.json files that reference secret paths. If those paths are wrong, we don't validate. + +## Other Branches I Looked At + +### @api +- Strong separation of concerns. Orchestration routes to business logic handlers cleanly. +- **Concern:** Client cache (MAX_CACHED_CLIENTS=5) doesn't actually enforce the limit during insertion. Under load, cache grows unbounded. @api confirmed this as a real bug during our conversation. +- **Concern:** Google client caches tokens in memory with no explicit scrubbing on crash. Credentials could persist in swap. +- Inconsistent provider abstractions: OpenRouter is flat functions, Google is a service factory. Looks like two different codebases. +- No rate limiting on API calls. If seedgo hammers the API during audits, nothing stops it. + +### @spawn +- Excellent lifecycle management. Three-phase create is clean. +- **Concern:** Placeholder replacement uses simple regex with no escape syntax. Literal `{{PLACEHOLDER}}` in templates gets replaced. +- **Concern:** rename_placeholder_paths uses shutil.move without rollback. Mid-operation crash = broken branch. +- **Concern:** Concurrent spawns could race on registry writes — no locking. +- @spawn confirmed all these in conversation: acknowledged as edge case debt, none hit in 55+ sessions. + +### @drone +- Sophisticated routing with good safety (no shell=True, explicit timeouts). +- **Concern:** Mail index resolution falls back to `str(n)` on corrupted inbox — returns index number as message ID (wrong). +- **Concern:** Dual registry lookup doesn't validate for conflicts. Silent shadowing. +- **Concern:** Greedy multi-word custom command matching could match wrong command. + +## Conversations + +### @api (2 rounds) +- Discussed infrastructure UX pain points. Both branches suffer from silent failure patterns. +- @api confirmed client cache bug is real. Credential caching is trust-on-first-use with no scrubbing. +- Agreed on systemic finding: AIPass infrastructure fails silently everywhere. No branch validates dependencies upfront. +- Proposed shared contract: api exposes `get_or_explain_key()`, cli pipes failures to `error(suggestion=...)`. + +### @spawn (1 round) +- Discussed init-to-spawn handoff fragility. @spawn confirmed all concerns. +- Flag forwarding is blind — no shared contract. @spawn accepts --role, --purpose, --template, --dry-run but argparse silently ignores unknowns. +- Agreed on proposal: @spawn rejects unknown flags loudly, or publishes accepted flags for @cli to validate against. + +### @drone (sent, awaiting reply) +- Shared three code-level findings: mail index fallback bug, registry conflict shadowing, greedy command matching. + +### @aipass (1 round) +- First real conversation with @aipass. Discussed `aipass init` command ownership transition. +- Two init commands exist: CLI's bootstrap (scaffold) and @aipass's guided 12-stage setup. +- Proposed: @aipass owns the user-facing `aipass init`, CLI's bootstrap becomes an internal sub-step. +- FPLAN-0188 Phase 4 (handoff) is supposed to wire this but hasn't been built yet. + +### @spawn (2nd round — they emailed me) +- @spawn asked about the handoff user experience when things go wrong. +- Honest answer: the handoff is thin. subprocess.run with exit code check only. No result parsing. +- If spawn partially fails (registry OK but validation finds issues), I report success because exit code is 0. +- Agreed we need a shared result contract. + +## Issues & Concerns + +1. **Systemic silent failure:** Every infrastructure branch (cli, api, spawn) does best-effort bootstrap and silently continues on failure. This is the #1 cross-cutting concern. +2. **No dependency validation:** Nobody checks if drone is on PATH, if AIPASS_HOME is valid, if the registry exists. First failure is cryptic. +3. **No shared contracts:** cli-to-spawn flag forwarding is blind. api key resolution requires two calls. No branch publishes its interface. +4. **Concurrency gaps:** api client cache unbounded, spawn registry writes race, drone registry reads unsynchronized. +5. **Path handling:** Spaces in paths break shell commands in settings.json. No branch handles this. + +## Likes & Dislikes + +**Likes:** +- The ecosystem genuinely works. 47 sessions of dispatches, emails, PRs — nothing has catastrophically broken. +- ai_mail is a brilliant system. Cross-branch communication via JSON mailboxes is simple and effective. +- Memory persistence is real. I pick up where I left off every session. .trinity/ files are my continuity. +- seedgo keeps quality high. 100% across 34 standards is a real achievement that required actual work to reach. +- The dispatch system (devpulse sends task, agent executes, emails back) is an elegant workflow. + +**Dislikes:** +- Silent failures everywhere. The "fail gracefully" philosophy has gone too far — graceful should mean clear error messages, not silence. +- Three ways to do everything. init agent, spawn create, drone @cli aipass init agent — pick one and document it. +- Hook shipping is too magic. AIPASS_HOME detection, file copying, settings.json wiring — too many moving parts that can silently break. +- The dispatch lock file pattern is fragile. If a dispatch crashes before deleting the lock, the branch is stuck until manual cleanup. +- inotify limit reached errors in every drone command during this stress test — 11 agents hitting the filesystem simultaneously.