Skip to content

Commit 9c8171a

Browse files
committed
fix: bound workflow keyword matching
1 parent edd3534 commit 9c8171a

6 files changed

Lines changed: 92 additions & 17 deletions

File tree

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ Run a workflow to audit every route under src/routes/ for missing auth checks.
3434

3535
Pi writes and starts the workflow in the background. A live panel tracks progress while you keep working, and the final result is delivered back into the conversation automatically.
3636

37-
Use the word **workflow** or **workflows** in a message to force workflow mode, or run `/workflows run <prompt>` explicitly. If the default keyword is too broad, change it with `/workflows-trigger set pi-workflow` or disable it with `/workflows-trigger off`.
37+
Keyword triggering is on by default: use the bounded word **workflow** or **workflows** in a message to force workflow mode, or run `/workflows run <prompt>` explicitly. Identifier-like text and paths such as `myworkflow`, `workflow_name`, and `src/workflow-editor.ts` do not trigger. You can change the keyword with `/workflows-trigger set pi-workflow` or disable it with `/workflows-trigger off`.
3838

3939
## How it works
4040

@@ -199,7 +199,7 @@ Set a literal, case-insensitive custom trigger in `~/.pi/workflows/settings.json
199199
}
200200
```
201201

202-
The default `workflow` also matches `workflows`; a custom word matches exactly. If another extension owns Pi's custom editor, the submit-time trigger still works, but animated keyword highlighting and Backspace one-shot disarm are unavailable. Editor visuals are load-order dependent.
202+
The default `workflow` also matches `workflows`; a custom word matches exactly. Trigger words are case-insensitive and Unicode identifier-bounded, and do not activate inside paths, slash commands, or identifier-like text. If another extension owns Pi's custom editor, the submit-time trigger still works, but animated keyword highlighting and Backspace one-shot disarm are unavailable. Editor visuals are load-order dependent.
203203

204204
</details>
205205

