Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 53 additions & 2 deletions docs/audit/FINDINGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,65 @@

> **Purpose:** Real issues, gaps, and risks discovered during code audits and real-world testing.
> **This is NOT a task list.** Tasks live in [TASKS.md](TASKS.md). Findings document _what's wrong_ and _why it matters_.
> **Open:** 0 | **Fixed:** 0 (230 prior findings archived) | **Last Audit:** 2026-03-23
> **Open:** 4 | **Fixed:** 1 (230 prior findings archived) | **Last Audit:** 2026-03-24
> **History:** 230 findings fixed across v0.0.1–v0.1.0. All prior archived in [archive/](archive/).

---

## Open Findings

_No open findings._
### OB-F231 — Keyword classifier misclassifies action requests as quick-answer

- **Severity:** 🟠 High
- **Status:** Open
- **Key Files:** `src/master/classification-engine.ts`
- **Root Cause / Impact:**
Messages containing action verbs ("add", "create", "update", "modify", "delete") are classified as `quick-answer` by the keyword matcher, resulting in only 3 maxTurns. This causes turn exhaustion on tasks that require file reads/writes. Observed: "From the project docs can you add an item for a supplier?" → `quick-answer` → max turns hit.
- **Fix:** Add action-verb detection to the keyword matcher. If the message contains imperative action verbs, classify as `tool-use` minimum (not `quick-answer`), regardless of other keyword signals.

---

### OB-F232 — Turn exhaustion error leaks to user instead of auto-retrying

- **Severity:** 🔴 Critical
- **Status:** ✅ Fixed (2026-03-24)
- **Key Files:** `src/master/master-manager.ts`
- **Root Cause / Impact:**
When a task ends with `turnsExhausted: true`, the raw "Error: Reached max turns (3)" message is sent back to the user via the messaging channel. The user then manually resends the message with the error appended, wasting a round trip and degrading UX. Observed on telegram-2650 → telegram-2654.
- **Fix:** Added Master-level turn-escalation retry mirroring the worker pattern (OB-903). On first exhaustion, auto-retries with `ceil(maxTurns × 1.5)` (capped at 50), injecting partial output as continuation context. Only surfaces guidance to the user if the escalated retry also exhausts.

---

### OB-F233 — Length heuristic over-classifies conversational questions as tool-use

- **Severity:** 🟡 Medium
- **Status:** Open
- **Key Files:** `src/master/classification-engine.ts`
- **Root Cause / Impact:**
The length heuristic promotes long multi-sentence messages to `tool-use` (15 turns) even when the intent is conversational/planning ("I wanna provide you a shopfy store and you extract..."). This wastes model budget on unnecessary tool-use allocations. Length should be a tiebreaker, not a primary classifier signal.
- **Fix:** Check intent before applying length heuristic — interrogative phrasing ("can you", "would it be possible") with no imperative action should remain `quick-answer` or `conversation`. Only promote to `tool-use` when the message contains clear action intent.

---

### OB-F234 — RAG confidence consistently low due to shallow indexing

- **Severity:** 🟡 Medium
- **Status:** Open
- **Key Files:** `src/master/master-manager.ts`, `src/memory/chunk-store.ts`, `src/memory/retrieval.ts`
- **Root Cause / Impact:**
All RAG queries in the session returned confidence 0.32–0.48 with only 2–3 chunks from FTS5. The chunk store had only 4 indexed chunks at startup. The Master is operating mostly blind about the workspace, reducing answer quality and forcing unnecessary tool-use turns to re-read files.
- **Fix:** Trigger re-indexing when RAG confidence is consistently below a threshold (e.g., < 0.5 over N consecutive queries). Consider deeper initial indexing or incremental chunk ingestion when workers read files.

---

### OB-F235 — Timeout clamping fires on every tool-use task

- **Severity:** 🟢 Low
- **Status:** Open
- **Key Files:** `src/master/master-manager.ts`
- **Root Cause / Impact:**
Every `tool-use` classification triggers `WARN: Timeout clamped to message timeout boundary` (originalTimeout 390–480s → safeTimeout 300s). The calculated timeout consistently exceeds the safe boundary, making the warning noise rather than a useful signal. Either the timeout formula or the safe boundary needs adjustment.
- **Fix:** Either raise the message timeout boundary to accommodate typical tool-use durations, or adjust the timeout formula so tool-use tasks don't routinely exceed the boundary. Alternatively, lower the log level to DEBUG if clamping is expected behavior.

---

Expand Down
4 changes: 2 additions & 2 deletions src/core/agent-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ export function setAgentRunnerMetrics(collector: MetricsCollector | null): void
* the process timeout kills it (OB-F14: exit code 143 / SIGTERM).
*/

/** Max turns for exploration tasks (file listing, classification) — fast, bounded */
export const DEFAULT_MAX_TURNS_EXPLORATION = 15;
/** Max turns for exploration tasks (file listing, classification) — needs headroom for monorepos */
export const DEFAULT_MAX_TURNS_EXPLORATION = 25;

/** Max turns for user-facing tasks (implementation, reasoning) — more room to work */
export const DEFAULT_MAX_TURNS_TASK = 25;
Expand Down
2 changes: 1 addition & 1 deletion src/master/exploration-coordinator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ const DirectoryDiveResultAISchema = DirectoryDiveResultSchema.extend({
durationMs: z.number().int().nonnegative().optional().default(0),
}) as z.ZodType<DirectoryDiveResult>;

