From 86824c94e96ed9961db1240acca11820529c9343 Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 14 Jul 2026 19:01:47 +0000 Subject: [PATCH 1/3] fix(ttsr): registered rules with inline regex flags and malformed scope Rules whose condition led with a PCRE-style inline flag group (e.g. `(?i)`) never registered: `new RegExp("(?i)...")` throws in Bun/JS, so the condition failed to compile and `TtsrManager.addRule` dropped the rule as having zero usable conditions. - Add `compileRuleCondition` in capability/rule.ts translating a leading `(?i)`/`(?m)`/`(?s)` group into native RegExp flags; wire it into the TtsrManager and both ttsr-cli compile sites. - Strip surrounding quotes from scope tokens so a malformed `scope: "text","thinking"` recovers to canonical `text`/`thinking`. - Reparse each value in parseFrontmatter's YAML fallback so one bad line can't leave sibling values wrapped in literal quotes. Fixes #4796 --- packages/coding-agent/CHANGELOG.md | 4 ++ packages/coding-agent/src/capability/rule.ts | 39 ++++++++++- packages/coding-agent/src/cli/ttsr-cli.ts | 6 +- packages/coding-agent/src/export/ttsr.ts | 4 +- .../test/ttsr-inline-flags-scope.test.ts | 66 +++++++++++++++++++ packages/utils/CHANGELOG.md | 4 ++ packages/utils/src/frontmatter.ts | 19 +++++- packages/utils/test/frontmatter.test.ts | 22 +++++++ 8 files changed, 155 insertions(+), 9 deletions(-) create mode 100644 packages/coding-agent/test/ttsr-inline-flags-scope.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3a03a1977..c4621e80b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -6,6 +6,10 @@ - Memoized non-message token totals (system prompt, tool schemas, skills) so the per-turn compaction and context-threshold paths recompute them at most once per input change instead of on every call. `getContextBreakdown` and `#estimateStoredContextTokens` previously re-tokenized the system prompt and every tool's wire schema (per-tool `JSON.stringify`) several times per turn over inputs that change at most once per turn. +### Fixed + +- Fixed TTSR rules with a leading `(?i)`/`(?m)`/`(?s)` inline regex flag never registering: `new RegExp("(?i)...")` throws in Bun/JS, so the condition failed to compile and the rule was silently dropped. Leading inline flag groups are now translated to native `RegExp` flags. Also recover `scope` tokens and sibling values from malformed frontmatter (e.g. `scope: "text","thinking"`), which previously left literal quotes on the parsed values ([#4796](https://github.com/can1357/oh-my-pi/issues/4796)). + ## [16.3.11] - 2026-07-06 ### Changed diff --git a/packages/coding-agent/src/capability/rule.ts b/packages/coding-agent/src/capability/rule.ts index cdce087d3..831b8468d 100644 --- a/packages/coding-agent/src/capability/rule.ts +++ b/packages/coding-agent/src/capability/rule.ts @@ -158,7 +158,18 @@ function normalizeScopeField(value: unknown): string[] | undefined { return undefined; } - const tokens = normalized.flatMap(splitScopeTokens).filter(item => item.length > 0); + const tokens = normalized + .flatMap(splitScopeTokens) + .map(token => { + // Tolerate malformed frontmatter (e.g. `scope: "text","thinking"`) whose + // YAML-fallback parse leaves per-token quotes intact (issue #4796). + const quote = token[0]; + if (token.length >= 2 && (quote === '"' || quote === "'") && token[token.length - 1] === quote) { + return token.slice(1, -1).trim(); + } + return token; + }) + .filter(item => item.length > 0); if (tokens.length === 0) { return undefined; } @@ -226,6 +237,32 @@ export function parseRuleConditionAndScope( }; } +/** Leading PCRE-style inline flag group, e.g. `(?i)` or `(?ims)`. */ +const INLINE_FLAG_PREFIX = /^\(\?([a-z]+)\)/; + +/** Inline flags that map cleanly onto native `RegExp` flags. */ +const TRANSLATABLE_INLINE_FLAGS = /^[ims]+$/; + +/** + * Compile a rule `condition` into a `RegExp`, translating a leading PCRE-style + * inline flag group into native `RegExp` flags. + * + * JS/Bun `RegExp` rejects inline flag prefixes such as `(?i)`, so a rule written + * `condition: "(?i)pre.existing"` would otherwise throw at compile time and be + * silently dropped (see issue #4796). Only a *leading* group of `i`/`m`/`s` + * flags is translated; anything else — mid-pattern groups, unsupported flags — + * is passed through verbatim so the native error still surfaces for genuinely + * invalid patterns. + */ +export function compileRuleCondition(pattern: string): RegExp { + const match = INLINE_FLAG_PREFIX.exec(pattern); + if (match && TRANSLATABLE_INLINE_FLAGS.test(match[1])) { + const flags = Array.from(new Set(match[1])).join(""); + return new RegExp(pattern.slice(match[0].length), flags); + } + return new RegExp(pattern); +} + let activeRules: readonly Rule[] = []; /** diff --git a/packages/coding-agent/src/cli/ttsr-cli.ts b/packages/coding-agent/src/cli/ttsr-cli.ts index 108a05004..c20629bf7 100644 --- a/packages/coding-agent/src/cli/ttsr-cli.ts +++ b/packages/coding-agent/src/cli/ttsr-cli.ts @@ -15,7 +15,7 @@ import * as path from "node:path"; import { AstMatchStrictness, astMatch, FileType, type GlobMatch, glob } from "@oh-my-pi/pi-natives"; import { getProjectDir } from "@oh-my-pi/pi-utils/dirs"; import chalk from "chalk"; -import { BUILTIN_DEFAULTS_PROVIDER_ID, type Rule, ruleCapability } from "../capability/rule"; +import { BUILTIN_DEFAULTS_PROVIDER_ID, compileRuleCondition, type Rule, ruleCapability } from "../capability/rule"; import { bucketRules } from "../capability/rule-buckets"; import { Settings } from "../config/settings"; import type { TtsrSettings } from "../config/settings-schema"; @@ -173,7 +173,7 @@ async function regexMatches(rule: Rule, snippet: string): Promise { const out: string[] = []; for (const pattern of rule.condition ?? []) { try { - if (new RegExp(pattern).test(snippet)) out.push(pattern); + if (compileRuleCondition(pattern).test(snippet)) out.push(pattern); } catch { // Invalid regex — skip; the manager already warned at registration. } @@ -569,7 +569,7 @@ function compileScanRulePlans(rules: Rule[]): ScanRulePlan[] { const regexConditions: ScanRegexCondition[] = []; for (const pattern of rule.condition ?? []) { try { - regexConditions.push({ pattern, regex: new RegExp(pattern) }); + regexConditions.push({ pattern, regex: compileRuleCondition(pattern) }); } catch { // Same behavior as TtsrManager: invalid regex conditions are unusable. } diff --git a/packages/coding-agent/src/export/ttsr.ts b/packages/coding-agent/src/export/ttsr.ts index 27b885c40..ad3ba23ab 100644 --- a/packages/coding-agent/src/export/ttsr.ts +++ b/packages/coding-agent/src/export/ttsr.ts @@ -8,7 +8,7 @@ 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 { compileRuleCondition, type Rule } from "../capability/rule"; import type { TtsrSettings } from "../config/settings"; export type TtsrMatchSource = "text" | "thinking" | "tool"; @@ -103,7 +103,7 @@ export class TtsrManager { const compiled: RegExp[] = []; for (const pattern of rule.condition ?? []) { try { - compiled.push(new RegExp(pattern)); + compiled.push(compileRuleCondition(pattern)); } catch (error) { logger.warn("TTSR condition has invalid regex pattern, skipping condition", { ruleName: rule.name, diff --git a/packages/coding-agent/test/ttsr-inline-flags-scope.test.ts b/packages/coding-agent/test/ttsr-inline-flags-scope.test.ts new file mode 100644 index 000000000..aeffd8d73 --- /dev/null +++ b/packages/coding-agent/test/ttsr-inline-flags-scope.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from "bun:test"; +import { compileRuleCondition } from "@oh-my-pi/pi-coding-agent/capability/rule"; +import { buildRuleFromMarkdown, createSourceMeta } from "@oh-my-pi/pi-coding-agent/discovery/helpers"; +import { TtsrManager } from "@oh-my-pi/pi-coding-agent/export/ttsr"; + +/** + * Regression coverage for issue #4796: a rule with a leading `(?i)` inline regex + * flag and (separately) malformed `scope` frontmatter silently failed to + * register, so it could never fire. + */ +describe("TTSR inline flags + scope quoting (#4796)", () => { + it("translates leading (?i) into a case-insensitive RegExp", () => { + const regex = compileRuleCondition("(?i)pre.existing"); + expect(regex.flags).toBe("i"); + expect(regex.test("These are Pre-existing failures")).toBe(true); + }); + + it("passes through patterns without a leading inline flag group verbatim", () => { + const regex = compileRuleCondition("pre.existing"); + expect(regex.flags).toBe(""); + expect(regex.test("pre-existing")).toBe(true); + expect(regex.test("PRE-EXISTING")).toBe(false); + }); + + it("does not treat a mid-pattern (?...) group as an inline flag prefix", () => { + // `(?:...)` is a non-capturing group, not an inline flag directive. + const regex = compileRuleCondition("foo(?:bar)"); + expect(regex.flags).toBe(""); + expect(regex.test("foobar")).toBe(true); + }); + + it("registers and fires the reporter's exact rule end-to-end", () => { + // Reporter's frontmatter verbatim: leading (?i) condition + malformed + // `scope: "text","thinking"` (not valid YAML, forces the fallback path). + const content = [ + "---", + "name: fix-failures-now", + "description: prohibits pre-existing classification.", + 'condition: "(?i)(pre.existing|also fails on master|check.*master.*first)"', + 'scope: "text","thinking"', + "---", + "body", + ].join("\n"); + + const source = createSourceMeta("test", "fix-failures-now.md", "project"); + const rule = buildRuleFromMarkdown("fix-failures-now.md", content, "fix-failures-now.md", source); + + // Malformed scope recovers to canonical tokens (no literal quotes). + expect(rule.scope).toEqual(["text", "thinking"]); + // Condition survives the fallback without literal surrounding quotes. + expect(rule.condition).toEqual(["(?i)(pre.existing|also fails on master|check.*master.*first)"]); + + const manager = new TtsrManager(); + expect(manager.addRule(rule)).toBe(true); + + expect( + manager + .checkSnapshot("The CI failure was 4 pre-existing GPS map VR mismatches", { source: "thinking" }) + .map(r => r.name), + ).toEqual(["fix-failures-now"]); + + expect( + manager.checkSnapshot("Everything also fails on master anyway", { source: "text" }).map(r => r.name), + ).toEqual(["fix-failures-now"]); + }); +}); diff --git a/packages/utils/CHANGELOG.md b/packages/utils/CHANGELOG.md index 80ebafb3a..fae865822 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `parseFrontmatter`'s malformed-YAML fallback corrupting sibling values: one unparseable line (e.g. `scope: "text","thinking"`) forced every value through a raw key/value split that kept literal quotes. Each value is now reparsed independently as YAML, falling back to the raw trimmed string only for the lines that genuinely don't parse ([#4796](https://github.com/can1357/oh-my-pi/issues/4796)). + ## [16.3.10] - 2026-07-06 ### Added diff --git a/packages/utils/src/frontmatter.ts b/packages/utils/src/frontmatter.ts index 061ba87a3..8178172f1 100644 --- a/packages/utils/src/frontmatter.ts +++ b/packages/utils/src/frontmatter.ts @@ -149,12 +149,25 @@ export function parseFrontmatter( throw err; } - // Simple YAML parsing - just key: value pairs + // Simple key: value fallback. Reparse each value on its own so one + // malformed line (e.g. `scope: "text","thinking"`) can't leave sibling + // values wrapped in literal quotes; values that don't parse as YAML fall + // back to the raw trimmed string (issue #4796). for (const line of metadata.split("\n")) { const match = line.match(/^([\w-]+):\s*(.*)$/); - if (match) { - frontmatter[match[1]] = match[2].trim(); + if (!match) continue; + const raw = match[2].trim(); + let value: unknown = raw; + if (raw.length > 0) { + try { + const parsed = YAML.parse(raw); + if (parsed !== null && typeof parsed !== "object") value = parsed; + else if (Array.isArray(parsed)) value = parsed; + } catch { + // keep the raw string + } } + frontmatter[match[1]] = value; } return { frontmatter: normalizeKeys(frontmatter) as Record, body }; diff --git a/packages/utils/test/frontmatter.test.ts b/packages/utils/test/frontmatter.test.ts index a3adc4510..400b115a7 100644 --- a/packages/utils/test/frontmatter.test.ts +++ b/packages/utils/test/frontmatter.test.ts @@ -43,4 +43,26 @@ Body content`; expect.objectContaining({ err: expect.stringContaining("broken.md") }), ); }); + + it("reparses each fallback value so one malformed line can't corrupt its siblings", () => { + // `scope: "text","thinking"` is not valid YAML, forcing the line-by-line + // fallback. The sibling `condition` value must not inherit literal quotes, + // and `enabled` must reparse to a boolean (issue #4796). + const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {}); + const content = `--- +condition: "(?i)pre.existing" +scope: "text","thinking" +enabled: true +--- +Body`; + + const result = parseFrontmatter(content, { source: "rule.md" }); + + expect(result.frontmatter.condition).toBe("(?i)pre.existing"); + expect(result.frontmatter.enabled).toBe(true); + // The unrecoverable line survives as its raw trimmed string. + expect(result.frontmatter.scope).toBe('"text","thinking"'); + expect(result.body).toBe("Body"); + expect(warnSpy).toHaveBeenCalled(); + }); }); From 09fa0fcb6227d5548978f9146f1eb30265097f45 Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 15 Jul 2026 06:39:11 +0000 Subject: [PATCH 2/3] docs(ttsr): clarified scope frontmatter forms Documented both accepted scope encodings: a comma-separated YAML scalar and a YAML sequence. Added prose, thinking, block-list, and tool/path examples, plus an explicit warning that adjacent quoted scalars are invalid YAML. Updated malformed-frontmatter fallback semantics to match the fix in PR #5489. Fixes #4796 --- docs/rulebook-matching-pipeline.md | 43 +++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/docs/rulebook-matching-pipeline.md b/docs/rulebook-matching-pipeline.md index 3bd66fae5..d86bd4d7b 100644 --- a/docs/rulebook-matching-pipeline.md +++ b/docs/rulebook-matching-pipeline.md @@ -137,14 +137,15 @@ All providers use `parseFrontmatter` (`utils/frontmatter.ts`) with these semanti 1. Frontmatter is parsed only when content starts with `---` and has a closing `\n---`. 2. Body is trimmed after frontmatter extraction. -3. If YAML parse fails: - - warning is logged, - - parser falls back to simple `key: value` line parsing (`^([\w-]+):\s*(.*)$`). +3. If whole-document YAML parsing fails: + - a warning is logged, + - the parser falls back to simple `key: value` line parsing (`^([\w-]+):\s*(.*)$`), + - each captured value is reparsed independently as YAML, and only values that still fail parsing remain raw trimmed strings. -Ambiguity consequences: +Fallback limitations: -- Fallback parser does not support arrays, nested objects, or quoting rules. -- Fallback values become strings (for example `alwaysApply: true` becomes string `"true"`), so providers requiring boolean/string types may drop metadata. +- Multiline arrays, nested objects, and other indentation-dependent YAML structures are not reconstructed. A valid one-line flow value (for example `[text, thinking]`) can still survive the per-value reparse. +- An individually malformed value remains a raw string; providers requiring a boolean, list, or object may drop that metadata. - `ttsr_trigger` works in fallback (underscore key); hyphenated keys like `thinking-level` also parse and are normalized to camelCase (`thinkingLevel`) — key normalization applies to the YAML path too. - Files without valid frontmatter still load as rules with empty metadata and full content body. @@ -226,9 +227,33 @@ After rule discovery in `createAgentSession` (`sdk.ts`), `bucketRules(...)` appl ### `condition`, `astCondition`, `scope`, and `interruptMode` -- `condition` is the regex TTSR trigger field; legacy `ttsr_trigger` / `ttsrTrigger` are accepted as fallback inputs during parsing. -- `astCondition` is the ast-grep trigger field: a string or list of structural patterns, kept verbatim (no glob inference). It only matches on edit/write tool streams, where the language is inferred from the file path. A rule may set `condition`, `astCondition`, or both. -- `scope` narrows TTSR matching scope. A `condition` token that looks like a file glob becomes `tool:edit()` and `tool:write()` scope entries plus catch-all condition `.*`; `astCondition` tokens never trigger this shorthand. +- `condition` is the regex TTSR trigger field; legacy `ttsr_trigger` / `ttsrTrigger` are accepted as fallback inputs during parsing. A leading `(?i)`, `(?m)`, or `(?s)` inline flag group is translated to the equivalent JavaScript `RegExp` flags. +- `astCondition` is the ast-grep trigger field: a string or YAML sequence of structural patterns, kept verbatim (no glob inference). It only matches on edit/write tool streams, where the language is inferred from the file path. A rule may set `condition`, `astCondition`, or both. +- `scope` narrows TTSR matching to an allowlist of stream surfaces. It accepts either a comma-separated YAML string or a YAML sequence. Omitting it watches assistant prose (`text`) and all tool arguments (`tool`), but not thinking. + + ```yaml + # Prose and thinking; equivalent forms: + scope: "text, thinking" + ``` + + ```yaml + scope: [text, thinking] + ``` + + ```yaml + # A block-style YAML sequence is also valid: + scope: + - text + - thinking + ``` + + ```yaml + # Only TypeScript source snapshots produced by edit/write: + scope: "tool:edit(*.ts), tool:write(*.ts)" + ``` + + Valid tokens are `text`, `thinking`, `tool` (or `toolcall`), and `tool:()`. Do not write `scope: "text","thinking"`: adjacent quoted scalars are not valid YAML; put the comma inside one string or use a YAML sequence. +- A `condition` token that looks like a file glob becomes `tool:edit()` and `tool:write()` scope entries plus catch-all condition `.*`; `astCondition` tokens never trigger this shorthand. - `interruptMode` can override the global TTSR interrupt mode for the rule. ## 7. System prompt inclusion path From 02efdee788830a97e4b71658e0f379114d20f50f Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 15 Jul 2026 06:45:41 +0000 Subject: [PATCH 3/3] fix(omfg): compiled generated-rule conditions via inline-flag helper The /omfg generated-rule path validated conditions with raw new RegExp in normalizeConditionRegex and isValidRegexCondition, so a generated rule with a leading (?i)/(?m)/(?s) inline flag was rejected as "Invalid condition regex" before reaching TtsrManager.addRule. Route all three validation sites through compileRuleCondition. Fixes #4796 --- .../src/modes/controllers/omfg-rule.ts | 8 ++++---- .../test/modes/controllers/omfg-rule.test.ts | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/src/modes/controllers/omfg-rule.ts b/packages/coding-agent/src/modes/controllers/omfg-rule.ts index 59ff36631..f5e9fcf7f 100644 --- a/packages/coding-agent/src/modes/controllers/omfg-rule.ts +++ b/packages/coding-agent/src/modes/controllers/omfg-rule.ts @@ -1,7 +1,7 @@ import * as path from "node:path"; import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; import type { AssistantMessage } from "@oh-my-pi/pi-ai"; -import type { Rule } from "../../capability/rule"; +import { compileRuleCondition, type Rule } from "../../capability/rule"; import { buildRuleFromMarkdown, createSourceMeta } from "../../discovery/helpers"; import { TtsrManager, type TtsrMatchContext } from "../../export/ttsr"; @@ -73,13 +73,13 @@ function normalizeConditionRegexes(conditions: readonly string[]): { condition: function normalizeConditionRegex(condition: string): { condition: string } | { error: string } { try { - new RegExp(condition); + compileRuleCondition(condition); return { condition }; } catch (originalError) { const repaired = unescapeRegexConditionOnce(condition); if (repaired !== condition) { try { - new RegExp(repaired); + compileRuleCondition(repaired); return { condition: repaired }; } catch {} } @@ -404,7 +404,7 @@ export function ruleMatchesAssistantHistory(rule: Rule, messages: readonly Agent function isValidRegexCondition(condition: string): boolean { try { - new RegExp(condition); + compileRuleCondition(condition); return true; } catch { return false; diff --git a/packages/coding-agent/test/modes/controllers/omfg-rule.test.ts b/packages/coding-agent/test/modes/controllers/omfg-rule.test.ts index 9b0863b09..d86172623 100644 --- a/packages/coding-agent/test/modes/controllers/omfg-rule.test.ts +++ b/packages/coding-agent/test/modes/controllers/omfg-rule.test.ts @@ -97,6 +97,11 @@ describe("omfg rule parsing", () => { expect("error" in invalidRegex ? invalidRegex.error : "").toContain("Invalid condition regex"); }); + it("accepts a leading inline regex flag in generated conditions", () => { + const result = mustParse(ruleJson({ name: "no-preexisting", condition: "(?i)pre.existing", scope: "text" })); + expect(result.rule.condition).toEqual(["(?i)pre.existing"]); + }); + it("sanitizes generated names to slugs", () => { expect(sanitizeRuleName(" Caps & Spaces!! ")).toBe("caps-spaces"); expect(sanitizeRuleName("already_ok-123")).toBe("already_ok-123"); @@ -130,6 +135,15 @@ describe("ruleMatchesAssistantHistory", () => { expect(ruleMatchesAssistantHistory(rule, messages)).toBe(true); }); + it("matches case-insensitively when the condition leads with (?i)", () => { + const { rule } = mustParse(ruleJson({ name: "no-preexisting", condition: "(?i)pre.existing", scope: "text" })); + const messages: AgentMessage[] = [ + createAssistantMessage([{ type: "text", text: "These are Pre-existing failures." }]), + ]; + + expect(ruleMatchesAssistantHistory(rule, messages)).toBe(true); + }); + it("returns false when the pattern is absent", () => { const { rule } = mustParse(ruleJson({ name: "absent", condition: "needle", scope: "text" })); const messages: AgentMessage[] = [createAssistantMessage([{ type: "text", text: "Only hay here." }])];