making editor more resistant for suggestions trailing effect and permissions issues

Author: igorcosta

src/core/SuggestionEngine.ts

@@ -29,7 +29,14 @@ Examples of good startup suggestions:
 const MAX_SUGGESTION_LENGTH = 80;
 /** Max conversation messages included in the suggestion prompt (system prompt added on top). */
 const MAX_HISTORY_MESSAGES = 6; // 3 user+assistant pairs → 7 messages total sent to LLM
-const SUGGESTION_TIMEOUT_MS = 3000;
+/** Max characters per message to keep the suggestion prompt small and fast. */
+const MAX_MESSAGE_CONTENT_LENGTH = 500;
+/**
+ * Internal timeout for the background LLM call. Set higher than the user-facing
+ * deadline in promptForInstruction (3s) so the request can finish in the background
+ * and be available for the next prompt cycle.
+ */
+const SUGGESTION_TIMEOUT_MS = 10_000;
 
 export interface SuggestionEngineOptions {
   /** When provided, constrains suggestions to only actions achievable with these tools. */
@@ -80,7 +87,24 @@ export class SuggestionEngine {
   }
 
   async generate(history: LLMMessage[]): Promise<void> {
-    const recentHistory = history.slice(-MAX_HISTORY_MESSAGES);
+    // Clear stale suggestion from previous turn immediately so that a lazy
+    // provider (e.g., `() => engine.getSuggestion()`) won't return outdated text
+    // while the new LLM call is in flight.
+    this.suggestion = null;
+
+    // Strip tool messages, empty assistant messages (tool-call-only turns),
+    // and internal metadata (tool_calls, priority, etc.) to avoid breaking
+    // the LLM API with orphaned tool responses or invalid sequences.
+    const cleanHistory = history
+      .filter(m => (m.role === 'user' || m.role === 'assistant') &&
+                   typeof m.content === 'string' && m.content.trim().length > 0)
+      .map(m => ({
+        role: m.role,
+        content: m.content.length > MAX_MESSAGE_CONTENT_LENGTH
+          ? m.content.slice(0, MAX_MESSAGE_CONTENT_LENGTH) + '…'
+          : m.content,
+      }));
+    const recentHistory = cleanHistory.slice(-MAX_HISTORY_MESSAGES);
     await this.executeWithTimeout([
       { role: 'system', content: SUGGESTION_SYSTEM_PROMPT + this.toolConstraint },
       ...recentHistory,
@@ -107,8 +131,10 @@ export class SuggestionEngine {
 
     const controller = new AbortController();
     this.abortController = controller;
+    const debug = process.env.AUTOHAND_DEBUG === '1';
 
     const timeout = setTimeout(() => controller.abort(), SUGGESTION_TIMEOUT_MS);
+    const startTime = Date.now();
 
     try {
       const response = await this.llm.complete({
@@ -119,20 +145,27 @@ export class SuggestionEngine {
       });
 
       if (controller.signal.aborted) {
+        if (debug) process.stderr.write(`[SUGGESTION] Aborted after ${Date.now() - startTime}ms\n`);
         return;
       }
 
       const raw = (response.content ?? '').trim();
       if (!raw) {
         this.suggestion = null;
+        if (debug) process.stderr.write(`[SUGGESTION] Empty response after ${Date.now() - startTime}ms\n`);
         return;
       }
 
       this.suggestion = sanitizeSuggestion(raw);
-    } catch {
+      if (debug) process.stderr.write(`[SUGGESTION] Generated "${this.suggestion}" in ${Date.now() - startTime}ms\n`);
+    } catch (err) {
       if (!controller.signal.aborted) {
         this.suggestion = null;
       }
+      if (debug) {
+        const msg = err instanceof Error ? err.message : String(err);
+        process.stderr.write(`[SUGGESTION] Error after ${Date.now() - startTime}ms: ${msg}\n`);
+      }
     } finally {
       clearTimeout(timeout);
       if (this.abortController === controller) {

src/core/agent.ts

@@ -749,7 +749,8 @@ export class AutohandAgent {
       maxQueueSize: 10,
       silentMode: disableTerminalRegions,
       workspaceRoot: this.runtime.workspaceRoot,
-      resolveShellSuggestion: (input) => this.resolveLlmShellSuggestion(input)
+      resolveShellSuggestion: (input) => this.resolveLlmShellSuggestion(input),
+      suggestionProvider: () => this.suggestionEngine?.getSuggestion() ?? undefined,
     });
 
     this.persistentInput.on('queued', (text: string, count: number) => {
@@ -1012,6 +1013,7 @@ export class AutohandAgent {
           recentFiles,
         });
       })();
+      this.persistentInput.setPendingSuggestion(this.pendingSuggestion);
     }
 
     // Show prompt immediately - don't wait for init
@@ -1444,6 +1446,7 @@ If lint or tests fail, report the issues but do NOT commit.`;
         // so the LLM call runs concurrently with hooks/notifications below.
         if (this.suggestionEngine) {
           this.pendingSuggestion = this.suggestionEngine.generate(this.conversation.history());
+          this.persistentInput.setPendingSuggestion(this.pendingSuggestion);
         }
 
         // Fire stop hook after turn completes (non-blocking)
@@ -1596,16 +1599,23 @@ If lint or tests fail, report the issues but do NOT commit.`;
     // otherwise the default placeholder is shown.
     // Turns: wait up to 3s. The user is still reading output so a brief
     // wait for contextual ghost text is acceptable.
-    if (this.pendingSuggestion) {
-      if (!this.isStartupSuggestion) {
-        const deadline = new Promise<void>((r) => setTimeout(r, 3000));
-        await Promise.race([this.pendingSuggestion, deadline]).catch(() => {});
-      }
-      this.isStartupSuggestion = false;
-      this.pendingSuggestion = null;
-    }
-    const suggestionText = this.suggestionEngine?.getSuggestion() ?? undefined;
-    this.suggestionEngine?.clear();
+    // Suggestion uses a lazy provider: each render cycle in the prompt reads
+    // the latest value via getSuggestion(). This eliminates the race condition
+    // where the LLM takes >3s and the static snapshot was always undefined.
+    // The pendingSuggestion promise triggers a re-render when it resolves,
+    // so the ghost text appears as soon as the LLM responds — even if the
+    // prompt is already displayed.
+    const pendingSuggestion = this.pendingSuggestion;
+    this.isStartupSuggestion = false;
+    this.pendingSuggestion = null;
+
+    const debugSuggestion = process.env.AUTOHAND_DEBUG === '1';
+    if (debugSuggestion) {
+      const state = pendingSuggestion ? 'pending' : 'none';
+      process.stderr.write(`[SUGGESTION] Provider mode — pending=${state}, engine=${this.suggestionEngine ? 'exists' : 'null'}\n`);
+    }
+
+    const engine = this.suggestionEngine;
     const input = await readInstruction(
       () => this.workspaceFileCollector.getCachedFiles(),
       SLASH_COMMANDS,
@@ -1614,8 +1624,9 @@ If lint or tests fail, report the issues but do NOT commit.`;
       (data, mimeType, filename) => this.imageManager.add(data, mimeType, filename),
       this.runtime.workspaceRoot,
       initialValue,
-      suggestionText,
-      (line) => this.resolveLlmShellSuggestion(line)
+      () => engine?.getSuggestion() ?? undefined,
+      (line) => this.resolveLlmShellSuggestion(line),
+      pendingSuggestion ?? undefined
     );
     // Only exit on explicit ABORT (double Ctrl+C). Palette cancel or dismiss should continue.
     if (input === 'ABORT') { // double Ctrl+C from prompt
@@ -4327,7 +4338,11 @@ If lint or tests fail, report the issues but do NOT commit.`;
     if (this.inkRenderer) {
       this.inkRenderer.setStatus(status);
     } else if (this.runtime.spinner) {
+      // setSpinnerStatus already handles terminal regions internally
       this.setSpinnerStatus(status);
+    } else if (this.isUsingTerminalRegionsForActiveTurn()) {
+      // No spinner (suppressed when persistent input is used) — route directly
+      this.setPersistentInputActivityLine(status);
     }
   }
 
@@ -4821,6 +4836,8 @@ If lint or tests fail, report the issues but do NOT commit.`;
         this.inkRenderer.setElapsed(elapsed);
       } else if (this.runtime.spinner) {
         this.setSpinnerStatus(status);
+      } else if (this.isUsingTerminalRegionsForActiveTurn()) {
+        this.setPersistentInputActivityLine(status);
       }
     };
     update();

src/permissions/PermissionManager.ts

@@ -15,6 +15,8 @@ import {
   addToLocalWhitelist,
   mergePermissions
 } from './localProjectPermissions.js';
+import { matchesToolPattern } from './toolPatterns.js';
+import type { ToolPattern } from './toolPatterns.js';
 
 /**
  * Default security blacklist - always blocked patterns for sensitive files and dangerous commands.
@@ -213,6 +215,12 @@ export class PermissionManager {
       return { allowed: false, reason: 'blacklisted' };
     }
 
+    // Pattern-based checks (AFTER security blacklist, BEFORE session cache)
+    const patternDecision = this.checkPatterns(context);
+    if (patternDecision) {
+      return patternDecision;
+    }
+
     const cacheKey = this.getCacheKey(context);
 
     // Check session cache
@@ -349,6 +357,71 @@ export class PermissionManager {
     return `${context.tool}:${dir}/*`;
   }
 
+  /**
+   * Convert a PermissionContext to the { kind, target } shape used by matchesToolPattern.
+   */
+  private contextToCall(context: PermissionContext): { kind: string; target: string } {
+    return { kind: context.tool, target: this.getFullCommand(context) };
+  }
+
+  /**
+   * Check pattern-based allow/deny rules (denyPatterns, availableTools, excludedTools,
+   * allowPatterns, allPathsAllowed, allUrlsAllowed).
+   * Returns a decision when a pattern fires, or null to continue with normal flow.
+   */
+  private checkPatterns(context: PermissionContext): PermissionDecision | null {
+    const settings = this.getMergedSettings();
+    const call = this.contextToCall(context);
+
+    // 1. denyPatterns – always denied
+    if (settings.denyPatterns?.length) {
+      for (const p of settings.denyPatterns) {
+        if (matchesToolPattern(p, call)) {
+          return { allowed: false, reason: 'pattern_denied' };
+        }
+      }
+    }
+
+    // 2. availableTools – if non-empty, tool must appear in the list
+    if (settings.availableTools?.length) {
+      const inAvailable = settings.availableTools.some(p => matchesToolPattern(p, call));
+      if (!inAvailable) {
+        return { allowed: false, reason: 'not_in_available' };
+      }
+    }
+
+    // 3. excludedTools – always denied
+    if (settings.excludedTools?.length) {
+      for (const p of settings.excludedTools) {
+        if (matchesToolPattern(p, call)) {
+          return { allowed: false, reason: 'excluded' };
+        }
+      }
+    }
+
+    // 4. allowPatterns – explicitly allowed
+    if (settings.allowPatterns?.length) {
+      for (const p of settings.allowPatterns) {
+        if (matchesToolPattern(p, call)) {
+          return { allowed: true, reason: 'pattern_allowed' };
+        }
+      }
+    }
+
+    // 5. allPathsAllowed – allow any file-path tool
+    const fileTools = new Set(['read_file', 'write_file', 'list_dir', 'delete_path', 'move_path', 'copy_path']);
+    if (settings.allPathsAllowed && fileTools.has(context.tool)) {
+      return { allowed: true, reason: 'all_paths_allowed' };
+    }
+
+    // 6. allUrlsAllowed – allow url tool
+    if (settings.allUrlsAllowed && context.tool === 'url') {
+      return { allowed: true, reason: 'all_urls_allowed' };
+    }
+
+    return null;
+  }
+
   /**
    * Check if context matches the immutable security blacklist
    * This check CANNOT be bypassed by any mode, whitelist, or user setting

src/permissions/toolPatterns.ts

@@ -0,0 +1,58 @@
+import { minimatch } from 'minimatch';
+
+export interface ToolPattern {
+  kind: string;
+  argument?: string;
+}
+
+export function parseToolPattern(pattern: string): ToolPattern {
+  const match = pattern.match(/^([^(]+?)\s*\(\s*(.+?)\s*\)\s*$/);
+  if (match) {
+    return { kind: match[1]!.trim(), argument: match[2]!.trim() };
+  }
+  return { kind: pattern.trim() };
+}
+
+export function parseToolPatternList(input: string): ToolPattern[] {
+  return input.split(',').map(s => parseToolPattern(s.trim())).filter(p => p.kind);
+}
+
+export function matchesToolPattern(
+  pattern: ToolPattern,
+  call: { kind: string; target: string },
+): boolean {
+  if (pattern.kind !== call.kind) return false;
+  if (!pattern.argument) return true;
+
+  const arg = pattern.argument;
+
+  // Stem wildcard: "git:*" matches "git push" (starts with "git ") but not "gitea"
+  if (arg.endsWith(':*')) {
+    const stem = arg.slice(0, -2);
+    return call.target === stem || call.target.startsWith(stem + ' ');
+  }
+
+  // URL domain matching
+  if (pattern.kind === 'url') {
+    try {
+      const url = new URL(call.target);
+      const domain = url.hostname;
+      if (arg.startsWith('*.')) {
+        return domain.endsWith(arg.slice(1));
+      }
+      return domain === arg || domain.endsWith('.' + arg);
+    } catch {
+      return call.target.includes(arg);
+    }
+  }
+
+  // Exact match first (fast path)
+  if (arg === call.target) return true;
+
+  // Glob matching for file patterns
+  if (arg.includes('*') || arg.includes('?')) {
+    return minimatch(call.target, arg);
+  }
+
+  return false;
+}

src/permissions/types.ts

@@ -2,6 +2,7 @@
  * Permission System Types
  * @license Apache-2.0
  */
+import type { ToolPattern } from './toolPatterns.js';
 
 export type PermissionMode = 'interactive' | 'unrestricted' | 'restricted' | 'external';
 
@@ -25,14 +26,28 @@ export interface PermissionSettings {
   rules?: PermissionRule[];
   /** Remember user decisions for this session */
   rememberSession?: boolean;
+  /** Patterns that are always denied (checked before allowPatterns) */
+  denyPatterns?: ToolPattern[];
+  /** Patterns that are always allowed (checked after denyPatterns) */
+  allowPatterns?: ToolPattern[];
+  /** If non-empty, only tools matching these patterns are allowed */
+  availableTools?: ToolPattern[];
+  /** Tools matching these patterns are always excluded/denied */
+  excludedTools?: ToolPattern[];
+  /** If true, all file-path tools are allowed without prompting */
+  allPathsAllowed?: boolean;
+  /** If true, all URL-fetching tools are allowed without prompting */
+  allUrlsAllowed?: boolean;
 }
 
 export interface PermissionDecision {
   allowed: boolean;
   reason:
     | 'whitelisted' | 'blacklisted' | 'rule_match' | 'user_approved' | 'user_denied'
     | 'mode_unrestricted' | 'mode_restricted' | 'default'
-    | 'external_approved' | 'external_denied' | 'external_error';
+    | 'external_approved' | 'external_denied' | 'external_error'
+    | 'pattern_denied' | 'pattern_allowed' | 'not_in_available' | 'excluded'
+    | 'all_paths_allowed' | 'all_urls_allowed';
   cached?: boolean;
 }