Loop helpers extraction slice (#127)

PLAN.md

@@ -0,0 +1,42 @@
+# PLAN — Issue #127 slice: `src/engine/loop.ts` slim-down (tests-first)
+
+## Goal
+Reduce `src/engine/loop.ts:1-6567` (6567 lines, single `HeadlessSession` class at `src/engine/loop.ts:1363`) toward [`docs/typescript-standards.md`](docs/typescript-standards.md:17-24) (250 file / 200 class / 20 function lines) with **no behavior change**. Land a coherent partial split with tests + green gates if the full split does not fit; do not leave a sprawling WIP.
+
+## Scope boundaries (hard)
+- Owns ONLY `src/engine/loop.ts`. Do NOT touch `src/orchestrator/cli.ts`, `resume.ts`, or anything a sibling worker owns per the issue brief.
+- No behavior change: pure moves + thin delegation. Keep all 27 existing importers compiling (importers include [`src/cli.ts`](src/cli.ts:1), [`src/index.ts`](src/index.ts:1), [`src/orchestrator/pipeline.ts`](src/orchestrator/pipeline.ts:1)); keep `src/engine/loop.ts` as a re-export facade during the split.
+- Tests-first is mandatory: test must exist and pass BEFORE the unit it covers is reshaped.
+
+## Starting state (verified)
+- Monolith: `src/engine/loop.ts:1363` `HeadlessSession` + config interfaces `src/engine/loop.ts:744` (`HeadlessSessionConfig`), `src/engine/loop.ts:1218` (`ResolvedSessionConfig`), `src/engine/loop.ts:1208` (`SessionRunState`).
+- Largest method: [`runIterations()`](src/engine/loop.ts:3416) spans ~3416–5487 (~2000 lines) — the endgame; do it last, and accept a partial win.
+- Condensation cluster (extract as a group): [`maybeCondenseHistory()`](src/engine/loop.ts:2584), [`fireBackgroundCondense()`](src/engine/loop.ts:2862), [`applyBackgroundCondense()`](src/engine/loop.ts:2985), [`recordCondensationUsage()`](src/engine/loop.ts:3103), [`recordHandoffCondensationUsage()`](src/engine/loop.ts:3135), [`writeIterationCapHandoff()`](src/engine/loop.ts:3162), [`recordAuxLlmUsage()`](src/engine/loop.ts:3241), [`condensationClient()`](src/engine/loop.ts:3378). Prior art already extracted: [`src/engine/condense.ts:1-60`](src/engine/condense.ts:1), [`src/engine/handoff.ts:42-52`](src/engine/handoff.ts:42).
+- Handler cluster: [`handleNewTask()`](src/engine/loop.ts:5662), [`handleSwitchMode()`](src/engine/loop.ts:5842), [`handleExtensionManage()`](src/engine/loop.ts:5595), [`handleExtensionCall()`](src/engine/loop.ts:5641), [`handleActivateSkill()`](src/engine/loop.ts:6060), [`handleReadSkillReference()`](src/engine/loop.ts:6083), [`handleRequestTool()`](src/engine/loop.ts:6105), [`handleListTools()`](src/engine/loop.ts:6056).
+- Pure free functions (lowest-risk first extraction): [`combineAbortSignals()`](src/engine/loop.ts:173), [`isReadOnlyDirectCall()`](src/engine/loop.ts:361), [`isReadOnlyTurnCall()`](src/engine/loop.ts:379), [`artifactRejectionNudgeMessage()`](src/engine/loop.ts:524), [`stripSupersededReasoning()`](src/engine/loop.ts:572), [`countVerifiedCitations()`](src/engine/loop.ts:613), [`matchingArtifactFileStatus()`](src/engine/loop.ts:649), [`hasUncommittedTrackedChanges()`](src/engine/loop.ts:709), [`estimateTokenSize()`](src/engine/loop.ts:6283), [`summarizeToolArg()`](src/engine/loop.ts:6300), [`truncateHistory()`](src/engine/loop.ts:6474), [`sameToolNames()`](src/engine/loop.ts:6387), [`messagesPrefixEquals()`](src/engine/loop.ts:6415), `env*` helpers [`src/engine/loop.ts:6493-6560`](src/engine/loop.ts:6493).
+- Existing test style: plain `node:assert/strict` scripts via `tsx`, auto-discovered (`scripts/run-tests.mjs:41-55`). Main suite [`src/engine/__tests__/loop.test.ts:1-80`](src/engine/__tests__/loop.test.ts:1) (4402 lines, FakeLlmClient DI pattern at `src/engine/__tests__/loop.test.ts:44-65`). Focused suites exist for condense/pause/inject/switch-mode — mirror them.
+- Check tooling: [`scripts/check-code-standards.mjs:18`](scripts/check-code-standards.mjs:18) limits, [`scripts/check-architecture.mjs:19`](scripts/check-architecture.mjs:19) limits; baselines `scripts/code-standards-baseline.json`, `scripts/architecture-baseline.json` (content-anchored; regenerate with `--write-baseline` in the SAME change, never by hand).
+
+## Steps
+1. **Measure (record numbers for the closing report).** Run: `npm run check-standards -- src/engine/loop.ts`, `npm run check-architecture -- src/engine/loop.ts` (or full runs), `npm run coverage -- --filter engine`. Save outputs; also `wc -l src/engine/loop.ts`.
+2. **Tests FIRST — pure helpers.** Add `src/engine/__tests__/loop-helpers.test.ts` covering every pure function to be moved first (`combineAbortSignals`, `isReadOnlyDirectCall/TurnCall`, `artifactRejectionNudgeMessage`, `stripSupersededReasoning`, `countVerifiedCitations` + `matchingArtifactFileStatus` with temp dirs, `estimateTokenSize`, `summarizeToolArg`, `truncateHistory`, `sameToolNames`, `messagesPrefixEquals`, env helpers). Keep each test file <250 lines — split into `loop-helpers-*.test.ts` if needed. Run `npx tsx <file>` then `npm test -- loop-helpers` until green. Do not move code until these pass.
+3. **Extract pure helpers.** Create `src/engine/loop-helpers.ts` (or two files if >250 lines: `loop-pure.ts` + `loop-history.ts` for truncate/messages fns). Move the step-2 functions verbatim, fix imports, re-export from `loop.ts` so existing `import { … } from "../loop.js"` in tests (`src/engine/__tests__/loop.test.ts:17-30`) keeps working. Verify `npx tsc --noEmit`.
+4. **Tests FIRST — condensation.** Add `src/engine/__tests__/loop-condense.test.ts` using the FakeLlmClient pattern for `maybeCondenseHistory` / `fireBackgroundCondense` / `applyBackgroundCondense` / usage recorders. Reuse fixtures from `src/engine/__tests__/condense.test.ts`, not new harness machinery.
+5. **Extract condensation.** Create `src/engine/loop-condense.ts` taking explicit params (messages, budgetTracker, contextWindowTokens) rather than reaching into private session fields; `HeadlessSession` methods at `src/engine/loop.ts:2584-3415` become thin delegates. Follow the `src/engine/condense.ts` / `src/engine/handoff.ts` module idiom (one layout/concern per file). Verify typecheck.
+6. **Tests FIRST + extract handlers.** Add `src/engine/__tests__/loop-handlers.test.ts` for `handleSwitchMode` / `handleNewTask` / extension + skill handlers with fake executor/registry (see existing `switch-mode.test.ts`, `new-task.test.ts`). Then create `src/engine/loop-handlers.ts`; pass a minimal session-context interface (not the whole `HeadlessSession`) to avoid an import cycle (`cycle` rule allows zero cycles).
+7. **runIterations — partial, coherent only.** Do NOT attempt to split the whole `src/engine/loop.ts:3416-5487` in one go. If time permits, extract ONE seam (e.g. pause/inject polling `src/engine/loop.ts:2439-2583` → `loop-poll.ts`, or checkpoint helpers `src/engine/loop.ts:5488-5575` → `loop-checkpoints.ts`). Prefer landing steps 2–6 green over a half-split runIterations.
+8. **Baselines + gates (all green, same change).** `npx tsc --noEmit`; `npm test` (full); `npm run check-standards`; `npm run check-architecture`; regenerate both baselines in the same commit(s); `npm run coverage -- --min 24` must not regress vs step 1.
+9. **Commit / PR.** One logical commit per extraction (`git add` + `git commit` with `#127` in message). Push branch, open PR (ready, not draft) against `master` linking `#127`. Close with the ORCHESTRATOR_TASK.md template (Files changed / Baseline before+after / Boot check / Residual risk / PR link).
+
+## Verification
+- `npx tsc --noEmit` clean.
+- `npm test` full passes (runner: `scripts/run-tests.mjs:105-177`, fail-fast, sorted).
+- `npm run check-standards` and `npm run check-architecture` pass with regenerated baselines in the same change.
+- `npm run coverage -- --min 24` passes; `npm run coverage -- --filter engine` recorded before/after (must not regress).
+- Boot check: N/A (library refactor, no CLI behavior change) — state that explicitly in the closing report unless a touched path warrants `npx tsx src/cli.ts --help`.
+
+## Risks / notes
+- Content-anchored baselines: editing a flagged line creates a NEW violation key — expect to fix (not baseline) touched lines; keep lines ≤80 chars and functions ≤20 lines in all new files.
+- Cycle risk: new `loop-*.ts` modules must not import `loop.ts` back (zero `cycle` tolerance). Inject a narrow context interface instead.
+- `runIterations` statefulness (locals at `src/engine/loop.ts:3419-3456` like `completionNudged`, `lastExecuteCommandFailed`) makes extraction error-prone — default to leaving it intact and claiming the helpers+condensation+handlers win.
+- Scratch only under `.headlesscode/scratch/`; never `/tmp` (outside-workspace writes fail).