const PHASE_TIMEOUT = 300_000; // 5 minutes per phase (large workspaces need more time)
const PHASE_TIMEOUT = 600_000; // 10 minutes per phase (large workspaces with multiple sub-projects)
const DIRECTORY_DIVE_TIMEOUT = 180_000; // 3 minutes per directory dive
const MAX_DIRECTORY_DIVE_TIMEOUT = 600_000; // 10 minutes max per directory dive
const MAX_RETRIES = 3;
Expand Down
52 changes: 46 additions & 6 deletions src/master/master-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3773,16 +3773,56 @@ export class MasterManager {

let response = result.stdout.trim() || 'No response from AI';

// Guard: Master session itself hit max-turns — provide actionable feedback (OB-F230)
// Guard: Master session itself hit max-turns — auto-retry with escalated budget (OB-F232)
// Mirrors the worker turn-escalation pattern (OB-903) at the Master level.
if (result.turnsExhausted) {
const partial = response.length > 20 ? response : '';
const turnsUsedStr = result.turnsUsed ? ` (used ${result.turnsUsed} turns)` : '';
const originalMaxTurns = maxTurnsToUse;
const escalatedMaxTurns = Math.min(Math.ceil(originalMaxTurns * 1.5), 50);

logger.warn(
{ taskId, maxTurns: maxTurnsToUse, turnsUsed: result.turnsUsed },
'Master session hit max-turns limit — returning partial result with guidance',
{ taskId, originalMaxTurns, escalatedMaxTurns, turnsUsed: result.turnsUsed },
'Master session hit max-turns limit — auto-retrying with escalated budget',
);

const partialOutput = result.stdout.trim();
const incompleteMatch = partialOutput.match(/\[INCOMPLETE:\s*([^\]]+)\]/i);
const continuationNote = incompleteMatch?.[1]
? `Previous attempt was incomplete: ${incompleteMatch[1].trim()}. Continue from where it left off.`
: 'Previous attempt hit the turn limit before completing. Continue from where it left off.';

const escalationPrompt = [
promptToSend,
'',
'---',
'CONTEXT FROM PREVIOUS ATTEMPT (partial output):',
partialOutput.slice(-2000),
'---',
continuationNote,
].join('\n');

const escalationOpts = this.buildMasterSpawnOptions(
escalationPrompt,
safeTimeout,
escalatedMaxTurns,
masterContext,
);
result = await this.agentRunner.spawn(escalationOpts);
await this.updateMasterSession();
response = result.stdout.trim() || 'No response from AI';

if (result.turnsExhausted) {
logger.warn(
{ taskId, escalatedMaxTurns, turnsUsed: result.turnsUsed },
'Escalated retry also exhausted — returning partial result with guidance',
);
}
}

// If still exhausted after escalation, provide actionable feedback (OB-F230)
if (result.turnsExhausted) {
const partial = response.length > 20 ? response : '';
const turnsUsedStr = result.turnsUsed ? ` (used ${result.turnsUsed} turns)` : '';
if (partial && !hasSpawnMarkers(partial)) {
// Master produced some useful output before exhaustion — append guidance
response =
partial +
`\n\n⚠️ I ran out of processing capacity${turnsUsedStr}. ` +
Expand Down
10 changes: 5 additions & 5 deletions tests/core/agent-runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,16 +324,16 @@ describe('Tool group constants', () => {
// ── Max-turns defaults ──────────────────────────────────────────────

describe('Max-turns defaults', () => {
it('DEFAULT_MAX_TURNS_EXPLORATION is 15', () => {
expect(DEFAULT_MAX_TURNS_EXPLORATION).toBe(15);
it('DEFAULT_MAX_TURNS_EXPLORATION is 25 (monorepo headroom)', () => {
expect(DEFAULT_MAX_TURNS_EXPLORATION).toBe(25);
});

it('DEFAULT_MAX_TURNS_TASK is 25', () => {
expect(DEFAULT_MAX_TURNS_TASK).toBe(25);
});

it('exploration default is lower than task default', () => {
expect(DEFAULT_MAX_TURNS_EXPLORATION).toBeLessThan(DEFAULT_MAX_TURNS_TASK);
it('exploration default does not exceed task default', () => {
expect(DEFAULT_MAX_TURNS_EXPLORATION).toBeLessThanOrEqual(DEFAULT_MAX_TURNS_TASK);
});

it('--max-turns is always present in args even without explicit maxTurns', () => {
Expand All @@ -348,7 +348,7 @@ describe('Max-turns defaults', () => {
maxTurns: DEFAULT_MAX_TURNS_EXPLORATION,
});
const idx = args.indexOf('--max-turns');
expect(args[idx + 1]).toBe('15');
expect(args[idx + 1]).toBe('25');
});
});

Expand Down
8 changes: 7 additions & 1 deletion tests/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,20 @@ import * as os from 'node:os';
* by interrupted test runs. Runs once before all tests.
*/
async function cleanStaleTestWorkspaces(): Promise<void> {
const STALE_THRESHOLD_MS = 300_000; // 5 minutes — only clean dirs older than this
const now = Date.now();
const dirs = [process.cwd(), os.tmpdir()];
for (const dir of dirs) {
try {
const entries = await fs.readdir(dir);
const stale = entries.filter((e) => e.startsWith('test-workspace-'));
for (const entry of stale) {
try {
await fs.rm(path.join(dir, entry), { recursive: true, force: true });
const fullPath = path.join(dir, entry);
const stat = await fs.stat(fullPath);
if (now - stat.mtimeMs > STALE_THRESHOLD_MS) {
await fs.rm(fullPath, { recursive: true, force: true });
}
} catch {
// Ignore individual cleanup errors
}
Expand Down
Loading