fix(ts): close escalation/approval/autonomy correctness and security gaps

- approval: a partial `{ approved: false }` callback result no longer flips to
  an approval (non-empty object was truthy); normalise via approvalDecision
- approval: ConsoleBackend readline resolves the answer before rl.close() so
  the synchronous close event no longer rejects every answer as EOF
- autonomy: AUTO_EDIT_TOOL_PATTERN no longer auto-approves unrelated tools via
  an open suffix (write_to_s3, get_secret, update_iam_policy); closed suffix set
- autonomy: actionKey serialises nested args (recursive stable stringify) so
  distinct tool calls differing only in a nested field don't collide
- escalation: DIRECT stage strips tools for the call (no-tools contract) and
  restores them in finally
- escalation: a failed AUTONOMOUS stage records an error and success now
  requires the final stage to have succeeded (no false success)
- escalation: DoomLoopConfig rejects non-positive/non-integer count and
  chunk-size options (contentChunkSize:0 caused an infinite loop)
- tests: regression coverage for each fix

Co-authored-by: Mervin Praison <[email protected]>

src/praisonai-ts/src/agent/features/autonomy.ts

@@ -116,9 +116,14 @@ export function resolveAutonomy(
 /**
  * Tools an `auto_edit` agent may run without asking: reads and file edits.
  * Anything else (commands, network, deletes, deploys) still prompts.
+ *
+ * Matches only the exact verb (`edit`) or a `<verb>_file` / `<verb>_text`
+ * variant. An open suffix such as `([_-].*)?` would auto-approve unrelated
+ * tools like `write_to_s3`, `update_iam_policy`, or `get_secret`, so the
+ * accepted suffix is a small closed set rather than "anything".
  */
 export const AUTO_EDIT_TOOL_PATTERN =
-  /^(read|list|get|glob|grep|search|find|view|cat|stat|head|tail|edit|write|create|apply_patch|patch|replace|insert|append|multi_edit|str_replace|update)([_-].*)?$/i;
+  /^(read|list|get|glob|grep|search|find|view|cat|stat|head|tail|edit|write|create|apply_patch|patch|replace|insert|append|multi_edit|str_replace|update)(_(file|files|text|line|lines|content))?$/i;
 
 /** Which tools the level lets through without approval. */
 export function isAutoApprovedByLevel(level: AutonomyLevel, toolName: string): boolean {
@@ -142,6 +147,21 @@ export function applyAutonomyToApproval(config: AutonomyConfig, manager: Approva
 
 export type DoomLoopRecovery = 'continue' | 'retry_different' | 'escalate_model' | 'request_help' | 'abort';
 
+/**
+ * Deterministic JSON with keys sorted at every depth. `JSON.stringify(value,
+ * replacerArray)` only sorts the top level and, worse, drops any nested key
+ * not present in the array — so distinct tool calls that differ only in a
+ * nested argument (e.g. `opts.recursive`) would collide. This recurses so the
+ * full argument shape contributes to the key.
+ */
+function stableStringify(value: unknown): string {
+  if (value === null || typeof value !== 'object') return JSON.stringify(value) ?? 'null';
+  if (Array.isArray(value)) return `[${value.map(stableStringify).join(',')}]`;
+  const entries = Object.keys(value as Record<string, unknown>).sort()
+    .map((key) => `${JSON.stringify(key)}:${stableStringify((value as Record<string, unknown>)[key])}`);
+  return `{${entries.join(',')}}`;
+}
+
 interface RecordedAction {
   key: string;
   name: string;
@@ -167,7 +187,7 @@ export class DoomLoopTracker {
   static actionKey(name: string, args: unknown): string {
     let serialised: string;
     try {
-      serialised = JSON.stringify(args, Object.keys((args as Record<string, unknown>) ?? {}).sort());
+      serialised = stableStringify(args);
     } catch {
       serialised = String(args);
     }

src/praisonai-ts/src/approval/backends.ts

@@ -481,10 +481,16 @@ export class ConsoleBackend implements ApprovalBackend {
     const readline = await import('readline');
     const rl = readline.createInterface({ input: this.input, output: this.output });
     return new Promise<string>((resolve, reject) => {
-      rl.once('close', () => reject(new Error('EOF')));
+      let settled = false;
+      rl.once('close', () => {
+        // `rl.close()` fires this synchronously; only a genuine EOF (close
+        // before any answer) should reject.
+        if (!settled) reject(new Error('EOF'));
+      });
       rl.question(question, (answer) => {
-        rl.close();
+        settled = true;
         resolve(answer);
+        rl.close();
       });
     });
   }
@@ -735,8 +741,18 @@ function isDecision(value: unknown): value is ApprovalBackendDecision {
     && 'reason' in (value as object);
 }
 
+/** Object carrying an explicit boolean `approved` field (may omit `reason`). */
+function hasApprovedField(value: unknown): value is Partial<ApprovalBackendDecision> & { approved: boolean } {
+  return typeof value === 'object' && value !== null
+    && typeof (value as { approved?: unknown }).approved === 'boolean';
+}
+
 function coerceDecision(result: unknown): ApprovalBackendDecision {
   if (isDecision(result)) return result;
+  // A partial decision such as `{ approved: false }` must stay a denial:
+  // normalise it through approvalDecision rather than falling through to the
+  // truthiness path below (a non-empty object is truthy, which would flip it).
+  if (hasApprovedField(result)) return approvalDecision({ ...result });
   if (typeof result === 'boolean') return approvalDecision({ approved: result });
   return approvalDecision({ approved: Boolean(result) });
 }

src/praisonai-ts/src/escalation/doom-loop.ts

@@ -81,6 +81,21 @@ export interface DoomLoopConfigOptions {
   contentChunkSize?: number;
 }
 
+/**
+ * Coerce a count/chunk-size option to a positive integer, rejecting values
+ * that would break the detectors. `contentChunkSize: 0` would make
+ * `recordResponse()` loop forever (the index never advances) and
+ * `maxIdenticalActions: 0` would dereference an empty slice, so a non-finite
+ * or non-positive value is a configuration error.
+ */
+function positiveInt(value: number | undefined, fallback: number, name: string): number {
+  if (value === undefined) return fallback;
+  if (!Number.isInteger(value) || value < 1) {
+    throw new RangeError(`DoomLoopConfig.${name} must be a positive integer, got ${value}`);
+  }
+  return value;
+}
+
 /**
  * Configuration for doom loop detection.
  *
@@ -104,21 +119,21 @@ export class DoomLoopConfig {
   contentChunkSize: number;
 
   constructor(options: DoomLoopConfigOptions = {}) {
-    this.maxIdenticalActions = options.maxIdenticalActions ?? 3;
-    this.maxSimilarActions = options.maxSimilarActions ?? 5;
-    this.maxConsecutiveFailures = options.maxConsecutiveFailures ?? 3;
-    this.maxNoProgressSteps = options.maxNoProgressSteps ?? 5;
+    this.maxIdenticalActions = positiveInt(options.maxIdenticalActions, 3, 'maxIdenticalActions');
+    this.maxSimilarActions = positiveInt(options.maxSimilarActions, 5, 'maxSimilarActions');
+    this.maxConsecutiveFailures = positiveInt(options.maxConsecutiveFailures, 3, 'maxConsecutiveFailures');
+    this.maxNoProgressSteps = positiveInt(options.maxNoProgressSteps, 5, 'maxNoProgressSteps');
     this.similarityThreshold = options.similarityThreshold ?? 0.85;
     this.maxTimePerAction = options.maxTimePerAction ?? 60.0;
     this.maxTotalTime = options.maxTotalTime ?? 300.0;
     this.enableAutoRecovery = options.enableAutoRecovery ?? true;
-    this.maxRecoveryAttempts = options.maxRecoveryAttempts ?? 2;
+    this.maxRecoveryAttempts = positiveInt(options.maxRecoveryAttempts, 2, 'maxRecoveryAttempts');
     this.escalateOnLoop = options.escalateOnLoop ?? true;
     this.initialBackoff = options.initialBackoff ?? 1.0;
     this.backoffMultiplier = options.backoffMultiplier ?? 2.0;
     this.maxBackoff = options.maxBackoff ?? 30.0;
-    this.maxRepeatedChunks = options.maxRepeatedChunks ?? 8;
-    this.contentChunkSize = options.contentChunkSize ?? 50;
+    this.maxRepeatedChunks = positiveInt(options.maxRepeatedChunks, 8, 'maxRepeatedChunks');
+    this.contentChunkSize = positiveInt(options.contentChunkSize, 50, 'contentChunkSize');
   }
 }
 

src/praisonai-ts/src/escalation/pipeline.ts

@@ -166,6 +166,7 @@ export class EscalationPipeline {
     let currentStage = stage;
 
     let response = '';
+    let stageSucceeded = false;
     const errors: string[] = [];
     const warnings: string[] = [];
 
@@ -177,6 +178,7 @@ export class EscalationPipeline {
         const stageResult = await this.executeStage(currentStage, prompt);
         response = stageResult.response ?? '';
         const success = stageResult.success ?? false;
+        stageSucceeded = success;
 
         this.observability?.emit(ObservabilityEventType.STAGE_EXIT, { stage: stageName(currentStage), success });
 
@@ -249,8 +251,16 @@ export class EscalationPipeline {
           }
         }
 
-        // Success or can't escalate further
-        if (success || currentStage === EscalationStage.AUTONOMOUS) {
+        // Terminal: succeeded, or failed at the top stage with nowhere to
+        // escalate. Record the failure so a failed AUTONOMOUS stage is not
+        // reported as a success. When autoEscalate is off the loop instead
+        // retries the current stage; the doom-loop detector is the stopping
+        // mechanism there (Python parity), so we do not break here.
+        if (success) {
+          break;
+        }
+        if (currentStage === EscalationStage.AUTONOMOUS) {
+          errors.push(response || 'Escalation stage failed without recovery');
           break;
         }
 
@@ -281,7 +291,7 @@ export class EscalationPipeline {
 
     return new EscalationResult({
       response,
-      success: errors.length === 0,
+      success: stageSucceeded && errors.length === 0,
       initialStage,
       finalStage: currentStage,
       escalations,
@@ -323,12 +333,23 @@ export class EscalationPipeline {
     if (!this.agent) {
       return { response: 'Agent not configured', success: false };
     }
+    // DIRECT is a no-tools stage: strip any configured tools for this call so
+    // a tool-equipped agent cannot invoke them, then restore afterwards.
+    const hasTools = 'tools' in this.agent;
+    const originalTools = this.agent.tools;
+    if (hasTools) {
+      this.agent.tools = [];
+    }
     try {
       const response = await this.chat(this.agent, prompt);
       this.currentContext?.addStep('direct_response', response.slice(0, 100), true);
       return { response, success: true };
     } catch (e) {
       return { response: errorText(e), success: false };
+    } finally {
+      if (hasTools) {
+        this.agent.tools = originalTools;
+      }
     }
   }
 

src/praisonai-ts/tests/unit/agent/autonomy-security.test.ts

@@ -0,0 +1,41 @@
+/**
+ * Regression tests for src/agent/features/autonomy.ts security fixes:
+ * - AUTO_EDIT_TOOL_PATTERN must not auto-approve unrelated tools via an open suffix.
+ * - DoomLoopTracker.actionKey must keep nested arguments distinct.
+ */
+
+import { describe, it, expect } from '@jest/globals';
+import {
+  AUTO_EDIT_TOOL_PATTERN,
+  isAutoApprovedByLevel,
+  DoomLoopTracker,
+} from '../../../src/agent/features/autonomy';
+
+describe('AUTO_EDIT_TOOL_PATTERN', () => {
+  it('auto-approves reads and file edits', () => {
+    for (const name of ['read', 'edit', 'write', 'read_file', 'write_text', 'multi_edit', 'str_replace']) {
+      expect(AUTO_EDIT_TOOL_PATTERN.test(name)).toBe(true);
+    }
+  });
+
+  it('does not auto-approve unrelated tools sharing a verb prefix', () => {
+    for (const name of ['write_to_s3', 'update_iam_policy', 'get_secret', 'read_credentials', 'delete_bucket']) {
+      expect(AUTO_EDIT_TOOL_PATTERN.test(name)).toBe(false);
+      expect(isAutoApprovedByLevel('auto_edit', name)).toBe(false);
+    }
+  });
+});
+
+describe('DoomLoopTracker.actionKey', () => {
+  it('distinguishes calls that differ only in a nested argument', () => {
+    const a = DoomLoopTracker.actionKey('run', { path: '/x', opts: { recursive: true } });
+    const b = DoomLoopTracker.actionKey('run', { path: '/x', opts: { recursive: false } });
+    expect(a).not.toBe(b);
+  });
+
+  it('is stable regardless of key order', () => {
+    const a = DoomLoopTracker.actionKey('run', { a: 1, b: { c: 2, d: 3 } });
+    const b = DoomLoopTracker.actionKey('run', { b: { d: 3, c: 2 }, a: 1 });
+    expect(a).toBe(b);
+  });
+});

src/praisonai-ts/tests/unit/approval/backends.test.ts

@@ -240,6 +240,17 @@ describe('CallbackBackend', () => {
     expect((await asyncCb.requestApproval(request())).approved).toBe(false);
     expect(() => asyncCb.requestApprovalSync(request())).toThrow('use requestApproval()');
   });
+
+  it('keeps a partial denial ({ approved: false }, no reason) a denial', async () => {
+    // Regression: a non-empty object is truthy, so Boolean(result) once flipped
+    // an explicit denial into an approval.
+    const denyNoReason = new CallbackBackend(() => ({ approved: false }));
+    expect(denyNoReason.requestApprovalSync(request()).approved).toBe(false);
+    expect((await denyNoReason.requestApproval(request())).approved).toBe(false);
+
+    const approveNoReason = new CallbackBackend(() => ({ approved: true }));
+    expect(approveNoReason.requestApprovalSync(request()).approved).toBe(true);
+  });
 });
 
 describe('AgentApproval', () => {

src/praisonai-ts/tests/unit/escalation/doom-loop.test.ts

@@ -23,6 +23,18 @@ describe('DoomLoopConfig', () => {
     expect(c.maxRepeatedChunks).toBe(8);
     expect(c.contentChunkSize).toBe(50);
   });
+
+  it('rejects non-positive or non-integer count and chunk-size options', () => {
+    // contentChunkSize: 0 would make recordResponse() loop forever; a zero
+    // count would dereference an empty slice.
+    expect(() => new DoomLoopConfig({ contentChunkSize: 0 })).toThrow(RangeError);
+    expect(() => new DoomLoopConfig({ maxIdenticalActions: 0 })).toThrow(RangeError);
+    expect(() => new DoomLoopConfig({ maxRecoveryAttempts: -1 })).toThrow(RangeError);
+    expect(() => new DoomLoopConfig({ contentChunkSize: 2.5 })).toThrow(RangeError);
+    expect(() => new DoomLoopConfig({ maxSimilarActions: NaN })).toThrow(RangeError);
+    // Valid positive integers are accepted unchanged.
+    expect(new DoomLoopConfig({ contentChunkSize: 10 }).contentChunkSize).toBe(10);
+  });
 });
 
 describe('DoomLoopDetector repeated identical tool calls', () => {

src/praisonai-ts/tests/unit/escalation/pipeline.test.ts

@@ -132,6 +132,25 @@ describe('EscalationPipeline', () => {
     expect(agent.calls).toEqual(['hi']);
   });
 
+  it('strips tools during the DIRECT stage and restores them afterwards', async () => {
+    const original = [{ name: 'run_shell' }];
+    let toolsSeenByChat: unknown;
+    const agent = {
+      calls: [] as string[],
+      tools: original,
+      async chat(prompt: string) {
+        this.calls.push(prompt);
+        toolsSeenByChat = this.tools;
+        return 'ok';
+      },
+    };
+    const pipeline = new EscalationPipeline({ agent, config: { autoDeescalate: false } });
+    await pipeline.executeAtStage('hi', EscalationStage.DIRECT);
+    expect(toolsSeenByChat).toEqual([]);
+    // Restored after the call.
+    expect(agent.tools).toBe(original);
+  });
+
   it('stops at AUTONOMOUS when every stage fails and reports the failure', async () => {
     const agent = agentFailingAt([0, 1, 2, 3]);
     const pipeline = new EscalationPipeline({ agent, doomLoopConfig: { initialBackoff: 0 } });