diff --git a/.squad/agents/fenster/history.md b/.squad/agents/fenster/history.md index 1276508e5..a885e4888 100644 --- a/.squad/agents/fenster/history.md +++ b/.squad/agents/fenster/history.md @@ -1,3 +1,4 @@ +πŸ“Œ Team update (2026-03-05T22:46:00Z): Azure Function samples require \main\ field and build step β€” decided by Fenster # Project Context - **Owner:** Brady @@ -695,6 +696,8 @@ px tsx), it logs a test-mode warning but still validates config β€” useful for d - For TypeScript Azure Functions: always add a `build` step (`tsc`) and ensure `main` points to `dist/` output (e.g., `"main": "dist/functions/squad-prompt.js"`). The `start` script should chain build + func start: `npm run build && func start`. - A `prestart:func` npm script ensures `func start` always gets fresh compiled JS, preventing stale-build confusion. +πŸ“Œ Team update (2026-03-06): Fixed Azure Function sample β€” added missing `main` field to package.json, updated `start` script to build-then-run, added build documentation to README. Root cause was runtime couldn't discover function registration without `main` pointing to compiled output. β€” decided by Fenster + πŸ“Œ Team update (2026-03-06): Fixed Azure Function sample β€” added missing `main` field to package.json, updated `start` script to build-then-run, added build documentation to README. Root cause was runtime couldn't discover function registration without `main` pointing to compiled output. β€” decided by Fenster ## Issue #228 β€” Squad Guard vs Scribe Runtime State diff --git a/.squad/agents/hockney/history.md b/.squad/agents/hockney/history.md index 60416c67b..3ccba1ab0 100644 --- a/.squad/agents/hockney/history.md +++ b/.squad/agents/hockney/history.md @@ -5,6 +5,25 @@ - **Stack:** TypeScript (strict mode, ESM-only), Node.js β‰₯20, @github/copilot-sdk, Vitest, esbuild - **Created:** 2026-02-21 +## Core Context β€” Hockney's Focus Areas + +**Testing Specialist:** Hockney owns CLI/test gap analysis, REPL UX test suites, coverage expansion, hostile QA scenarios, test roadmap, error boundary coverage. Standard: 80% floor, 100% on critical paths. 2931 tests passing. + +**Pre-Phase-1 Testing Foundation (2026-02-21 to 2026-03-04):** +- REPL UX visual test suite: 40 tests across 6 categories (ThinkingIndicator, AgentPanel, MessageStream, InputPrompt, Welcome, Never-Feels-Dead) +- Shell integration tests: 47 tests for SessionRegistry, spawn, Coordinator, ShellLifecycle, StreamBridge +- CRLF normalization tests: 13 tests across 5 parsers +- Consumer-perspective import tests: 6 tests validating barrel exports (index/parsers/types) +- Hostile QA (Issue #327): 32 adversarial scenarios covering small terminal, missing config, invalid input, corrupt config, non-TTY, UTF-8, rapid input +- Test gap filing (2026-02-28): 10 issues (#558–#567) tracked explicitly for Wave E + +**Known Issues Discovered:** +- --version output missing "squad" prefix (cli-entry.ts:48) +- Empty/whitespace CLI args trigger interactive shell in non-TTY (should show help/error) +- No sanitization for null bytes in spawn args (Node.js-level, pre-validate) + +## Learnings + ## πŸ“Œ Core Context β€” Hockney's Focus Areas **Testing Specialist:** Hockney owns CLI/test gap analysis, REPL UX test suites, coverage expansion, hostile QA scenarios, test roadmap, error boundary coverage. Standard: 80% floor, 100% on critical paths (casting, spawning, coordinator routing). 2931 tests passing. @@ -18,7 +37,7 @@ - Test gap filing (2026-02-28): 10 issues (#558–#567) now tracked explicitly for Wave E **Known Bugs Found:** -- `--version` output missing "squad" prefix (cli-entry.ts:48) +- --version output missing "squad" prefix (cli-entry.ts:48) - Empty/whitespace CLI args trigger interactive shell in non-TTY (should show help/error instead) - No sanitization for null bytes in spawn args (Node.js-level issue, should pre-validate) @@ -28,868 +47,32 @@ **Hockney's Phase 1 SDK-First Tests Complete.** Built 60 contract-first tests: 36 for builders, 24 for build command. All passing. Stubs documented with exact import locations ready for swaps when Edie/Fenster implementations land. -**Team Context:** -- **Keaton:** Scope complete. Markdown source, TS facade. 5 success criteria defined. -- **Edie:** Builders ready (8 functions, runtime validation, zero deps, BuilderRoutingRule naming). -- **Fenster:** `squad build --check` ready. Uses SquadSDKConfig. HTML header stamps. -- **Kujan:** OTel readiness βœ… all 8 modules compile. -- **Verbal:** Coordinator SDK mode detection live. - -Test stubs execute PRD contract. When implementations land, replace inline stubs with real imports. All 3571 tests passing. Build clean. - -## Learnings - -### Knock-knock Docker fix (2026-03-03) -**Status:** Fixed β€” docker-compose build + up now works. Committed to `migration` branch. -- **Root cause 1:** SDK `package.json` has `"prepare": "npm run build"` which runs `tsc` during `npm install` of the `file:` dependency. Alpine container doesn't have `tsc`. Fix: strip `prepare`/`prepublishOnly` from SDK package.json inside Dockerfile before `npm install`. -- **Root cause 2:** npm workspace hoisting puts `@opentelemetry/api` in repo root `node_modules`, not in `packages/squad-sdk/node_modules`. Old Dockerfile copied host `node_modules` which were incomplete. Fix: run `npm install` for the SDK inside the container to get a complete dependency tree, then copy pre-built `dist` on top. -- **Key insight:** Never copy workspace-hoisted `node_modules` into Docker. Always install deps fresh inside the container. -- **Verification:** `docker-compose build` succeeds, `docker-compose up` prints expected "Missing GITHUB_TOKEN" and exits 1 (correct behavior without token). - -### Knock-knock sample verification (2026-03-03) -**Status:** No fixes needed β€” sample already compiles and runs correctly. -- **Verification steps:** SDK build (`npm run build`) β†’ clean. Sample `npm install` + `npx tsc --noEmit` β†’ zero TS errors. Runtime `npx tsx index.ts` β†’ prints expected "Missing GITHUB_TOKEN" and exits 1. -- **API surface audit:** CastingEngine (no-arg ctor, `castTeam(CastingConfig) β†’ CastMember[]`), StreamingPipeline (`onDelta`, `processEvent`, `markMessageStart`, `attachToSession`), SquadClientWithPool (`connect`, `createSession`, `resumeSession`) β€” all sample usages align with SDK signatures. -- **Imports verified:** `CastingEngine` + `StreamingPipeline` + `StreamDelta` from main barrel export; `SquadClientWithPool` from `@bradygaster/squad-sdk/client` subpath export β€” both exist in SDK `exports` map. -- **Dockerfile/docker-compose:** Correct paths, correct build context (`../..`), correct CMD. - -### Multi-squad resolution tests β€” Issue #652 (2026-03-02) -**Status:** Complete β€” 48 proactive tests in `test/multi-squad.test.ts`, all passing. PR #690 (draft). -- **Categories:** getSquadRoot (3), resolveSquadPath resolution chain (9), listSquads (5), createSquad (5), deleteSquad (4), switchSquad (4), migrateIfNeeded (5), edge cases + lifecycle (13). -- **Written proactively** from PRD spec while Fenster implements `packages/squad-sdk/src/multi-squad.ts`. Tests validate expected data structures (squads.json schema, directory layout) using real filesystem fixtures via `mkdtempSync`. -- **Key decisions:** Tests currently don't import from the implementation module β€” they validate the contract (squads.json shape, directory structure, name validation regex `^[a-z0-9]([a-z0-9-]{0,38}[a-z0-9])?$`). Import path wiring needed once Fenster's code lands. -- **Edge cases covered:** Corrupted/empty/missing squads.json, path traversal in squad names, orphaned entries (JSON lists squad that has no directory), concurrent create/delete races, platform separator safety. -- **Test strategy:** Isolated temp dirs via `mkdtempSync`, per-test fixtures, cleanup in `afterEach`. Same patterns as `nap.test.ts` and `resolution.test.ts`. No mocked fs β€” all real filesystem operations against tmp dirs. - -### πŸ“Œ Team update (2026-03-01T23:07:00): Issue audit completed β€” Cheritto + Hockney parallel TUI audit (#673–#681) -- **Agents:** Cheritto (TUI code audit), Hockney (test verification) -- **Result:** 3 OPEN (#673, #675, #679), 2 PARTIAL (#674, #681) -- **Aligned verification:** Test behavior confirmed all statuses -- **Session log:** `.squad/log/20260301T23-07-00-issue-audit.md` - -### Nap feature tests β€” Issue #635 (2026-03-01) -**Status:** Complete β€” 38 new tests in `test/nap.test.ts`, all passing. -- **Categories:** Metrics collection (3), History compression (5), Log pruning (3), Inbox cleanup (3), Decision archival (2), Deep mode (2), Dry-run mode (3), Journal safety (2), Report formatting (5), Edge cases (8), Combined scenarios (2). -- **Key finding:** History compressor operates on `##` headings as section boundaries, NOT `###` entries. Test fixtures must use `## Date: Entry N` format for compression to trigger. Real history.md uses `## Learnings` as a single section container β€” compression only kicks in when there are >5 top-level `##` sections. -- **Substring trap:** `toContain('Entry 1')` false-positives on `Entry 10`. Use full heading assertions like `not.toContain('## 2026-03-01: Entry 1')` for unambiguous matching. -- **Decision archival dates:** `daysAgoFromLine()` computes real calendar age. Test fixtures must use genuinely old dates (e.g., `2024-01-xx`) for entries to exceed the 30-day threshold β€” future dates compute negative age and won't archive. -- **Non-existent dir:** Implementation returns `{ before: emptyMetrics(), after: emptyMetrics(), actions: [] }` instead of throwing. Tests should assert empty result, not `rejects.toThrow()`. -- **Test strategy:** Isolated temp dirs via `mkdtempSync`, per-test fixtures, cleanup in `afterEach`. Same patterns as `bump-build.test.ts`. No component rendering needed β€” pure filesystem + function-call assertions. - -### Multi-line paste handling tests (2026-03-01) -**Status:** Complete β€” 18 new tests in `test/multiline-paste.test.ts`, all passing. -- **Categories:** MessageStream multi-line rendering (6), Single-line submit behavior (3), Disabled-state buffering with newlines (4), Multi-line content integrity in ShellMessage (5). -- **Bug context:** Ink's `useInput` fires per-character. Newlines in pasted text trigger `key.return` which submits the first line, then `disabled=true` causes remaining newlines to be stripped β€” garbling multi-line pastes. -- **Test strategy:** ink-testing-library render tests for both MessageStream and InputPrompt components. Same patterns as `repl-ux.test.ts` β€” `React.createElement`, `stdin.write()`, `makeMessage()` helpers. Tests assert on rendered TEXT content, not internal state. -- **Key finding:** `stdin.write()` for single characters in disabled mode requires `await setTimeout(50)` for React reconciler flush β€” same compatibility gap documented in REPL UX visual test suite learnings. Two tests initially failed without the async flush. -- **Coverage:** Tests document expected behavior for multi-line input preservation, blank line handling, CRLF retention, special character rendering, disabled-state buffering with backspace, and mixed single/multi-line conversation display. - -### Banner simplification tests β€” Issues #626, #627 (2026-03-01) -**Status:** Complete β€” 5 new tests in `test/first-run-gating.test.ts`, all 35 tests passing. -- **Tests added:** Single CTA (no dual-path `squad init`), middle-dot `Β·` separators in usage line, concise "Type naturally" prefix, "Ctrl+C to exit" formatting, no redundant spacers between roster and usage line. -- **Also fixed:** Updated existing #625 test that expected both `/init` and `squad init` β€” now aligns with single-CTA behavior. -- **Test strategy:** Source-code structural assertions (read App.tsx as text, regex match on JSX patterns). Same pattern as #607 and #625 tests β€” deterministic, no Ink rendering needed. -- **Key finding:** Tests written in parallel with Cheritto's code changes. All passed because Cheritto had already committed the banner changes on `squad/626-627-banner-polish` branch. - -### First-run gating tests β€” Issue #607 (2026-03-01) -**Status:** Complete β€” 25 new tests in `test/first-run-gating.test.ts`, all passing. -- **Categories:** Banner renders once (3), First-run hint (5), Console warning suppression (4), Assembled message gating (5), Session-scoped Static keys (5), Terminal clear ordering (3). -- **Key findings:** `loadWelcomeData` consumes the `.first-run` marker file on read (unlinkSync), making `isFirstRun` a one-shot flag. Terminal clear (`\x1b[2J\x1b[H`) at `index.ts:695` precedes `render()` at line 697. The `firstRunElement` in App.tsx requires both `isFirstRun=true` AND `rosterAgents.length > 0` to show "Your squad is assembled" β€” empty roster falls back to init guidance. Session resume in `runShell` is gated by `(hasTeam && !isFirstRun)`. -- **Coverage gaps found:** No E2E test that renders the full App component for first-run flow (requires too many SDK dependencies). No test for `console.error('β—† Loading Squad shell...')` appearing exactly once. Warning suppression tests are logic-replicated, not testing `cli-entry.ts` directly. -- **Test strategy:** Logic-level conditional extraction from App.tsx JSX, filesystem marker lifecycle via loadWelcomeData, source-code structural assertions for render ordering, Ink render tests for banner uniqueness, MemoryManager integration for key stability. - -### Round 2 REPL UX fix tests (2026-03-01) -**Status:** Complete β€” 17 new tests added to `test/repl-ux-fixes.test.ts` (30β†’47 total, all passing). -- **Categories:** Screen corruption prevention (3), Banner logic (5), Compaction removal (3), Coordinator label (3), Init guidance (3). -- **Key findings:** MemoryManager import requires `await import()` (ESM-only project β€” no `require()`). Static keys now use `${sessionId}-${i}` pattern with base-36 timestamp prefix, preventing Ink key collisions across session boundaries. Coordinatorβ†’Squad label mapping verified in both completed messages and streaming content paths. -- **Test strategy:** Logic-level tests extracting conditionals from App.tsx JSX. Ink render tests via ink-testing-library for MessageStream coordinator label and streaming content display. MemoryManager tested directly for archival overflow behavior. - -### πŸ“Œ Team update (2026-02-28T15:34:36Z): 10 test gaps filed + Wave E planning ready -- **Status:** Completed β€” Hockney conducted coverage analysis, filed 10 test gap issues (#558–#567) -- **Issues filed:** Exit code consistency, timeout edge cases, per-command help, shell-specific flags, env var fallbacks, REPL transitions, config precedence, spawn flags, flag aliases, error handling -- **All labeled** `squad:hockney` for routing; Brady to triage and assign -- **Impact:** Test gaps now explicit in GitHub; ready for Wave E prioritization; 10 high–moderate priority gaps identified -- **Session log:** `.squad/log/2026-02-28T15-34-36Z-issue-filing-sprint.md` β€” decided by Keaton, McManus, Hockney, Waingro - -### REPL UX visual test suite (2026-02-23) -**Status:** Complete β€” 40 tests, all passing across 6 test categories. -- Created `test/repl-ux.test.ts` using ink-testing-library v4 + Ink v6 components. -- **Categories:** ThinkingIndicator visibility (5), AgentPanel status display (7), MessageStream formatting (8), InputPrompt behavior (8), Welcome experience (3), Never-feels-dead lifecycle (5). -- **Key finding:** ink-testing-library v4 stdin.write() does NOT synchronously trigger Ink v6's useInput hook. Requires `await setTimeout(50)` after each write for React reconciler to flush. This is a compatibility gap that should be watched for in future ink-testing-library releases. -- **Kovash conflict resolved:** Components were modified mid-task (InputPrompt now hardcodes `β—† squad>` prompt with spinner when disabled, AgentPanel now shows pulsing dot + "streaming"/"working" text instead of old "responding" label). Tests adapted to match current rendered output. -- **Strategy:** Tests assert on TEXT content in rendered frames (what the user sees), not internal state. This makes them resilient to Kovash's implementation changes as long as the visual contract holds. - -[CORRECTED: Removed duplicate "πŸ“Œ Core Context: Test Foundation & Beta Learnings" section (lines 460-475) that was copied verbatim from lines 95-110. This was an intermediate state capture rather than new learning.] - -### Issue #214: Resolution & CLI global/status tests (2026-02-21) -- Added 14 new tests to resolution.test.ts: deeply nested dirs, nearest .squad/ wins, symlink support -- Created cli-global.test.ts with 10 tests: status routing (repo/personal/none), --global flag for init/upgrade -- Test count grew from ~1592 to 1616 across 51 files β€” all passing -- Symlink test skipped on Windows (requires elevated privileges) β€” pattern: `if (process.platform === 'win32') return;` -- CLI routing testable without spawning processes by replicating the conditional logic from src/index.ts main() -- resolveGlobalSquadPath() always creates the directory β€” tests that check global .squad/ must clean up after themselves - -### Issue #248: Shell module integration tests (2026-02-21) -Created test/shell.test.ts (47 tests): SessionRegistry (9), spawn infrastructure (6), Coordinator (11), ShellLifecycle (10), StreamBridge (11). Used real test-fixtures for integration confidence. Shell modules well-structured: pure functions (parsing), simple classes (registry), callback-based (bridge). Test count: 1621β†’1668. - -### Issue #228: CRLF normalization tests (2026-02-21) -Created test/crlf-normalization.test.ts (13 tests) across 5 parsers using withCRLF() helper and expectNoCR() assertions. All passing. Validates Fenster's normalizeEol() applied correctly. - -### Issue #230: Consumer-perspective import tests (2026-02-22) -Created test/consumer-imports.test.ts (6 tests): main barrel, parsers barrel, types barrel, side-effect-free imports. Validates barrel split (index.ts/parsers.ts/types.ts) works for consumers. - -### Post-restructure assessment (2026-02-22) -**Build:** Clean (exit 0). **Tests:** 1719 passing across 56 files. **Import state:** Tests import from root ../src/ (old monolith). **Migration deferred:** Premature migration risks breaking tests. Expand exports maps or add vitest alias config when root src/ deleted. Exports map gap + CLI no exports + barrel divergence = high risk now. - -### 2026-02-23: Streaming regression test suite & root cause analysis -**Status:** Complete (13 new tests added, 42 total, 41 passing + 1 todo) -- Identified root cause: empty `sendAndWait()` result + zero `message_delta` events = empty accumulated = silent ghost response -- Added `simulateDispatchWithFallback()` test helper (existing helper only exercised delta path) -- Bug report filed: "Empty coordinator response β€” the silent swallow" -- Recommendation for future: minimum-length validation on coordinator responses post-accumulation -- Marked `it.todo()` for SQUAD_DEBUG diagnostic logging coverage (Kovash task) - ---- - -πŸ“Œ Team update (2026-02-23T09:25Z): Streaming diagnostics infrastructure complete β€” root cause identified, 13 regression tests added. Kovash added SQUAD_DEBUG logging infrastructure. Saul fixed OTel protocol to gRPC. Version bump to 0.8.5.1. β€” decided by Scribe - -### πŸ“Œ Team update (2026-02-22T041800Z): SDK/CLI split verified, all 1719 tests passing, test import migration deferred β€” decided by Hockney -- Created test/consumer-imports.test.ts with 6 tests validating package exports from a consumer's perspective -- **Main barrel** (3 tests): key parser functions (parseTeamMarkdown, parseDecisionsMarkdown, parseRoutingMarkdown), CLI functions (runInit, runExport, runImport, scrubEmails), VERSION export as string -- **Parsers barrel** (1 test): parseTeamMarkdown and parseCharterMarkdown importable from src/parsers.js -- **Types barrel** (1 test): Object.keys(types).length === 0 confirms pure type re-exports produce no runtime values -- **Side-effect-free import** (1 test): importing index.ts doesn't mutate process.argv or trigger CLI behavior β€” test completing without hanging proves clean separation -- Dynamic `await import()` used throughout to keep tests independent and avoid module caching issues -- All 6 tests pass on first run; validates the barrel file split (index.ts / parsers.ts / types.ts) works correctly for consumers - -### Post-restructure verification (2026-02-22) -- **Build:** `npm run build` compiles both `@bradygaster/squad-sdk` and `@bradygaster/squad-cli` cleanly via workspace scripts. Exit code 0. -- **Tests:** All 1719 tests pass across 56 test files. `npm run build && npm test` exits clean. -- **vitest.config.ts:** Works as-is β€” no path aliases needed while root `src/` still exists. -- **Import state:** All 56 test files still import from root `../src/` (the old monolith barrel). Only `consumer-imports.test.ts` had 3 workspace package references but dynamically imports from `../src/index.js`. -- **Import migration deferred:** Cannot blindly rewrite `../src/X.js` β†’ `@bradygaster/squad-sdk/X` because: - 1. Tests import deep internal modules (e.g., `../src/config/agent-doc.js`, `../src/casting/casting-engine.js`) that aren't exposed via the SDK package's `exports` map β€” only 18 subpath exports exist. - 2. CLI test files import from `../src/cli/...` which lives in `@bradygaster/squad-cli`, but that package has no subpath exports at all. - 3. Root `src/index.ts` (v0.7.0) still re-exports CLI functions (`runInit`, `runExport`, etc.) which SDK package (v0.8.0) correctly does not export β€” the `consumer-imports.test.ts` tests CLI exports that don't exist in the SDK barrel. - 4. Migrating requires either expanding the `exports` maps in both packages or adding vitest `resolve.alias` config. Both are non-trivial. -- **Recommendation:** Migration should happen as a dedicated task when root `src/` is actually removed. Attempting it now risks breaking 1719 passing tests for no immediate benefit. -- **Flaky test observed:** One run showed 1 failure / 1718 pass in CLI export-import tests (timing-sensitive fs operations). Not reproducible on immediate re-run β€” pre-existing flake. - -[CORRECTED: Removed two duplicate entries of "SDK/CLI split verified" team update that were captured as both a full version (lines 129-149) AND a summary version (line 151) AND then repeated completely again (lines 469-491). Kept the detailed version and removed all duplicates.] - -### Test infrastructure: coverage config + package exports test (2026-02-22) -- **Coverage:** Installed `@vitest/coverage-v8@^3.2.0`, configured vitest with `v8` provider and `text`, `text-summary`, `html` reporters. Coverage output goes to `./coverage/` (already in `.gitignore`). Include patterns cover `src/**/*.ts` and `packages/*/src/**/*.ts`. -- **Package exports test:** Created `test/package-exports.test.ts` with 8 tests covering SDK exports map: root (`VERSION`), `/config` (`DEFAULT_CONFIG`), `/resolution` (`resolveSquad`), `/parsers` (`parseTeamMarkdown`), `/types` (type-only, no runtime values), `/agents`, `/skills`, `/tools`. -- Discovered `types` subpath has zero runtime exports (pure `export type` statements) β€” test only verifies module resolves. -- Config subpath exports `DEFAULT_CONFIG`, `AgentRegistry`, `ModelRegistry`, etc. β€” not `loadSquadConfig` as initially assumed. -- `npm install` needed `--legacy-peer-deps` flag due to `workspace:*` protocol in squad-cli's package.json (pnpm syntax, not native npm). -- Build passes cleanly. All 8 package-exports tests pass with coverage reporting. - -### Test Health Assessment (2026-02-22T23:02Z) - -### Public Readiness Assessment (2026-02-24) - -**Context:** Brady asked whether SDK and CLI are ready to go public β€” source and all. - -**Test Suite Health:** -- **Total tests:** 2930 passing + 1 skipped (todo) = 2931 total -- **Test files:** 110 test files (100 in root test/, 10 in test/cli/) -- **Duration:** 54.31s total runtime (transform 9.15s, collect 155.51s, tests 464.66s) -- **Exit status:** Clean exit code 0 β€” zero test failures -- **Skipped tests:** 4 files have .skip/.todo markers (aspire-integration, e2e-migration, migration, repl-streaming) β€” all are future-facing features or edge cases, not critical path blockers - -**Coverage Analysis:** -- **Statement coverage:** 8.87% (1332/15013) β€” CRITICALLY LOW -- **Branch coverage:** 90.68% (448/494) β€” EXCELLENT -- **Function coverage:** 80.42% (189/235) β€” VERY GOOD -- The statement coverage anomaly suggests vitest coverage config may not be capturing all source files correctly or tests focus on specific hot paths. Branch coverage at 90%+ indicates tested code paths are well-exercised. - -**Test Quality β€” The Good:** -- **Comprehensive edge case testing:** hostile-integration.test.ts runs 95 nasty input strings through parseInput/executeCommand/MessageStream. None crash. Proves robustness. -- **Journey tests:** 6 full human journey tests (first conversation, error handling, waiting anxious user, power user, specific agent, next day) β€” covers real user flows end-to-end -- **Speed gates:** Performance assertions on help (< 5s, < 55 lines), init (< 3s), welcome (< 2s) β€” quantified UX requirements -- **UX gates:** Terminal width checks (80 chars), error remediation hints, version format validation β€” enforces professional CLI experience -- **Stress tests:** 500+ message rendering, rapid dispatch, concurrent operations, memory tracking β€” proves stability under load -- **E2E shell tests:** Full ink-based App component driven via stdin like real users β€” integration confidence -- **Acceptance tests:** 32 hostile environment tests (40x10 terminal, missing config, corrupt files, non-TTY pipe, UTF-8 edge cases) β€” 52+ seconds of real process spawning - -**Test Quality β€” The Gaps:** -- **No SDK adapter tests with real Copilot CLI:** All tests mock CopilotClient. No live integration tests against actual @github/copilot-sdk talking to a real Copilot CLI server. If Copilot CLI changes its protocol, we won't know until users report it. -- **No CI coverage for actual agent spawning:** Tests verify routing logic, session management, prompt building β€” but no test actually spawns a Copilot agent and validates the response parsing pipeline works end-to-end with real SDK streaming. -- **CI config is BROKEN:** .github/workflows/squad-ci.yml runs `node --test test/*.test.js` β€” but tests are TypeScript (.test.ts) and use Vitest, not Node.js built-in test runner. CI is not running the actual test suite. This is a silent failure. -- **No flaky test tracking:** Test suite is deterministic in local runs, but hostile.test.ts takes 52s+ with real process spawning β€” timing-dependent. No retry logic or flake tracking. - -**CI/CD Readiness:** -- **squad-ci.yml:** BROKEN β€” runs `node --test test/*.test.js` instead of `npm test` (vitest). CI green is FALSE POSITIVE. -- **squad-main-guard.yml:** Good β€” prevents .squad/ and team-docs/ from leaking into main/preview branches. Clear error messages with remediation. -- **squad-preview.yml:** Good β€” validates version in CHANGELOG, runs tests (but WRONG test command), checks no .squad/ tracked. -- **15 workflows total:** Comprehensive automation (triage, labels, heartbeat, publish, release, docs). But if CI doesn't run tests correctly, all downstream workflows are built on sand. - -**Critical Missing Test Areas:** -1. **Copilot SDK protocol integration** β€” No tests with real CopilotClient. Protocol breakage = invisible until production. -2. **Agent spawn with real streaming** β€” Coordinator routing tested, but not the full spawn β†’ stream β†’ parse β†’ render pipeline with real SDK. -3. **CLI entry point (cli.js)** β€” No tests of the actual Node.js entry point. Tests import modules directly. -4. **Error state recovery** β€” Journey tests cover error handling UI, but not "what if SDK throws mid-stream and we need to gracefully recover a session?" -5. **Windows-specific behavior** β€” Symlink tests skipped on Windows. CRLF tests exist but no tests for Windows-specific CLI behaviors (process spawning, path handling). - -**Verdict: 🟑 READY WITH CAVEATS** - -**Why not πŸ”΄:** -- 2930 tests passing with zero failures is a strong foundation -- Hostile/stress/journey tests prove robustness and real-world usability -- No TODOs/FIXMEs/HACKs in src/ β€” clean codebase -- Test quality is genuine (not just smoke tests) β€” speed gates, UX gates, edge cases all covered - -**Why not 🟒:** -- **CI is broken** β€” squad-ci.yml doesn't run vitest. This must be fixed BEFORE public launch. CI green means nothing right now. -- **No real Copilot SDK integration tests** β€” mocking is good for unit tests, but zero tests with actual @github/copilot-sdk talking to Copilot CLI = blind spot. If GitHub changes the protocol, we're toast. -- **8.87% statement coverage** β€” likely a config issue, but needs investigation. Branch coverage at 90%+ suggests tests are good, but missing files from coverage report is concerning. - -**Required before public launch:** -1. **Fix CI immediately:** Change `node --test test/*.test.js` β†’ `npm test` in squad-ci.yml. Verify it actually runs vitest. -2. **Investigate coverage anomaly:** 90% branch coverage but 8.87% statement coverage doesn't add up. Likely vitest include paths are wrong. -3. **Add 1 integration test with real Copilot CLI:** Spawn a local Copilot CLI server, start a session, send a message, verify streaming works. Can be skipped in CI if COPILOT_CLI_PATH not set, but proves the adapter works. - -**Recommended (not blocking):** -- Add retry logic to hostile.test.ts (52s of process spawning is flake-prone) -- Track skipped tests (4 files with .skip/.todo) in a TODO.md or issue tracker -- Expand CLI entry point tests (test cli.js directly, not just imported modules) - -**Bottom line:** -The test suite is skeptical-tester-approved for robustness and real-world coverage. But CI is broken (false green), and zero real Copilot SDK integration is a blind spot. Fix CI, verify coverage config, add 1 integration test β†’ 🟒 ready. -- **Test Results:** All 1727 tests passing across 57 files. Duration: 4.08s (transform 7.23s, setup 0ms, collect 21.44s, tests 16.15s, environment 12ms, prepare 16.17s). -- **No skipped/pending tests:** Zero `.skip()` or `.only()` patterns found. All 57 test files active. -- **Test file coverage:** Distributed across SDK (config, runtime, agents, casting, coordinator, marketplace, sharing, shell, adapter, tools) and CLI (init, upgrade, export-import, cli-global). Strong test-to-source-file ratio. -- **CI Health:** Recent runs show mixed status on feature branches (squad-UI, feat/remote-squad-mode), but main dev branch (run 103) and most completed runs are green. squad-ci.yml triggers on push/PR to main/bradygaster/dev/insider. Two-job matrix (build-node, test-node) with Node 20/22. Rollup "build" job requires both to pass for branch protection. -- **Coverage Infrastructure:** Vitest configured for v8 provider with text, text-summary, html reporters. Include patterns: `packages/*/src/**/*.ts`. Coverage dir: `./coverage/` (gitignored). - -### πŸ“Œ Team update (2026-02-22T08:50:00Z): Runtime Module Test Patterns β€” decided by Hockney -Two EventBus APIs require different mocks: client bus uses on()/emit(), runtime bus uses subscribe()/emit(). Tests must use correct mock based on module. CharterCompiler tests use real test-fixtures (integration-level confidence); parseCharterMarkdown uses inline strings (unit isolation). Coordinator routing priority verified: direct > @mention > team keyword > default. RalphMonitor tests future-proof stubs. 105 new tests written (1727 β†’ 1832, all passing). -- **Test Patterns:** Good structure observed: pure functions (parsers, coordinators), simple classes (SessionRegistry, StreamBridge), callback-based async (shell lifecycle). Windows symlink tests skipped (elevated privileges). -- **Flaky tests:** One pre-existing flake in export-import CLI tests (timing-sensitive fs operations on first run, passes on retry). Not blocking merges. -- **Known Issues:** None blocking. Pre-existing TS error in cli-entry.ts VERSION export (mentioned in history). Test import migration deferred until root `src/` deletion. - -### Proactive runtime module tests (2026-02-22) -- Created 4 new test files (105 tests) for runtime modules being built in parallel by Fenster, Edie, and Fortier. -- **charter-compiler.test.ts** (34 tests): `parseCharterMarkdown` identity/section/edge cases, `compileCharterFull` metadata/overrides, `CharterCompiler` class compile/compileAll with real test-fixtures charters. Discovered CharterCompiler and AgentSessionManager are now fully implemented (not stubs). -- **agent-session-manager.test.ts** (25 tests): spawn (state, sessionId, timestamps, modes, EventBus events), resume (reactivation, timestamp update, error cases), destroy (map removal, event emission, non-existent agent safety), getAgent/getAllAgents state management. -- **coordinator-routing.test.ts** (27 tests): Coordinator.route() covering direct responses (status/help/show/list/who/what/how), @mention routing (fenster/verbal/hockney), "team" keyword fan-out, default-to-lead, priority ordering (@mention > team, direct > @mention), initialize/execute/shutdown lifecycle. -- **ralph-monitor.test.ts** (19 tests): RalphMonitor start/stop lifecycle, healthCheck, getStatus, config options, edge cases (healthCheck after stop, multiple start/stop calls). -- Test count grew from 1727 to 1832 across 61 files β€” all passing. -- Key edge cases found: (1) @mention priority beats "team" keyword, (2) direct patterns beat @mentions, (3) AgentSessionManager.destroy() is safe on non-existent agents, (4) CharterCompiler.compileAll() silently skips invalid charters. -- Pattern: EventBus mock for AgentSessionManager uses `on()` method (client EventBus pattern), not `subscribe()` (runtime EventBus pattern) β€” the two bus implementations have different APIs. +## πŸ“Œ Phase 2 Test Validation β€” 2026-03-07T01-13-00Z -### OTel observability tests β€” proactive (2026-02-22) -- Created 4 new test files (54 tests) for OTel observability modules being built by Fortier and Edie. -- **otel-provider.test.ts** (20 tests): `initializeOTel` returns `{tracing, metrics}` booleans; `getTracer()`/`getMeter()` return valid no-op instances when unconfigured; `shutdownOTel()` is safe to call with no initialization; config priority verified (explicit endpoint > `OTEL_EXPORTER_OTLP_ENDPOINT` env var > disabled). Also covers `initializeTracing()` and `initializeMetrics()` individually. -- **otel-bridge.test.ts** (12 tests): `createOTelTransport()` returns a function conforming to `TelemetryTransport`. All 5 event types (`squad.init`, `squad.agent.spawn`, `squad.error`, `squad.run`, `squad.upgrade`) produce correctly-named spans. `squad.error` sets `SpanStatusCode.ERROR` and emits an `exception` event. Properties map to span attributes. Batch processing verified. -- **otel-agent-traces.test.ts** (10 tests): Proactive β€” validates that `AgentSessionManager.spawn()` and `destroy()` create OTel spans with agent name and mode attributes. Error spans verified for invalid charters and resume of non-existent agents. Currently pass with `[PROACTIVE]` warnings since OTel instrumentation is not yet wired into AgentSessionManager. -- **otel-coordinator-traces.test.ts** (12 tests): Proactive β€” validates that `Coordinator.route()` creates `squad.coordinator.route` spans with tier/message/agents attributes. Span hierarchy tested (route β†’ execute). Currently pass with `[PROACTIVE]` warnings since OTel instrumentation is not yet wired into Coordinator. -- Test count grew from 1832 to 1886 across 65 files β€” all passing. -- Key discovery: `@opentelemetry/sdk-trace-base` v2.x uses `BasicTracerProvider` (not `NodeTracerProvider`), requires `spanProcessors` in constructor, and uses `trace.setGlobalTracerProvider()` instead of `provider.register()`. -- `AgentSessionInfo` uses `charter.name` and `state` fields (not `name`/`status` directly). -- OTel SDK deps (`@opentelemetry/api`, `@opentelemetry/sdk-trace-base`, `@opentelemetry/sdk-metrics`) installed at root for test resolution. +**BASELINE TEST RUN ON DEV POST-PHASE2-MERGES:** All 5 Phase 2 PRs merged. Full test suite validation run. -### OTel Metrics tests β€” Issues #261-264 (2026-02-23) -- Created `test/otel-metrics.test.ts` (34 tests): Comprehensive coverage of all four metric categories β€” token usage (#261), agent performance (#262), session pool (#263), response latency (#264), plus reset/cleanup and no-op safety. -- Created `test/otel-metric-wiring.test.ts` (5 tests): Integration tests verifying StreamingPipeline calls recordTokenUsage on usage events, module resolution of otel-metrics subpath and barrel exports. -- Testing strategy: Mock `getMeter()` from otel provider to return spy-enabled meter with tracked instruments. Each `createCounter`/`createHistogram`/`createUpDownCounter`/`createGauge` call returns a spy with `.add()` and `.record()` mocks, allowing precise verification of metric names, values, and attributes. -- Key findings: (1) StreamingPipeline has no constructor args β€” just `new StreamingPipeline()`, (2) session attach method is `attachToSession()` not `attachSession()`, (3) `_resetMetrics()` clears all four cached instrument categories independently, (4) all metric functions are safe no-ops when OTel is not configured. -- Test count grew from 1901β†’1940 across 68 files β€” all passing. +- Total test files: 134 +- Passing files: 130 +- Failing tests: 12 (pre-existing, not from Phase 2) -### πŸ“Œ Team update (2026-02-22T093300Z): OTel Phase 2 complete β€” session traces, latency metrics, tool enhancements, agent metrics, token usage wiring, metrics tests β€” decided by Fortier, Fenster, Edie, Hockney -All four agents shipped Phase 2 in parallel: Fortier wired TTFT/duration/throughput metrics. Fenster established tool trace patterns and agent metric wiring conventions. Edie wired token usage and session pool metrics. Hockney created spy-meter test pattern (39 new tests). Total: 1940 tests passing, metrics ready for production telemetry. +**Failures isolated to 4 modules (pre-existing):** +- Consult mode: 6 failures +- REPL UX: 3 failures +- Status command: 2 failures +- Acceptance: 1 failure -### PR #300 upstream inheritance test review β€” requested by Brady (2026-02-23) -- **Verdict: PR #300 does not exist.** No PR, no branch, no source files, no test files found in repo or on GitHub remote. The referenced files (`packages/squad-sdk/src/upstream/resolver.ts`, `packages/squad-sdk/src/upstream/types.ts`, `packages/squad-cli/src/cli/commands/upstream.ts`, `test/upstream.test.ts`, `test/upstream-e2e.test.ts`) do not exist anywhere. -- Searched: all branches (25 remote), all PRs (open/closed), issues, local filesystem, glob patterns. Zero matches for "upstream" in any context. -- Prepared a test coverage requirements spec for when this PR materializes. Key gaps to enforce: CLI command tests (add/remove/list/sync), circular reference detection, .ai-team/ fallback, malformed JSON, empty upstreams array, transitive inheritance proof in E2E. -- Baseline at time of review: 1940 tests across 68 files, all passing. -### Issue #267: OTel integration tests (2026-02-22) -- Created `test/otel-integration.test.ts` (37 tests) covering 9 integration suites across all OTel modules. -- **Bridge + Provider pipeline** (5 tests): End-to-end span capture, error spans with status/exception events, unknown events, timestamp attributes, multiple transports. -- **Bridge span sequencing** (3 tests): Mixed batch integrity, sequential batches, empty-then-nonempty. -- **Agent spawn telemetry flow** (3 tests): Name/mode/model attributes through bridge, multiple independent agents, missing properties handled. -- **Session lifecycle spans** (4 tests): Run/error event mapping, ERROR status chain, full initβ†’spawnβ†’runβ†’error sequence. -- **Metrics end-to-end** (7 tests): Full agent lifecycle, session lifecycle, latency metrics, token usage, concurrent multi-agent, _resetMetrics. -- **Error scenarios** (6 tests): No-op tracer/meter, bridge with no-op, shutdown safety, events without properties/timestamps. -- **Provider lifecycle** (4 tests): Disabled init, independent tracing/metrics, manual provider, cleanup isolation. -- **EventBus β†’ Bridge translation** (3 tests): All 5 event types, error fallback chain, property type preservation. -- **Cross-module coordination** (2 tests): Bridge + direct spans coexist, concurrent transports safe. -- Key pattern: vi.mock at module scope with spyMeter declared globally; vi.importActual() to bypass mock in error scenario tests. -- Test count: 1969 β†’ 2006+ (37 new integration tests, all passing). Pre-existing squad-observer.test.ts failures unrelated. +**Core CLI solid.** No regressions introduced by Phase 2 work. Failures are backlog items for Phase 3 stabilization. -### Issue #267: OTel integration E2E tests + aspire CLI tests (2026-02-23) -- Created `test/otel-integration-e2e.test.ts` (21 tests): Full trace hierarchy, zero-overhead verification, metrics integration, EventBus β†’ OTel bridge. -- Created `test/cli/aspire.test.ts` (16 tests): Docker availability, container command generation, OTLP endpoint configuration, stop/cleanup, module resolution. -- **Trace hierarchy** (5 tests): Request β†’ route β†’ agent β†’ tool span chain with verified parentSpanContext linkage, shared traceId, error isolation, attribute flow, parallel fan-out. -- **Zero-overhead** (5 tests): No-op tracer, no-op metrics, transport with no provider, nested no-op spans, all safe without throwing. -- **Metrics integration** (4 tests): StreamingPipeline usage aggregation, TTFT tracking, unattached session filtering, multi-session independence. -- **EventBus β†’ OTel bridge** (7 tests): TelemetryCollector flush β†’ spans, subscribeAll bridge pattern, tool_call events, ERROR status on session:error, 50-event burst, no-op after disable, detach stops span creation. -- **Aspire CLI** (16 tests): Docker version check, absent Docker handling, default/custom docker run commands (port, OTLP port, container name, image), env var config, OTLP port mapping, stop+rm commands, idempotent lifecycle, [PROACTIVE] module resolution. -- Key discovery: OTel SDK v2 uses `parentSpanContext` (not `parentSpanId`) for parent-child relationships on `ReadableSpan`. -- Key discovery: `BasicTracerProvider` requires explicit `AsyncLocalStorageContextManager` registration for context propagation in vitest β€” without it, `trace.setSpan()` creates contexts but `startSpan()` ignores the parent. -- Key discovery: `bridgeEventBusToOTel` function is referenced in `otel-init.ts` but not yet exported from `otel-bridge.ts` β€” tests use manual bridge pattern. -- Test count grew from 1985 β†’ 2022 across 74 files β€” all passing. - -### Coverage gap audit β€” 4 new test files (2026-02-23) -- Created `test/shell-integration.test.ts` (32 tests): ShellLifecycle startup (7 tests β€” missing .squad/, missing team.md, error state, agent discovery, registry population, ready state, message history), input routing via parseInput (11 tests β€” @agent direct, coordinator fallback, slash commands, comma syntax, case-insensitive, empty input, unknown agent), coordinator response parsing (7 tests β€” ROUTE/DIRECT/MULTI formats, fallback, empty content, missing CONTEXT), formatConversationContext (3 tests β€” prefixes, maxMessages, empty), session cleanup (2 tests β€” registry + history cleared), error handling (2 tests β€” unhealthy HealthMonitor when disconnected, idempotent shutdown). -- Created `test/health.test.ts` (17 tests): HealthMonitor.check() success (3 tests β€” healthy result, ping called with health-check, connection checked first), check() timeout (5 tests β€” ping exceeds timeout β†’ unhealthy, disconnected β†’ unhealthy, no ping when disconnected, ping throws β†’ unhealthy, responseTimeMs captured on failure), check() degraded (2 tests β€” slow response β†’ degraded, default 5000ms timeout), getStatus() (5 tests β€” connected β†’ healthy, disconnected β†’ unhealthy, reconnecting β†’ degraded, error β†’ unhealthy, no ping on passive check), diagnostics (2 tests β€” logs when enabled, silent when disabled). -- Created `test/model-fallback.test.ts` (25 tests): Cross-tier fallback (4 tests β€” standard/premium/fast chain walks all models, null on exhaustion), tier ceiling (5 tests β€” fast chain only fast models, no premium in fast or standard chains, getNextFallback stays in tier, getModelsByTier confirms), provider preference (5 tests β€” preferSameProvider reorders for Claude/GPT, default chain without preference, unknown model fallback), nuclear fallback (5 tests β€” null for all tiers when exhausted, full cascade simulation, chain validity), edge cases (6 tests β€” empty attempted set, unknown model, minimum 3 options per tier, no duplicates, catalog existence). -- Created `test/cli/upstream-clone.test.ts` (40 tests): Git ref validation (14 tests β€” valid branches/tags/underscores, rejects semicolons/backticks/dollar/$pipe/ampersand/newline/spaces/glob/brackets/braces/empty), upstream name validation (3 tests), source type detection (6 tests β€” https/http/file:// URLs, .git suffix, slash heuristic, ambiguous throws), deriveName (4 tests β€” git URL, no .git, export, fallback), git clone args (3 tests β€” valid ref, feature branch, pull args), failure recovery (4 tests β€” network/timeout/permission/not-found errors), file I/O (4 tests β€” missing file, malformed JSON, parent dir creation, round-trip), gitignore behavior (3 tests β€” add to empty, no duplicate, trailing newline). -- Test count grew from 2022 β†’ 2136 across 78 files β€” all passing. -- Key pattern: `getNextFallback` walks by provider preference, not raw chain order β€” visited set comparison is correct, array order comparison is not. -- Key pattern: HealthMonitor.check() uses `Promise.race` for timeout β€” timeout of N ms with ping delay > N produces unhealthy; response > 80% of timeout produces degraded. -- Key pattern: `parseInput` supports both `@Agent message` and `Agent, message` comma syntax for direct addressing. - -### Issue #207: Docs site build verification tests (2026-02-23) -- Created `test/docs-build.test.ts` (17 tests): Comprehensive validation for docs site build pipeline. -- **Markdown validation** (10 tests): All 8 markdown files in docs/guide/ have proper headings, properly fenced code blocks, no broken relative links between guides, header anchors, no empty files. -- **Code example validation** (2 tests): Code blocks contain valid content, bash examples have proper syntax. -- **Docs build script** (5 tests): Conditional tests that verify docs/build.js execution when it exists (created by parallel agents), produces HTML output in docs/dist/, HTML files have proper DOCTYPE/closing tags, contain nav/menu elements, contain main/article/section content areas, have proper internal links. -- Test design: Markdown validation always active (guides exist now). Build script tests are conditional β€” they skip gracefully if docs/build.js doesn't exist yet (other agents still creating). Tests use regex patterns for HTML structure validation (flexible, not brittle). All 17 tests pass. -- Test count grew from 2141 β†’ 2158 across 80 files β€” all passing. - -### Docs build tests upgraded for markdown-it (2026-02-23) -- Rewrote `test/docs-build.test.ts` from 17 β†’ 30 tests for the markdown-it upgrade of docs/build.js. -- **Source markdown validation** (6 tests): Verifies all 14 expected guide files exist, have headings, properly fenced code blocks, non-empty content, bash examples valid. -- **Build execution** (4 tests): build.js exists, runs exit 0, all 14 guides produce HTML in docs/dist/, no unexpected extra files. -- **markdown-it output quality** (7 tests): Code blocks have `language-*` class on `` elements (typescript, bash), tables render as `///
/`, nested lists produce nested `
    ` inside `
  • `, bold/links/inline code rendered correctly. -- **Assets** (1 test): style.css and app.js copied to dist/assets/. -- **Homepage** (1 test): index.html generated with DOCTYPE and "Squad Documentation" content. -- **Frontmatter/title** (2 tests): Title extracted from h1 (not raw ---), no `{{TITLE}}` placeholders left in output. -- **Navigation** (4 tests): Every page has `