diff --git a/crates/pi-natives/src/ast.rs b/crates/pi-natives/src/ast.rs index c62212c7e..541b4ab32 100644 --- a/crates/pi-natives/src/ast.rs +++ b/crates/pi-natives/src/ast.rs @@ -128,6 +128,45 @@ pub struct AstFindResult { pub parse_errors: Option>, } +/// Options for `astMatch`: run ast-grep patterns against an in-memory source +/// string instead of files on disk. +#[napi(object)] +pub struct AstMatchOptions<'env> { + /// Source code to match against (parsed in memory, never read from disk). + pub source: String, + /// Language of `source` (required; e.g. "ts", "tsx", "rust", "python"). + pub lang: String, + /// ast-grep patterns to search for (OR across patterns). + pub patterns: Vec, + /// Rule selector for multi-rule ast-grep configurations. + pub selector: Option, + /// Pattern strictness; defaults to smart matching when omitted. + pub strictness: Option, + /// Maximum matches to return after `offset` (default applies when omitted). + pub limit: Option, + /// Number of leading matches to skip before applying `limit`. + pub offset: Option, + /// When true, include meta-variable bindings per match. + pub include_meta: Option, + /// Optional cancellation handle (library-specific). + pub signal: Option>, + /// Wall-clock timeout for the worker task in milliseconds. + pub timeout_ms: Option, +} + +/// Result of an in-memory `astMatch` run. +#[napi(object)] +pub struct AstMatchResult { + /// Page of matches after sort, offset, and limit. + pub matches: Vec, + /// Total matches found before paging (can exceed `matches.length`). + pub total_matches: u32, + /// True when results were truncated by `limit`. + pub limit_reached: bool, + /// Non-fatal parse or pattern-compile errors collected during the run. + pub parse_errors: Option>, +} + /// Options for `astEdit`: rewrite rules, scan scope, safety limits, and /// dry-run. #[napi(object)] @@ -688,6 +727,112 @@ pub fn ast_grep(options: AstFindOptions<'_>) -> task::Promise { }) } +/// Match ast-grep patterns against an in-memory source string; returns a +/// promise resolved on a worker thread. +/// +/// This is the file-free counterpart to [`ast_grep`]: callers that already hold +/// the source (streaming buffers, generated code, editor contents) avoid a +/// temp-file round trip. `lang` is required since there is no path to infer it +/// from. +#[napi] +pub fn ast_match(options: AstMatchOptions<'_>) -> task::Promise { + let AstMatchOptions { + source, + lang, + patterns, + selector, + strictness, + limit, + offset, + include_meta, + signal, + timeout_ms, + } = options; + + let ct = task::CancelToken::new(timeout_ms, signal); + let normalized_limit = limit.unwrap_or(DEFAULT_FIND_LIMIT).max(1); + let normalized_offset = offset.unwrap_or(0); + + task::blocking("ast_match", ct, move |ct| { + let patterns = normalize_pattern_list(Some(patterns))?; + let strictness = resolve_strictness(strictness); + let include_meta = include_meta.unwrap_or(false); + let lang_str = lang.trim(); + if lang_str.is_empty() { + return Err(Error::from_reason("`lang` is required for ast_match".to_string())); + } + let language = resolve_supported_lang(lang_str)?; + + let mut parse_errors = Vec::new(); + let mut compiled_patterns = Vec::with_capacity(patterns.len()); + for pattern in &patterns { + ct.heartbeat()?; + match compile_pattern(pattern, selector.as_deref(), &strictness, language) { + Ok(compiled) => compiled_patterns.push(compiled), + Err(err) => parse_errors.push(format!("{pattern}: {err}")), + } + } + + let mut all_matches = Vec::new(); + let mut total_matches = 0u32; + if !compiled_patterns.is_empty() { + let ast = language.ast_grep(&source); + if ast.root().dfs().any(|node| node.is_error()) { + parse_errors.push("parse error (syntax tree contains error nodes)".to_string()); + } + for pattern in &compiled_patterns { + ct.heartbeat()?; + for matched in ast.root().find_all(pattern.clone()) { + ct.heartbeat()?; + total_matches = total_matches.saturating_add(1); + let range = matched.range(); + let start = matched.start_pos(); + let end = matched.end_pos(); + let meta_variables = if include_meta { + Some(HashMap::::from(matched.get_env().clone())) + } else { + None + }; + all_matches.push(AstFindMatch { + path: String::new(), + text: matched.text().into_owned(), + byte_start: to_u32(range.start), + byte_end: to_u32(range.end), + start_line: to_u32(start.line().saturating_add(1)), + start_column: to_u32(start.column(matched.get_node()).saturating_add(1)), + end_line: to_u32(end.line().saturating_add(1)), + end_column: to_u32(end.column(matched.get_node()).saturating_add(1)), + meta_variables, + }); + } + } + } + + all_matches.sort_by(|left, right| { + left + .start_line + .cmp(&right.start_line) + .then(left.start_column.cmp(&right.start_column)) + .then(left.end_line.cmp(&right.end_line)) + .then(left.end_column.cmp(&right.end_column)) + .then(left.byte_start.cmp(&right.byte_start)) + .then(left.byte_end.cmp(&right.byte_end)) + }); + + let visible_matches = + all_matches.into_iter().skip(normalized_offset as usize).collect::>(); + let limit_reached = visible_matches.len() > normalized_limit as usize; + let matches = visible_matches.into_iter().take(normalized_limit as usize).collect::>(); + + Ok(AstMatchResult { + matches, + total_matches, + limit_reached, + parse_errors: (!parse_errors.is_empty()).then_some(parse_errors), + }) + }) +} + /// Apply ast-grep rewrite rules to matching files; honors `dryRun` and returns /// a promise. #[napi] diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 58880e0e3..5fe6b37bd 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,14 +1,16 @@ # Changelog ## [Unreleased] - ### Added +- Added `astCondition` to TTSR rule frontmatter as a syntax-aware alternative to regex `condition`, enabling AST-based matching for edit/write tool snapshots +- Added a built-in `ts-redundant-clear-guard` rule that flags redundant guards around `clearTimeout`, `clearInterval`, and `clearImmediate` calls - Added support for paste marker highlighting with accent styling (`[Paste #N, +X lines]`/`[Paste #N, Y chars]`) in the prompt editor, matching the visual treatment of image references - Added pixel dimensions to pasted/loaded image placeholders in the prompt — the marker now reads `[Image #N, WxH]` (falling back to `[Image #N]` when the header can't be decoded). ### Changed +- Changed TTSR rule bucketing and matching so rules with only `astCondition` are treated as TTSR rules and evaluated in the interrupt flow using reconstructed edit/write source snapshots - Normalized image content before it enters model context so attached images are downscaled and preprocessed for prompts, steering messages, follow-ups, and custom agent messages - Changed image marker format to include pixel dimensions when available (`[Image #N, WxH]`), falling back to bare `[Image #N]` when header cannot be decoded - Changed the prompt editor to highlight large-paste placeholders (`[Paste #N, +X lines]`/`[Paste #N, Y chars]`) with the same accent styling as image references (bold, no hyperlink), and to delete image/paste markers atomically: a single backspace or forward-delete removes the whole marker instead of leaving a broken `[Paste #N, +X lines` behind. @@ -28,7 +30,8 @@ - Fixed clipboard-pasted images being rejected when steering or following up during compaction. Instead of bailing with "Retry after it completes to send images", the message and its images are now queued via `queueCompactionMessage` and forwarded to the session (steer/follow-up/prompt) when the compaction queue flushes. - Fixed edit tool result previews to show only current-file lines and collapse long inserted blocks instead of echoing removed content. - Fixed `generateDiffString` to omit the mid-skip `...` placeholder between two nearby edits, conveying the elided gap via the jump in line numbers instead (consistent with how leading/trailing context skips already render). The placeholder row was indistinguishable from a genuine `...` context line and wasted a row in compact previews. -- Fixed the `ask` tool hanging when the model emitted two `ask` calls in one tool batch. The interactive selector is a single shared UI surface (`ExtensionUiController.showHookSelector` has no queue and overwrites `ctx.hookSelector` on each call), so concurrent asks clobbered each other: the second stole focus and orphaned the first, whose select promise hung until the user aborted the whole turn (surfacing a stray `Ask input was cancelled`). `ask` now declares `concurrency: "exclusive"`, so the agent loop serializes the batch and each question's selector runs to completion. +- Fixed concurrent interactive dialogs clobbering each other on the shared editor surface. `ExtensionUiController` presents the selector / input / editor modals by swapping a component into the single `editorContainer` and stealing focus, with no serialization — so a second `select`/`input`/`editor` request (from a hook, extension, the `ask` tool, or an internal flow) opened while one was already up would clear the container and re-focus, orphaning the first dialog. Its promise then hung until the caller's signal aborted (surfacing a stray `Ask input was cancelled` on top of the answered call). These modals are now serialized through `#presentDialog`: at most one shows at a time and the rest queue (FIFO); a queued request whose signal aborts before its turn resolves `undefined` and is never shown. The first dialog is still presented synchronously, so single-dialog timing is unchanged. +- Fixed the `ask` tool potentially hanging when the model emitted two `ask` calls in one tool batch. `ask` now declares `concurrency: "exclusive"`, so the agent loop serializes the batch and each question's selector runs to completion before the next starts, instead of racing for the shared selector surface. ## [15.10.4] - 2026-06-08 diff --git a/packages/coding-agent/src/capability/rule-buckets.ts b/packages/coding-agent/src/capability/rule-buckets.ts index 16afdc307..0ffd99dfe 100644 --- a/packages/coding-agent/src/capability/rule-buckets.ts +++ b/packages/coding-agent/src/capability/rule-buckets.ts @@ -6,7 +6,7 @@ * manager, and splits the rest into the always-apply and rulebook buckets. * * Bucket precedence (matches docs/rulebook-matching-pipeline.md §5): - * 1. TTSR — non-empty `condition` that `TtsrManager.addRule` accepts + * 1. TTSR — non-empty `condition`/`astCondition` that `TtsrManager.addRule` accepts * 2. always — `alwaysApply === true` * 3. rulebook — has a `description` */ @@ -49,7 +49,9 @@ export function bucketRules( if (disabled.has(rule.name)) continue; if (!includeBuiltin && rule._source?.provider === BUILTIN_DEFAULTS_PROVIDER_ID) continue; - const isTtsrRule = rule.condition && rule.condition.length > 0 ? ttsrManager.addRule(rule) : false; + const hasTtsrCondition = + (rule.condition && rule.condition.length > 0) || (rule.astCondition && rule.astCondition.length > 0); + const isTtsrRule = hasTtsrCondition ? ttsrManager.addRule(rule) : false; if (isTtsrRule) continue; if (rule.alwaysApply === true) { alwaysApplyRules.push(rule); diff --git a/packages/coding-agent/src/capability/rule.ts b/packages/coding-agent/src/capability/rule.ts index 0b5d8d2b3..cdce087d3 100644 --- a/packages/coding-agent/src/capability/rule.ts +++ b/packages/coding-agent/src/capability/rule.ts @@ -26,6 +26,8 @@ export interface RuleFrontmatter { alwaysApply?: boolean; /** New key for TTSR match conditions. */ condition?: string | string[]; + /** TTSR match condition(s) expressed as ast-grep patterns (edit/write streams only). */ + astCondition?: string | string[]; /** New key for TTSR stream scope. */ scope?: string | string[]; /** Per-rule TTSR interrupt mode override. */ @@ -51,6 +53,8 @@ export interface Rule { description?: string; /** Regex condition(s) that can trigger TTSR interruption. */ condition?: string[]; + /** ast-grep pattern condition(s) that can trigger TTSR interruption (edit/write streams only). */ + astCondition?: string[]; /** Optional stream scope tokens (for example: text, thinking, tool:edit(*.ts)). */ scope?: string[]; /** Per-rule TTSR interrupt mode override (falls back to global ttsr.interruptMode). */ @@ -188,10 +192,14 @@ function isLikelyFileGlob(value: string): boolean { * - legacy `ttsr_trigger` / `ttsrTrigger` are accepted as a `condition` fallback * - condition tokens that look like file globs become scope shorthands: * `*.rs` => `tool:edit(*.rs)`, `tool:write(*.rs)` and a catch-all condition `.*` + * - `astCondition` holds ast-grep patterns and is kept verbatim (no glob inference) */ -export function parseRuleConditionAndScope(frontmatter: RuleFrontmatter): Pick { +export function parseRuleConditionAndScope( + frontmatter: RuleFrontmatter, +): Pick { const rawCondition = frontmatter.condition ?? frontmatter.ttsr_trigger ?? frontmatter.ttsrTrigger; const parsedCondition = normalizeRuleField(rawCondition); + const astCondition = normalizeRuleField(frontmatter.astCondition); const parsedScope = normalizeScopeField(frontmatter.scope); const inferredScope: string[] = []; @@ -213,6 +221,7 @@ export function parseRuleConditionAndScope(frontmatter: RuleFrontmatter): Pick 0 ? Array.from(new Set(condition)) : undefined, + astCondition, scope: scope.length > 0 ? Array.from(new Set(scope)) : undefined, }; } diff --git a/packages/coding-agent/src/discovery/builtin-rules/index.ts b/packages/coding-agent/src/discovery/builtin-rules/index.ts index 3f0ce1c77..6188c36dc 100644 --- a/packages/coding-agent/src/discovery/builtin-rules/index.ts +++ b/packages/coding-agent/src/discovery/builtin-rules/index.ts @@ -22,6 +22,7 @@ import tsNoDynamicImport from "./ts-no-dynamic-import.md" with { type: "text" }; import tsNoReturnType from "./ts-no-return-type.md" with { type: "text" }; import tsNoTinyFunctions from "./ts-no-tiny-functions.md" with { type: "text" }; import tsPromiseWithResolvers from "./ts-promise-with-resolvers.md" with { type: "text" }; +import tsRedundantClearGuard from "./ts-redundant-clear-guard.md" with { type: "text" }; import tsSetMap from "./ts-set-map.md" with { type: "text" }; /** A bundled rule's stable name and raw markdown (frontmatter + body). */ @@ -46,5 +47,6 @@ export const BUILTIN_RULE_SOURCES: readonly BuiltinRuleSource[] = [ { name: "ts-no-return-type", content: tsNoReturnType }, { name: "ts-no-tiny-functions", content: tsNoTinyFunctions }, { name: "ts-promise-with-resolvers", content: tsPromiseWithResolvers }, + { name: "ts-redundant-clear-guard", content: tsRedundantClearGuard }, { name: "ts-set-map", content: tsSetMap }, ]; diff --git a/packages/coding-agent/src/discovery/builtin-rules/ts-redundant-clear-guard.md b/packages/coding-agent/src/discovery/builtin-rules/ts-redundant-clear-guard.md new file mode 100644 index 000000000..4c4a7fec7 --- /dev/null +++ b/packages/coding-agent/src/discovery/builtin-rules/ts-redundant-clear-guard.md @@ -0,0 +1,75 @@ +--- +description: Do not guard clearTimeout/clearInterval/clearImmediate with a truthiness or null/undefined check — they accept null and undefined +scope: "tool:edit(*.{ts,tsx,js,jsx,mts,cts,mjs,cjs}), tool:write(*.{ts,tsx,js,jsx,mts,cts,mjs,cjs})" +interruptMode: never +astCondition: + - "if ($X) clearTimeout($X)" + - "if ($X) { clearTimeout($X) }" + - "if ($X) clearInterval($X)" + - "if ($X) { clearInterval($X) }" + - "if ($X) clearImmediate($X)" + - "if ($X) { clearImmediate($X) }" + - "if ($X !== null) clearTimeout($X)" + - "if ($X !== null) { clearTimeout($X) }" + - "if ($X !== null) clearInterval($X)" + - "if ($X !== null) { clearInterval($X) }" + - "if ($X !== null) clearImmediate($X)" + - "if ($X !== null) { clearImmediate($X) }" + - "if ($X != null) clearTimeout($X)" + - "if ($X != null) { clearTimeout($X) }" + - "if ($X != null) clearInterval($X)" + - "if ($X != null) { clearInterval($X) }" + - "if ($X != null) clearImmediate($X)" + - "if ($X != null) { clearImmediate($X) }" + - "if ($X !== undefined) clearTimeout($X)" + - "if ($X !== undefined) { clearTimeout($X) }" + - "if ($X !== undefined) clearInterval($X)" + - "if ($X !== undefined) { clearInterval($X) }" + - "if ($X !== undefined) clearImmediate($X)" + - "if ($X !== undefined) { clearImmediate($X) }" + - "if ($X != undefined) clearTimeout($X)" + - "if ($X != undefined) { clearTimeout($X) }" + - "if ($X != undefined) clearInterval($X)" + - "if ($X != undefined) { clearInterval($X) }" + - "if ($X != undefined) clearImmediate($X)" + - "if ($X != undefined) { clearImmediate($X) }" +--- + +**Do not guard `clearTimeout` / `clearInterval` / `clearImmediate` with a truthiness or `null`/`undefined` check.** Per the WHATWG/Node timers spec these functions are no-ops when handed `null`, `undefined`, or any value that doesn't correspond to a live timer. The guard adds a redundant branch that the reader must still reason about. + +## Why it's wrong + +- The branch can never change behavior — clearing a missing/`null`/`undefined` handle does nothing. +- Extra branches inflate the code and hide the one line that matters. +- It signals a misunderstanding of the timer API to future readers. + +## Avoid + +```ts +if (this.timer) clearTimeout(this.timer); +if (handle !== null) clearInterval(handle); +if (id != undefined) { + clearImmediate(id); +} +``` + +## Use + +```ts +clearTimeout(this.timer); +clearInterval(handle); +clearImmediate(id); +``` + +## When a guard *is* warranted + +Keep the check only when the body does more than clear — e.g. it also reassigns the handle or runs other cleanup: + +```ts +if (this.timer) { + clearTimeout(this.timer); + this.timer = undefined; // extra work → guard is not purely redundant +} +``` + +This rule only fires when the clear call is the sole statement in the guarded branch, so those legitimate cases are left alone. diff --git a/packages/coding-agent/src/discovery/helpers.ts b/packages/coding-agent/src/discovery/helpers.ts index ac6e92ad0..830c1d717 100644 --- a/packages/coding-agent/src/discovery/helpers.ts +++ b/packages/coding-agent/src/discovery/helpers.ts @@ -163,7 +163,7 @@ export function buildRuleFromMarkdown( }, ): Rule { const { frontmatter, body } = parseFrontmatter(content, { source: filePath }); - const { condition, scope } = parseRuleConditionAndScope(frontmatter as RuleFrontmatter); + const { condition, astCondition, scope } = parseRuleConditionAndScope(frontmatter as RuleFrontmatter); let globs: string[] | undefined; if (Array.isArray(frontmatter.globs)) { @@ -186,6 +186,7 @@ export function buildRuleFromMarkdown( alwaysApply: frontmatter.alwaysApply === true, description: typeof frontmatter.description === "string" ? frontmatter.description : undefined, condition, + astCondition, scope, interruptMode, _source: source, diff --git a/packages/coding-agent/src/export/ttsr.ts b/packages/coding-agent/src/export/ttsr.ts index 8dba7a399..7655df200 100644 --- a/packages/coding-agent/src/export/ttsr.ts +++ b/packages/coding-agent/src/export/ttsr.ts @@ -5,6 +5,8 @@ * the agent's output. When a match occurs, the stream is aborted, the rule is * injected as a system reminder, and the request is retried. */ +import * as path from "node:path"; +import { AstMatchStrictness, astMatch } from "@oh-my-pi/pi-natives"; import { logger } from "@oh-my-pi/pi-utils"; import type { Rule } from "../capability/rule"; import type { TtsrSettings } from "../config/settings"; @@ -38,6 +40,8 @@ interface TtsrScope { interface TtsrEntry { rule: Rule; conditions: RegExp[]; + /** ast-grep pattern strings; matched only against edit/write tool snapshots. */ + astConditions: string[]; scope: TtsrScope; globalPathGlobs?: Bun.Glob[]; } @@ -70,6 +74,8 @@ export class TtsrManager { readonly #rules = new Map(); readonly #injectionRecords = new Map(); readonly #buffers = new Map(); + /** Last snapshot evaluated for AST conditions, keyed by stream key, to dedupe matcher runs. */ + readonly #lastAstSnapshots = new Map(); #messageCount = 0; constructor(settings?: TtsrSettings) { @@ -302,7 +308,8 @@ export class TtsrManager { } const conditions = this.#compileConditions(rule); - if (conditions.length === 0) { + const astConditions = (rule.astCondition ?? []).map(pattern => pattern.trim()).filter(p => p.length > 0); + if (conditions.length === 0 && astConditions.length === 0) { return false; } @@ -318,6 +325,7 @@ export class TtsrManager { this.#rules.set(rule.name, { rule, conditions, + astConditions, scope, globalPathGlobs, }); @@ -325,6 +333,7 @@ export class TtsrManager { logger.debug("TTSR rule registered", { ruleName: rule.name, conditions: rule.condition, + astConditions: rule.astCondition, scope: rule.scope, globs: rule.globs, }); @@ -359,6 +368,112 @@ export class TtsrManager { return this.#matchBuffer(snapshot, context); } + /** Derive an ast-grep language alias from candidate paths (bare extension, e.g. "ts"), if any. */ + #deriveLang(filePaths: string[] | undefined): string | undefined { + for (const filePath of filePaths ?? []) { + const ext = path.extname(this.#normalizePath(filePath)); + if (ext.length > 1) { + return ext.slice(1).toLowerCase(); + } + } + return undefined; + } + + /** + * Evaluate ast-grep `astCondition` rules against a reconstructed tool snapshot. + * + * Only edit/write tool streams reach here (AST conditions need a language, which + * we infer from the file extension on the tool's path argument). The snapshot is + * matched in memory by the native engine (`astMatch`), so this is async and + * intentionally throttled: identical consecutive snapshots (the common case when + * only non-source arguments change between deltas) are skipped. + */ + async checkAstSnapshot(snapshot: string, context: TtsrMatchContext): Promise { + if (!this.#settings.enabled || context.source !== "tool") { + return []; + } + + const lang = this.#deriveLang(context.filePaths); + if (!lang) { + return []; + } + + const candidates: TtsrEntry[] = []; + for (const [name, entry] of this.#rules) { + if (entry.astConditions.length === 0) { + continue; + } + if ( + !this.#canTrigger(name) || + !this.#matchesScope(entry, context) || + !this.#matchesGlobalPaths(entry, context) + ) { + continue; + } + candidates.push(entry); + } + if (candidates.length === 0) { + return []; + } + + // Throttle: skip re-running the matcher when the source content is unchanged. + const bufferKey = this.#bufferKey(context); + if (this.#lastAstSnapshots.get(bufferKey) === snapshot) { + return []; + } + this.#lastAstSnapshots.set(bufferKey, snapshot); + + const matches: Rule[] = []; + for (const entry of candidates) { + if (await this.#astConditionsMatch(entry.astConditions, snapshot, lang)) { + matches.push(entry.rule); + logger.debug("TTSR ast condition matched", { + ruleName: entry.rule.name, + astConditions: entry.rule.astCondition, + toolName: context.toolName, + filePaths: context.filePaths, + }); + } + } + return matches; + } + + async #astConditionsMatch(patterns: string[], source: string, lang: string): Promise { + try { + const result = await astMatch({ + patterns, + source, + lang, + strictness: AstMatchStrictness.Smart, + limit: 1, + }); + if (result.parseErrors && result.parseErrors.length > 0) { + logger.debug("TTSR ast match reported parse errors", { parseErrors: result.parseErrors }); + } + return result.totalMatches > 0; + } catch (error) { + logger.warn("TTSR ast match failed, treating as no match", { + patterns, + lang, + error: error instanceof Error ? error.message : String(error), + }); + return false; + } + } + + /** True when any registered rule carries ast-grep conditions. */ + hasAstRules(): boolean { + if (!this.#settings.enabled) { + return false; + } + for (const entry of this.#rules.values()) { + if (entry.astConditions.length > 0) { + return true; + } + } + return false; + } + #matchBuffer(buffer: string, context: TtsrMatchContext): Rule[] { if (!this.#settings.enabled) { return []; @@ -435,6 +550,7 @@ export class TtsrManager { /** Reset stream buffers (called on new turn). */ resetBuffer(): void { this.#buffers.clear(); + this.#lastAstSnapshots.clear(); } /** Check if any TTSR rules are registered. */ diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index f4ea08104..b3c5d630f 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -1674,89 +1674,18 @@ export class AgentSession { } if (matchContext && "delta" in assistantEvent) { + const targetMessageTimestamp = event.message.role === "assistant" ? event.message.timestamp : undefined; const matches = this.#checkTtsrStream(assistantEvent.delta, matchContext, streamingToolCall); - if (matches.length > 0) { - // Decide first: a non-interrupting tool-source match attaches to the - // specific tool call's result instead of driving a loop-wide follow-up. - const shouldInterrupt = this.#shouldInterruptForTtsrMatch(matches, matchContext); - const perToolId = shouldInterrupt ? undefined : this.#extractTtsrToolCallId(matchContext); - if (perToolId) { - this.#addPerToolTtsrInjections(perToolId, matches); - this.#emitSessionEvent({ type: "ttsr_triggered", rules: matches }).catch(() => {}); - } else { - // Queue rules for injection; mark as injected only after successful enqueue. - this.#addPendingTtsrInjections(matches); - - if (shouldInterrupt) { - // Abort the stream immediately — do not gate on extension callbacks - this.#ttsrAbortPending = true; - this.#ensureTtsrResumePromise(); - this.agent.abort(this.#formatTtsrAbortReason(matches)); - // Notify extensions (fire-and-forget, does not block abort) - this.#emitSessionEvent({ type: "ttsr_triggered", rules: matches }).catch(() => {}); - // Schedule retry after a short delay - const retryToken = ++this.#ttsrRetryToken; - const generation = this.#promptGeneration; - const targetMessageTimestamp = - event.message.role === "assistant" ? event.message.timestamp : undefined; - this.#schedulePostPromptTask( - async () => { - if (this.#ttsrRetryToken !== retryToken) { - this.#resolveTtsrResume(); - return; - } - - const targetAssistantIndex = this.#findTtsrAssistantIndex(targetMessageTimestamp); - if ( - !this.#ttsrAbortPending || - this.#promptGeneration !== generation || - targetAssistantIndex === -1 - ) { - this.#ttsrAbortPending = false; - this.#pendingTtsrInjections = []; - this.#perToolTtsrInjections.clear(); - this.#resolveTtsrResume(); - return; - } - this.#ttsrAbortPending = false; - this.#perToolTtsrInjections.clear(); - const ttsrSettings = this.#ttsrManager?.getSettings(); - if (ttsrSettings?.contextMode === "discard") { - // Remove the partial/aborted assistant turn from agent state - this.agent.replaceMessages(this.agent.state.messages.slice(0, targetAssistantIndex)); - } - // Inject TTSR rules as system reminder before retry - const injection = this.#getTtsrInjectionContent(); - if (injection) { - const details = { rules: injection.rules.map(rule => rule.name) }; - this.agent.appendMessage({ - role: "custom", - customType: "ttsr-injection", - content: injection.content, - display: false, - details, - attribution: "agent", - timestamp: Date.now(), - }); - this.sessionManager.appendCustomMessageEntry( - "ttsr-injection", - injection.content, - false, - details, - "agent", - ); - this.#markTtsrInjected(details.rules); - } - try { - await this.agent.continue(); - } catch { - this.#resolveTtsrResume(); - } - }, - { delayMs: 50 }, - ); - return; - } + if (matches.length > 0 && this.#handleTtsrMatches(matches, matchContext, targetMessageTimestamp)) { + return; + } + // ast-grep `astCondition` rules match against the reconstructed edit/write + // snapshot, which only exists for tool argument streams. The native worker + // call is async, so this path is awaited and self-throttled by the manager. + if (matchContext.source === "tool" && this.#ttsrManager?.hasAstRules()) { + const astMatches = await this.#checkTtsrAstStream(matchContext, streamingToolCall); + if (astMatches.length > 0 && this.#handleTtsrMatches(astMatches, matchContext, targetMessageTimestamp)) { + return; } } } @@ -2428,19 +2357,134 @@ export class AgentSession { if (!manager) { return []; } - if (toolCall) { - const tools = this.agent.state.tools; - const tool = - tools.find(t => t.name === toolCall.name) ?? - tools.find(t => t.customWireName !== undefined && t.customWireName === toolCall.name); - const digest = tool?.matcherDigest?.(toolCall.arguments ?? {}); - if (digest !== undefined) { - return manager.checkSnapshot(digest, matchContext); - } + const digest = this.#resolveTtsrMatcherDigest(toolCall); + if (digest !== undefined) { + return manager.checkSnapshot(digest, matchContext); } return manager.checkDelta(delta, matchContext); } + /** Reconstruct the tool's normalized source snapshot via its `matcherDigest`, if any. */ + #resolveTtsrMatcherDigest(toolCall: ToolCall | undefined): string | undefined { + if (!toolCall) { + return undefined; + } + const tools = this.agent.state.tools; + const tool = + tools.find(t => t.name === toolCall.name) ?? + tools.find(t => t.customWireName !== undefined && t.customWireName === toolCall.name); + return tool?.matcherDigest?.(toolCall.arguments ?? {}); + } + + /** + * Match ast-grep `astCondition` rules against the reconstructed tool snapshot. + * + * Only edit/write tool streams expose a `matcherDigest`, which is the real source + * the call introduces; AST matching needs that (and a language inferred from the + * path argument), so non-digest streams never produce AST matches. + */ + async #checkTtsrAstStream(matchContext: TtsrMatchContext, toolCall: ToolCall | undefined): Promise { + const manager = this.#ttsrManager; + if (!manager) { + return []; + } + const digest = this.#resolveTtsrMatcherDigest(toolCall); + if (digest === undefined) { + return []; + } + return manager.checkAstSnapshot(digest, matchContext); + } + + /** + * Route TTSR matches to either a per-tool injection or a stream-interrupting + * retry. Returns true when the stream was aborted and the caller should stop + * processing this event. + */ + #handleTtsrMatches( + matches: Rule[], + matchContext: TtsrMatchContext, + targetMessageTimestamp: number | undefined, + ): boolean { + // Decide first: a non-interrupting tool-source match attaches to the + // specific tool call's result instead of driving a loop-wide follow-up. + const shouldInterrupt = this.#shouldInterruptForTtsrMatch(matches, matchContext); + const perToolId = shouldInterrupt ? undefined : this.#extractTtsrToolCallId(matchContext); + if (perToolId) { + this.#addPerToolTtsrInjections(perToolId, matches); + this.#emitSessionEvent({ type: "ttsr_triggered", rules: matches }).catch(() => {}); + return false; + } + + // Queue rules for injection; mark as injected only after successful enqueue. + this.#addPendingTtsrInjections(matches); + if (!shouldInterrupt) { + return false; + } + + // Abort the stream immediately — do not gate on extension callbacks + this.#ttsrAbortPending = true; + this.#ensureTtsrResumePromise(); + this.agent.abort(this.#formatTtsrAbortReason(matches)); + // Notify extensions (fire-and-forget, does not block abort) + this.#emitSessionEvent({ type: "ttsr_triggered", rules: matches }).catch(() => {}); + // Schedule retry after a short delay + const retryToken = ++this.#ttsrRetryToken; + const generation = this.#promptGeneration; + this.#schedulePostPromptTask( + async () => { + if (this.#ttsrRetryToken !== retryToken) { + this.#resolveTtsrResume(); + return; + } + + const targetAssistantIndex = this.#findTtsrAssistantIndex(targetMessageTimestamp); + if (!this.#ttsrAbortPending || this.#promptGeneration !== generation || targetAssistantIndex === -1) { + this.#ttsrAbortPending = false; + this.#pendingTtsrInjections = []; + this.#perToolTtsrInjections.clear(); + this.#resolveTtsrResume(); + return; + } + this.#ttsrAbortPending = false; + this.#perToolTtsrInjections.clear(); + const ttsrSettings = this.#ttsrManager?.getSettings(); + if (ttsrSettings?.contextMode === "discard") { + // Remove the partial/aborted assistant turn from agent state + this.agent.replaceMessages(this.agent.state.messages.slice(0, targetAssistantIndex)); + } + // Inject TTSR rules as system reminder before retry + const injection = this.#getTtsrInjectionContent(); + if (injection) { + const details = { rules: injection.rules.map(rule => rule.name) }; + this.agent.appendMessage({ + role: "custom", + customType: "ttsr-injection", + content: injection.content, + display: false, + details, + attribution: "agent", + timestamp: Date.now(), + }); + this.sessionManager.appendCustomMessageEntry( + "ttsr-injection", + injection.content, + false, + details, + "agent", + ); + this.#markTtsrInjected(details.rules); + } + try { + await this.agent.continue(); + } catch { + this.#resolveTtsrResume(); + } + }, + { delayMs: 50 }, + ); + return true; + } + /** Extract path-like arguments from tool call payload for TTSR glob matching. */ #extractTtsrFilePathsFromArgs(args: unknown): string[] | undefined { if (!args || typeof args !== "object" || Array.isArray(args)) { @@ -4579,7 +4623,8 @@ export class AgentSession { content: msg.content, display: msg.display, details: msg.details, - attribution: msg.attribution ?? promptAttribution ?? (message.role === "user" ? "user" : "agent"), + attribution: + msg.attribution ?? promptAttribution ?? (message.role === "user" ? "user" : "agent"), timestamp: Date.now(), }), ); diff --git a/packages/coding-agent/test/capability/rule-buckets.test.ts b/packages/coding-agent/test/capability/rule-buckets.test.ts index c2b1a6a6c..dfcd6d31a 100644 --- a/packages/coding-agent/test/capability/rule-buckets.test.ts +++ b/packages/coding-agent/test/capability/rule-buckets.test.ts @@ -16,6 +16,7 @@ function makeRule(partial: Partial): Rule { alwaysApply: partial.alwaysApply, description: partial.description, condition: partial.condition, + astCondition: partial.astCondition, scope: partial.scope, interruptMode: partial.interruptMode, _source: partial._source ?? source("native"), @@ -34,6 +35,18 @@ describe("bucketRules", () => { expect(mgr.checkDelta("contains FORBIDDEN token", { source: "text" }).map(r => r.name)).toEqual(["no-foo"]); }); + it("registers an ast-only rule as TTSR and excludes it from rulebook/always buckets", () => { + const mgr = new TtsrManager(); + const ttsr = makeRule({ name: "no-console", astCondition: ["console.log($A)"], description: "blocks console" }); + + const { rulebookRules, alwaysApplyRules } = bucketRules([ttsr], mgr); + + expect(rulebookRules).toHaveLength(0); + expect(alwaysApplyRules).toHaveLength(0); + expect(mgr.hasRules()).toBe(true); + expect(mgr.hasAstRules()).toBe(true); + }); + it("splits non-TTSR rules into always-apply and rulebook by metadata", () => { const mgr = new TtsrManager(); const sticky = makeRule({ name: "sticky", alwaysApply: true, description: "sticky desc" }); diff --git a/packages/coding-agent/test/discovery/builtin-defaults.test.ts b/packages/coding-agent/test/discovery/builtin-defaults.test.ts index 9a36ca360..6fe44b039 100644 --- a/packages/coding-agent/test/discovery/builtin-defaults.test.ts +++ b/packages/coding-agent/test/discovery/builtin-defaults.test.ts @@ -26,6 +26,7 @@ const EXPECTED_RULE_NAMES = [ "ts-no-return-type", "ts-no-tiny-functions", "ts-promise-with-resolvers", + "ts-redundant-clear-guard", "ts-set-map", ].sort(); @@ -52,14 +53,22 @@ describe("builtin-defaults rule provider", () => { expect(rules.every(r => r._source.provider === BUILTIN_DEFAULTS_PROVIDER_ID)).toBe(true); }); - it("parses every bundled rule as a TTSR rule (non-empty condition and scope)", async () => { + it("parses every bundled rule as a TTSR rule (non-empty condition/astCondition and scope)", async () => { const rules = await loadBuiltinRules(); for (const rule of rules) { - expect(rule.condition?.length, `${rule.name} condition`).toBeGreaterThan(0); + const conditionCount = (rule.condition?.length ?? 0) + (rule.astCondition?.length ?? 0); + expect(conditionCount, `${rule.name} condition/astCondition`).toBeGreaterThan(0); expect(rule.scope?.length, `${rule.name} scope`).toBeGreaterThan(0); } }); + it("bundles ast-grep conditions for the redundant-clear-guard rule", async () => { + const rules = await loadBuiltinRules(); + const rule = rules.find(r => r.name === "ts-redundant-clear-guard"); + expect(rule?.condition).toBeUndefined(); + expect(rule?.astCondition?.length).toBeGreaterThan(0); + }); + it("parses YAML list-form conditions from the embedded text", async () => { const rules = await loadBuiltinRules(); const lazylock = rules.find(r => r.name === "rs-lazylock"); diff --git a/packages/coding-agent/test/ttsr.test.ts b/packages/coding-agent/test/ttsr.test.ts index 4f04ad3e0..96d9adca9 100644 --- a/packages/coding-agent/test/ttsr.test.ts +++ b/packages/coding-agent/test/ttsr.test.ts @@ -25,6 +25,7 @@ function makeRule(partial: Partial): Rule { alwaysApply: partial.alwaysApply, description: partial.description, condition: partial.condition, + astCondition: partial.astCondition, scope: partial.scope, _source: partial._source ?? { provider: "test", @@ -100,6 +101,26 @@ describe("parseRuleConditionAndScope", () => { expect(parsed.condition).toEqual([".*"]); expect(parsed.scope).toEqual(["tool:edit(*.rs)", "tool:write(*.rs)"]); }); + + it("normalizes astCondition strings and arrays without glob inference", () => { + expect(parseRuleConditionAndScope({ astCondition: "console.log($A)" }).astCondition).toEqual(["console.log($A)"]); + const parsed = parseRuleConditionAndScope({ + astCondition: ["console.log($A)", "debugger"], + }); + expect(parsed.astCondition).toEqual(["console.log($A)", "debugger"]); + // AST patterns never drive scope inference, and absent regex stays absent. + expect(parsed.condition).toBeUndefined(); + expect(parsed.scope).toBeUndefined(); + }); + + it("carries astCondition alongside a regex condition", () => { + const parsed = parseRuleConditionAndScope({ + condition: "TODO", + astCondition: "console.log($A)", + }); + expect(parsed.condition).toEqual(["TODO"]); + expect(parsed.astCondition).toEqual(["console.log($A)"]); + }); }); describe("TtsrManager scope matching", () => { @@ -394,6 +415,95 @@ describe("TtsrManager snapshot matching", () => { }); }); +describe("TtsrManager ast condition matching", () => { + const editContext = { + source: "tool" as const, + toolName: "edit", + filePaths: ["src/main.ts"], + streamKey: "toolcall:ast-1", + }; + + it("registers and reports ast-only rules without a regex condition", () => { + const manager = new TtsrManager(); + const rule = makeRule({ name: "no-console", astCondition: ["console.log($A)"] }); + + expect(manager.addRule(rule)).toBe(true); + expect(manager.hasRules()).toBe(true); + expect(manager.hasAstRules()).toBe(true); + }); + + it("does not report ast rules when only regex conditions are registered", () => { + const manager = new TtsrManager(); + manager.addRule(makeRule({ name: "regex-only", condition: ["TODO"], scope: ["text"] })); + expect(manager.hasAstRules()).toBe(false); + }); + + it("matches an ast pattern against a reconstructed source snapshot", async () => { + const manager = new TtsrManager(); + const rule = makeRule({ + name: "no-console", + astCondition: ["console.log($A)"], + scope: ["tool:edit(*.ts)"], + }); + manager.addRule(rule); + + const matches = await manager.checkAstSnapshot('function greet() {\n\tconsole.log("hi");\n}', editContext); + expect(matches).toEqual([rule]); + }); + + it("does not match when the ast pattern is absent", async () => { + const manager = new TtsrManager(); + manager.addRule(makeRule({ name: "no-console", astCondition: ["console.log($A)"] })); + + const matches = await manager.checkAstSnapshot("function greet() {\n\treturn 1;\n}", editContext); + expect(matches).toEqual([]); + }); + + it("infers language from the file extension and isolates other languages", async () => { + const manager = new TtsrManager(); + manager.addRule(makeRule({ name: "no-console", astCondition: ["console.log($A)"], scope: ["tool:edit(*.ts)"] })); + + // A `.rs` path is out of the rule's tool scope, so the TS pattern never runs. + const rustMatches = await manager.checkAstSnapshot('println!("{}", x);', { + ...editContext, + filePaths: ["src/main.rs"], + streamKey: "toolcall:ast-rs", + }); + expect(rustMatches).toEqual([]); + }); + + it("skips ast evaluation when no file path is available to infer a language", async () => { + const manager = new TtsrManager(); + manager.addRule(makeRule({ name: "no-console", astCondition: ["console.log($A)"] })); + + const matches = await manager.checkAstSnapshot('console.log("hi");', { + source: "tool", + toolName: "edit", + streamKey: "toolcall:ast-nopath", + }); + expect(matches).toEqual([]); + }); + + it("evaluates ast conditions only once for an unchanged snapshot", async () => { + const manager = new TtsrManager(); + const rule = makeRule({ name: "no-console", astCondition: ["console.log($A)"] }); + manager.addRule(rule); + const snapshot = 'console.log("hi");'; + + // First evaluation matches; the throttle returns nothing for the identical re-check. + expect(await manager.checkAstSnapshot(snapshot, editContext)).toEqual([rule]); + expect(await manager.checkAstSnapshot(snapshot, editContext)).toEqual([]); + }); + + it("returns no ast matches when ttsr is disabled", async () => { + const manager = ttsrManager({ enabled: false }); + manager.addRule(makeRule({ name: "no-console", astCondition: ["console.log($A)"] })); + + expect(manager.hasAstRules()).toBe(false); + expect(await manager.checkAstSnapshot('console.log("hi");', editContext)).toEqual([]); + }); +}); + describe("TtsrManager repeat behavior", () => { const turnContext = { source: "text" as const }; diff --git a/packages/natives/native/index.d.ts b/packages/natives/native/index.d.ts index e462e5a0f..1d652b8c2 100644 --- a/packages/natives/native/index.d.ts +++ b/packages/natives/native/index.d.ts @@ -228,6 +228,56 @@ export interface AstFindResult { */ export declare function astGrep(options: AstFindOptions): Promise +/** + * Match ast-grep patterns against an in-memory source string; returns a + * promise resolved on a worker thread. + * + * This is the file-free counterpart to [`ast_grep`]: callers that already hold + * the source (streaming buffers, generated code, editor contents) avoid a + * temp-file round trip. `lang` is required since there is no path to infer it + * from. + */ +export declare function astMatch(options: AstMatchOptions): Promise + +/** + * Options for `astMatch`: run ast-grep patterns against an in-memory source + * string instead of files on disk. + */ +export interface AstMatchOptions { + /** Source code to match against (parsed in memory, never read from disk). */ + source: string + /** Language of `source` (required; e.g. "ts", "tsx", "rust", "python"). */ + lang: string + /** ast-grep patterns to search for (OR across patterns). */ + patterns: Array + /** Rule selector for multi-rule ast-grep configurations. */ + selector?: string + /** Pattern strictness; defaults to smart matching when omitted. */ + strictness?: AstMatchStrictness + /** Maximum matches to return after `offset` (default applies when omitted). */ + limit?: number + /** Number of leading matches to skip before applying `limit`. */ + offset?: number + /** When true, include meta-variable bindings per match. */ + includeMeta?: boolean + /** Optional cancellation handle (library-specific). */ + signal?: unknown + /** Wall-clock timeout for the worker task in milliseconds. */ + timeoutMs?: number +} + +/** Result of an in-memory `astMatch` run. */ +export interface AstMatchResult { + /** Page of matches after sort, offset, and limit. */ + matches: Array + /** Total matches found before paging (can exceed `matches.length`). */ + totalMatches: number + /** True when results were truncated by `limit`. */ + limitReached: boolean + /** Non-fatal parse or pattern-compile errors collected during the run. */ + parseErrors?: Array +} + /** ast-grep pattern strictness (controls how patterns match syntax). */ export declare enum AstMatchStrictness { /** Match at the concrete syntax tree level. */ diff --git a/packages/natives/native/index.js b/packages/natives/native/index.js index a694fcdbc..18a5d4b3d 100644 --- a/packages/natives/native/index.js +++ b/packages/natives/native/index.js @@ -27,6 +27,7 @@ export const __piNativesV15_10_4 = nativeBindings.__piNativesV15_10_4; export const applyBashFixups = nativeBindings.applyBashFixups; export const astEdit = nativeBindings.astEdit; export const astGrep = nativeBindings.astGrep; +export const astMatch = nativeBindings.astMatch; export const blockRangeAt = nativeBindings.blockRangeAt; export const copyToClipboard = nativeBindings.copyToClipboard; export const countTokens = nativeBindings.countTokens;