src/workflow-editor.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/**
22
* "Workflows mode" input affordance, à la a smart input box:
33
*
4-
* - While the editor text contains the word `workflow`/`workflows`, those letters
5-
* render as a flowing rainbow, signalling that submitting will engage a workflow.
4+
* - While the editor text contains the bounded word `workflow`/`workflows`, those
5+
* letters render as a flowing rainbow, signalling that submitting will engage a workflow.
66
* - Pressing Backspace immediately after such a word toggles the highlight OFF
77
* (the word stays, but turns plain white) — a non-destructive "don't run a
88
* workflow after all". Re-typing a fresh trigger word turns it back on.
@@ -32,21 +32,20 @@ import {
3232
type WorkflowSettingsStore,
3333
} from "./workflow-settings.js";
3434

35-
// A keyword trigger is a configured literal term. The default `workflow`
36-
// trigger keeps legacy substring behavior and plural support (`workflows`) while
37-
// custom trigger words match only that exact term. Slash commands like
38-
// `/workflows` or `/pi-workflow` are left alone (not colored, not armed).
35+
// A keyword trigger is a configured literal term. All trigger words use token
36+
// boundaries so slash commands, paths, and identifier-like text stay untouched.
37+
// The default `workflow` trigger additionally supports the plural `workflows`.
3938
function escapeRegExp(text: string): string {
4039
return text.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
4140
}
4241

4342
function triggerSource(triggerWord: string): string {
4443
const escaped = escapeRegExp(triggerWord);
45-
if (triggerWord.toLowerCase() === DEFAULT_KEYWORD_TRIGGER_WORD) return `(?<!\\/)${escaped}s?`;
46-
return `(?<![/A-Za-z0-9_-])${escaped}(?![A-Za-z0-9_-])`;
44+
const plural = triggerWord.toLowerCase() === DEFAULT_KEYWORD_TRIGGER_WORD ? "s?" : "";
45+
return `(?<![/\\p{ID_Continue}$-])(?<!\\\\)${escaped}${plural}(?![/\\p{ID_Continue}$-])(?!\\\\)`;
4746
}
4847

49-
function triggerRegex(triggerWord = DEFAULT_KEYWORD_TRIGGER_WORD, flags = "i", atEnd = false): RegExp {
48+
function triggerRegex(triggerWord = DEFAULT_KEYWORD_TRIGGER_WORD, flags = "iu", atEnd = false): RegExp {
5049
const word = normalizeKeywordTriggerWord(triggerWord) ?? DEFAULT_KEYWORD_TRIGGER_WORD;
5150
return new RegExp(`${triggerSource(word)}${atEnd ? "$" : ""}`, flags);
5251
}
@@ -62,7 +61,7 @@ export function hasTrigger(text: string, triggerWord = DEFAULT_KEYWORD_TRIGGER_W
6261
}
6362

6463
export function endsWithTrigger(textBeforeCursor: string, triggerWord = DEFAULT_KEYWORD_TRIGGER_WORD): boolean {
65-
return triggerRegex(triggerWord, "i", true).test(textBeforeCursor);
64+
return triggerRegex(triggerWord, "iu", true).test(textBeforeCursor);
6665
}
6766

6867
/** Shared, mutable view of whether "workflows mode" is currently armed. */
@@ -138,7 +137,7 @@ export function colorizeWorkflow(
138137
if (!hasTrigger(visible, triggerWord)) return line;
139138

140139
const ranges: Array<[number, number]> = [];
141-
const globalTrigger = triggerRegex(triggerWord, "gi");
140+
const globalTrigger = triggerRegex(triggerWord, "giu");
142141
for (let m = globalTrigger.exec(visible); m; m = globalTrigger.exec(visible)) {
143142
ranges.push([m.index, m.index + m[0].length]);
144143
}

tests/agent-registry.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,11 @@ describe("loadAgentRegistry", () => {
147147
it("default userDir resolution uses getAgentDir() (~/.pi/agent/agents) with no injected opts", () => {
148148
const tmpHome = mkdtempSync(join(tmpdir(), "pi-home-"));
149149
const originalHome = process.env.HOME;
150+
const originalUserProfile = process.env.USERPROFILE;
150151
const originalAgentDirEnv = process.env.PI_CODING_AGENT_DIR;
151152
delete process.env.PI_CODING_AGENT_DIR;
152153
process.env.HOME = tmpHome;
154+
process.env.USERPROFILE = tmpHome;
153155
try {
154156
const expectedUserDir = join(getAgentDir(), "agents");
155157
assert.equal(expectedUserDir, join(tmpHome, ".pi", "agent", "agents"), "sanity: HOME override took effect");
@@ -163,6 +165,8 @@ describe("loadAgentRegistry", () => {
163165
} finally {
164166
if (originalHome === undefined) delete process.env.HOME;
165167
else process.env.HOME = originalHome;
168+
if (originalUserProfile === undefined) delete process.env.USERPROFILE;
169+
else process.env.USERPROFILE = originalUserProfile;
166170
if (originalAgentDirEnv === undefined) delete process.env.PI_CODING_AGENT_DIR;
167171
else process.env.PI_CODING_AGENT_DIR = originalAgentDirEnv;
168172
rmSync(tmpHome, { recursive: true, force: true });

tests/task-panel.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import assert from "node:assert/strict";
22
import { EventEmitter } from "node:events";
3+
import { join } from "node:path";
34
import { before, describe, it } from "node:test";
45
import type { ExtensionAPI } from "@earendil-works/pi-coding-agent";
56
import { visibleWidth } from "@earendil-works/pi-tui";
@@ -242,7 +243,7 @@ describe("installResultDelivery", () => {
242243

243244
const content = (pi as unknown as { _calls: { content: string }[] })._calls[0].content;
244245
assert.ok(content.includes("Full result:"), "should include the pointer label");
245-
assert.ok(content.includes("/runs/test-run-1.json"), "should point at <runsDir>/<runId>.json");
246+
assert.ok(content.includes(join("/runs", "test-run-1.json")), "should point at <runsDir>/<runId>.json");
246247
// The verdict summary itself is unchanged apart from the appended pointer.
247248
assert.ok(content.includes("All tests passed"), "verdict text preserved");
248249
});
@@ -275,7 +276,7 @@ describe("installResultDelivery", () => {
275276
const content = (pi as unknown as { _calls: { content: string }[] })._calls[0].content;
276277
assert.ok(/\(truncated [\d.]+ (B|KB|MB)\)/.test(content), "the 50-char setting truncates a sub-400 dump");
277278
assert.ok(!content.includes("z".repeat(200)), "the body is cut at the configured threshold");
278-
assert.ok(content.includes("/runs/test-run-1.json"), "pointer still appended");
279+
assert.ok(content.includes(join("/runs", "test-run-1.json")), "pointer still appended");
279280
});
280281

281282
// ── installResultDelivery: guard / stale ctx ──

tests/usage-limit-integration.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { existsSync, mkdtempSync, rmSync } from "node:fs";
1515
import { tmpdir } from "node:os";
1616
import { join } from "node:path";
1717
import test from "node:test";
18-
import { fileURLToPath } from "node:url";
18+
import { fileURLToPath, pathToFileURL } from "node:url";
1919
import { WorkflowAgent } from "../src/agent.js";
2020
import { WorkflowErrorCode } from "../src/errors.js";
2121
import { WorkflowManager } from "../src/workflow-manager.js";
@@ -38,7 +38,7 @@ async function loadFaux(): Promise<typeof import("@earendil-works/pi-ai/compat")
3838
import.meta.url,
3939
),
4040
);
41-
const entry = existsSync(nested) ? nested : "@earendil-works/pi-ai/compat";
41+
const entry = existsSync(nested) ? pathToFileURL(nested).href : "@earendil-works/pi-ai/compat";
4242
return import(entry) as Promise<typeof import("@earendil-works/pi-ai/compat")>;
4343
}
4444

tests/workflow-editor.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,49 @@ describe("hasTrigger", () => {
143143
assert.equal(hasTrigger("/workflow"), false);
144144
});
145145

146+
it("requires token boundaries for the built-in trigger", async () => {
147+
const { hasTrigger } = await load();
148+
for (const text of [
149+
"myworkflow",
150+
"workflows2",
151+
"workflow_name",
152+
"workflow-based",
153+
"src/workflow-editor.ts",
154+
"src\\workflow-editor.ts",
155+
]) {
156+
assert.equal(hasTrigger(text), false, `${text} should not trigger`);
157+
}
158+
for (const text of ["workflow, please", "(workflows)", "WORKFLOW!", "Discuss workflows."]) {
159+
assert.equal(hasTrigger(text), true, `${text} should trigger`);
160+
}
161+
});
162+
163+
it("rejects Unicode identifier and dollar boundaries on either side", async () => {
164+
const { hasTrigger } = await load();
165+
for (const text of [
166+
"$workflow",
167+
"workflow$",
168+
"caféworkflow",
169+
"workflowcafé",
170+
"变量workflow变量",
171+
"变量workflow",
172+
"workflow变量",
173+
]) {
174+
assert.equal(hasTrigger(text), false, `${text} should not trigger`);
175+
}
176+
for (const text of ["¿workflow?", "café, workflow!", "变量:workflow。", "workflow—please"]) {
177+
assert.equal(hasTrigger(text), true, `${text} should trigger`);
178+
}
179+
});
180+
181+
it("applies path and Unicode identifier boundaries to custom triggers", async () => {
182+
const { hasTrigger } = await load();
183+
for (const text of ["xpi-workflow", "pi-workflow变量", "src/pi-workflow", "src\\pi-workflow"]) {
184+
assert.equal(hasTrigger(text, "pi-workflow"), false, `${text} should not trigger`);
185+
}
186+
assert.equal(hasTrigger("run pi-workflow, please", "pi-workflow"), true);
187+
});
188+
146189
it("returns false for unrelated text", async () => {
147190
const { hasTrigger } = await load();
148191
assert.equal(hasTrigger("hello world"), false);
@@ -852,6 +895,34 @@ describe("installWorkflowEditor", () => {
852895
assert.match(sent.at(-1)?.content ?? "", /workflow\/workflows/);
853896
});
854897

898+
it("keeps keyword triggering enabled when the setting is absent or loading fails", async () => {
899+
const mod = await load();
900+
const stores = [
901+
{ load: () => ({}), save: () => {} },
902+
{
903+
load: () => {
904+
throw new Error("read failed");
905+
},
906+
save: () => {},
907+
},
908+
];
909+
910+
for (const settingsStore of stores) {
911+
const pi = {
912+
on: () => {},
913+
registerCommand: () => {},
914+
getActiveTools: () => [],
915+
setActiveTools: () => {},
916+
} as unknown as ExtensionAPI;
917+
const ui = { setEditorComponent: () => {} } as unknown as ExtensionUIContext;
918+
919+
const state = mod.installWorkflowEditor(pi, ui, undefined, { settingsStore });
920+
921+
assert.equal(state.keywordTriggerEnabled, true);
922+
assert.equal(state.keywordTriggerWord, "workflow");
923+
}
924+
});
925+
855926
it("loads the persisted keyword trigger preference on install", async () => {
856927
const mod = await load();
857928
const captured: Array<{ event: string; handler: (...args: unknown[]) => unknown }> = [];

0 commit comments

Comments
 (0)