Context
Follow-up items identified during PR #94 (feat/minimal-agent-speculative-compaction) merge review. None of these are blocking — the PR passed all CI checks, 100/100 review threads resolved, and Oracle + automated review gave a GO. These are improvements to address in subsequent PRs.
Source PR: #94
Review rounds: 3 (100 threads resolved)
Reviewed by: Oracle (architecture), Explore agents (test coverage + code quality)
1. Test Coverage Gaps (Low Priority)
1a. Thin test files
packages/harness/src/memory-presets.test.ts — only 4 test cases. Verify all preset combinations (min/mid/max token budgets) are covered.packages/cea/src/commands/compact.test.ts — only 3 test cases. Add CLI argument parsing edge cases and error handling paths.
1b. Missing regression tests (Oracle-recommended)
- Stale speculative compaction: speculative completes but user/tool messages added mid-flight → verify stale discard
- Per-turn cap reset: cap reached in turn N → notifyNewUserTurn() → auto/speculative allowed in turn N+1
- Active-scope truncation invariant: collectToolResultEntries must only iterate post-summary messages
- Headless speculative flow: onSpeculativeReady → applyReady → measureUsageAfterCompaction should apply exactly once
1c. End-to-end integration test
No cross-package E2E test for full compaction cycle with JSONL trajectory validation.
2. Code Quality Cleanup (Low Priority)
2a. Test debug output
8 console.log calls in compaction-integration.test.ts (lines 150, 233, 300, 386, 482, 589, 672, 675).
2b. Direct process.env access (pre-existing)
- shell-detection.ts:54 reads process.env.SHELL
- noninteractive-wrapper.ts:279 reads process.env.LC_ALL/LANG
Fix: add to cea env.ts schema.
3. Architecture Concerns (Medium Priority)
3a. Public API surface over-exposure
harness index.ts exports internal helpers (*Core functions) and workflow types. Separate into public stable API vs internal types.
3b. Orchestrator responsibility creep
CompactionOrchestrator serves 5+ roles. Refactor into state machine/strategy if next feature adds more.
3c. notifyNewUserTurn() consistency
Add defensive warning if compaction attempted without prior notifyNewUserTurn().
3d. onApplied detail fields always zero
baseMessageCount/newMessageCount always 0. Fix or remove.
Checklist
Context
Follow-up items identified during PR #94 (
feat/minimal-agent-speculative-compaction) merge review. None of these are blocking — the PR passed all CI checks, 100/100 review threads resolved, and Oracle + automated review gave a GO. These are improvements to address in subsequent PRs.Source PR: #94
Review rounds: 3 (100 threads resolved)
Reviewed by: Oracle (architecture), Explore agents (test coverage + code quality)
1. Test Coverage Gaps (Low Priority)
1a. Thin test files
packages/harness/src/memory-presets.test.ts— only 4 test cases. Verify all preset combinations (min/mid/max token budgets) are covered.packages/cea/src/commands/compact.test.ts— only 3 test cases. Add CLI argument parsing edge cases and error handling paths.1b. Missing regression tests (Oracle-recommended)
1c. End-to-end integration test
No cross-package E2E test for full compaction cycle with JSONL trajectory validation.
2. Code Quality Cleanup (Low Priority)
2a. Test debug output
8 console.log calls in compaction-integration.test.ts (lines 150, 233, 300, 386, 482, 589, 672, 675).
2b. Direct process.env access (pre-existing)
Fix: add to cea env.ts schema.
3. Architecture Concerns (Medium Priority)
3a. Public API surface over-exposure
harness index.ts exports internal helpers (*Core functions) and workflow types. Separate into public stable API vs internal types.
3b. Orchestrator responsibility creep
CompactionOrchestrator serves 5+ roles. Refactor into state machine/strategy if next feature adds more.
3c. notifyNewUserTurn() consistency
Add defensive warning if compaction attempted without prior notifyNewUserTurn().
3d. onApplied detail fields always zero
baseMessageCount/newMessageCount always 0. Fix or remove.
Checklist