diff --git a/AGENTS.md b/AGENTS.md index e6251d55..de989ba6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -198,6 +198,8 @@ Every file in `.openfox/` must be **committable** and **meaningful in the projec **Rationale:** Clean repo, no leaked personal config, reliable source of truth for CI and collaborators. +**Command arguments:** a command's Message template takes `{{name}}` placeholders, filled positionally by order of appearance, and `{{ARGUMENTS}}`, which takes everything typed after the id. Quoted values (`/cmd "two words"`) fill one placeholder whole. Parsing lives in `src/shared/slash-args.ts` and is shared by the chat composer and the task board's server-side resolution — change it there, not in either caller. Full design: [docs/DESIGN-SLASH-ARGUMENTS.md](DESIGN-SLASH-ARGUMENTS.md). + **Workflows:** When asked to create or edit a workflow, load the built-in `workflows` skill via `load_skill("workflows")` — it is the authoritative reference for workflow file format, storage locations (project `.openfox/workflows/` vs global `{configDir}/workflows/`), the full JSON schema, step types, transition conditions, and template variables ([docs/WORKFLOWS.md](docs/WORKFLOWS.md) is a pointer to it). Project workflows belong in `.openfox/workflows/` and are committable. ## TDD Workflow diff --git a/docs/DESIGN-SLASH-ARGUMENTS.md b/docs/DESIGN-SLASH-ARGUMENTS.md new file mode 100644 index 00000000..86ff0775 --- /dev/null +++ b/docs/DESIGN-SLASH-ARGUMENTS.md @@ -0,0 +1,87 @@ +# Slash Command Arguments + +## Goal + +Make command arguments usable for the two things people actually type: a value that contains spaces, and a free-form instruction. + +Commands already accepted positional arguments — `{{name}}` placeholders filled by order of appearance — but the invocation was split on `/\s+/` with no notion of quoting, and there was no way to capture a whole sentence. Both gaps push users back to editing the command every time they want to vary it. + +``` +/revue src/a.ts "gestion des erreurs" → {{angle}} got `"gestion`, the rest was dropped +/note rerun the flaky proxy test → no placeholder could hold this +``` + +## User Experience + +Two additions, both in the command's Message template. + +**Quoted values.** Wrap an argument in `"…"` or `'…'` and it fills one placeholder whole. + +```markdown +Relis {{file}} en te concentrant sur {{angle}}. +``` + +`/revue src/a.ts "gestion des erreurs"` → _Relis src/a.ts en te concentrant sur gestion des erreurs._ + +**`{{ARGUMENTS}}`.** Receives everything typed after the command id, verbatim. + +```markdown +Add a note to the plan: {{ARGUMENTS}} +``` + +`/note rerun the flaky proxy test` → _Add a note to the plan: rerun the flaky proxy test_ + +Both forms coexist with what already worked: unfilled placeholders still open the params modal, so `/revue` alone asks for `file` and `angle` in a dialog. The command editor now carries a one-line description of all of this, which nothing documented before. + +## Technical Design + +### One module, both callers + +Parsing lived in two places that had drifted into near-duplicates: `web/src/lib/parse-slash-command.ts` for the chat composer and `src/server/tasks/slash.ts` for slash commands seeded into task sessions. Each had its own `extractTemplateParams` and its own `split(/\s+/)`. + +`src/shared/slash-args.ts` is now the single implementation, reachable from web through the existing `@shared` alias. Both callers delegate to it, so a command typed in chat and the same command seeded into a task expand identically — which was the point of `src/server/tasks/slash.ts` existing in the first place. + +```ts +tokenizeArgs(line): string[] // quote-aware split +parseSlashInput(prompt): { id, args, rest } | null +extractTemplateParams(template): string[] // every {{name}}, in order, deduplicated +positionalTemplateParams(template): string[] // …except {{ARGUMENTS}} +templateParamHints(template): string[] // positional first, {{ARGUMENTS}} last +resolveTemplateParams(template, args, rest): { params, unfilledParams } +applyTemplateParams(template, params): string +expandCommandPrompt(template, args, rest): { prompt, unfilledParams } +``` + +### Tokenizer + +A `"…"` or `'…'` run is one token and the quotes are stripped. Inside double quotes a backslash escapes the next character; single quotes are literal, as in POSIX shells. Quoted and unquoted fragments that touch join into one token, so `src/"my file".ts` is a single path. + +An unterminated quote takes the rest of the line rather than erroring. Someone mid-keystroke has an unterminated quote most of the time, and the inline hint reads the same buffer. + +### `{{ARGUMENTS}}` + +Exact, uppercase, case-sensitive — a placeholder named `arguments` stays an ordinary positional slot. It is excluded from positional numbering, so it never consumes a token another placeholder was waiting for. It takes `rest`: the raw text after the id, trimmed, quotes included. + +Raw rather than re-joined tokens because `{{ARGUMENTS}}` means "what I typed". A template that mixes both forms sees each argument twice — once in its positional slot, once inside `{{ARGUMENTS}}` — which matches `$1` and `$ARGUMENTS` elsewhere and is what someone writing that template is asking for. + +Nothing typed after the id leaves `{{ARGUMENTS}}` unfilled, and it flows through the existing unfilled-placeholder path: the modal asks, or `resolveSlashLaunch` declines and the task falls back to the raw prompt. + +### Inline hints + +The composer's `param=?` hint counted arguments with `split(/\s+/)`, so a quoted value made it skip ahead. It now counts with `tokenizeArgs`. + +`templateParamHints` orders `{{ARGUMENTS}}` last regardless of where it appears in the template, so the hint walks the positional slots first and only then offers `ARGUMENTS=?` — the order in which they have to be typed. The server computes it in `routes/commands.ts`, so the composer and the task editor get the same hints from one place. + +## Edge Cases + +- **Backward compatibility.** With no `{{ARGUMENTS}}` in the template, positional resolution is what it was; with no quotes in the invocation, tokenization is what it was. Every pre-existing test passes with only the two new fields added to the parse result. +- **Empty quoted argument.** `/cmd "" b` yields `['', 'b']`; the placeholder is filled with an empty string rather than reported unfilled — the user said "nothing" explicitly. +- **Workflows.** They share the tokenizer, so quoted values now work for workflow parameters too. `{{ARGUMENTS}}` does not apply: workflows declare typed parameters instead of a template. +- **Repeated placeholder.** `{{a}} … {{a}}` counts as one slot and every occurrence is replaced. +- **Unfilled placeholder.** Left literally in the prompt and reported, never substituted with an empty string — that is what lets the caller ask instead of silently sending `{{file}}` to the model. + +## Out of Scope + +- Named arguments (`--file=x`) +- Default values and required markers for command placeholders — workflows have declared `parameters` with `required` and `position`; commands infer their names from the template +- Shell execution or file inclusion inside a command template diff --git a/src/server/routes/commands.ts b/src/server/routes/commands.ts index 6b857358..3f7c2917 100644 --- a/src/server/routes/commands.ts +++ b/src/server/routes/commands.ts @@ -14,6 +14,7 @@ import { } from '../commands/registry.js' import type { CommandDefinition } from '../commands/types.js' import { createCrudRoutes, validateNameIdPrompt, type CrudRouteConfig } from './crud-helpers.js' +import { templateParamHints } from '../../shared/slash-args.js' const config: CrudRouteConfig = { dirName: 'commands', @@ -34,7 +35,7 @@ const config: CrudRouteConfig = { mapToResponse: (c) => ({ ...c.metadata, - paramNames: [...new Set(Array.from(c.prompt.matchAll(/\{\{(\w+)\}\}/g), (m) => m[1]!))], + paramNames: templateParamHints(c.prompt), }) as unknown as { [key: string]: unknown }, } diff --git a/src/server/tasks/slash.test.ts b/src/server/tasks/slash.test.ts index 65d87bf2..b4d2bb72 100644 --- a/src/server/tasks/slash.test.ts +++ b/src/server/tasks/slash.test.ts @@ -12,13 +12,14 @@ describe('parseSlashInvocation', () => { }) it('parses a bare command id with no args', () => { - expect(parseSlashInvocation('/lint')).toEqual({ id: 'lint', args: [] }) + expect(parseSlashInvocation('/lint')).toEqual({ id: 'lint', args: [], rest: '' }) }) it('parses args, trimming surrounding whitespace', () => { expect(parseSlashInvocation(' /fixme crash src/a.ts ')).toEqual({ id: 'fixme', args: ['crash', 'src/a.ts'], + rest: 'crash src/a.ts', }) }) @@ -31,6 +32,7 @@ describe('parseSlashInvocation', () => { expect(parseSlashInvocation('/run vitest run src/x.test.ts')).toEqual({ id: 'run', args: ['vitest', 'run', 'src/x.test.ts'], + rest: 'vitest run src/x.test.ts', }) }) }) @@ -212,6 +214,62 @@ describe('resolveSlashLaunch (filesystem-backed)', () => { expect(resolved).toEqual({ kind: 'command', prompt: 'Fix the crash bug in src/a.ts.', agentMode: 'builder' }) }) + it('feeds a quoted multi-word argument into a single placeholder', async () => { + await writeFile( + join(configDir, 'commands', 'revue.command.md'), + '---\nid: revue\nname: Revue\n---\n\nRelis {{file}} en te concentrant sur {{angle}}.', + ) + const resolved = await resolveSlashLaunch(configDir, undefined, '/revue src/a.ts "gestion des erreurs"') + expect(resolved).toEqual({ kind: 'command', prompt: 'Relis src/a.ts en te concentrant sur gestion des erreurs.' }) + }) + + it('feeds the whole line into {{ARGUMENTS}}', async () => { + await writeFile( + join(configDir, 'commands', 'note.command.md'), + '---\nid: note\nname: Note\n---\n\nAdd a note: {{ARGUMENTS}}', + ) + const resolved = await resolveSlashLaunch(configDir, undefined, '/note rerun the flaky proxy test') + expect(resolved).toEqual({ kind: 'command', prompt: 'Add a note: rerun the flaky proxy test' }) + }) + + it('returns null for an {{ARGUMENTS}} command invoked with nothing after the id', async () => { + await writeFile( + join(configDir, 'commands', 'note.command.md'), + '---\nid: note\nname: Note\n---\n\nAdd a note: {{ARGUMENTS}}', + ) + expect(await resolveSlashLaunch(configDir, undefined, '/note')).toBeNull() + }) + + it('combines a positional placeholder with {{ARGUMENTS}}', async () => { + await writeFile( + join(configDir, 'commands', 'revue.command.md'), + '---\nid: revue\nname: Revue\n---\n\nRelis {{file}}. Consignes : {{ARGUMENTS}}', + ) + const resolved = await resolveSlashLaunch(configDir, undefined, '/revue src/a.ts sois impitoyable') + expect(resolved).toEqual({ + kind: 'command', + prompt: 'Relis src/a.ts. Consignes : src/a.ts sois impitoyable', + }) + }) + + it('honours quoted arguments for workflow parameters too', async () => { + await writeFile( + join(configDir, 'workflows', 'fixit.workflow.json'), + JSON.stringify( + fixtureWorkflow('fixit', [ + { id: 'issue', position: 0 }, + { id: 'file', position: 1 }, + ]), + ), + ) + const resolved = await resolveSlashLaunch(configDir, undefined, '/fixit "crash au boot" src/a.ts') + expect(resolved).toEqual({ + kind: 'workflow', + workflowId: 'fixit', + params: { issue: 'crash au boot', file: 'src/a.ts' }, + }) + }) + it('prefers workflows over commands when an id collides', async () => { await writeFile(join(configDir, 'workflows', 'both.workflow.json'), JSON.stringify(fixtureWorkflow('both'))) await writeFile(join(configDir, 'commands', 'both.command.md'), '---\nid: both\nname: Both\n---\n\nCommand body') diff --git a/src/server/tasks/slash.ts b/src/server/tasks/slash.ts index 5ec0ecc7..dc540162 100644 --- a/src/server/tasks/slash.ts +++ b/src/server/tasks/slash.ts @@ -13,11 +13,9 @@ import { loadAllCommands, findCommandById } from '../commands/registry.js' import { loadAllWorkflows, findWorkflowById } from '../workflows/registry.js' +import { expandCommandPrompt, parseSlashInput, type SlashInput } from '../../shared/slash-args.js' -export interface SlashInvocation { - id: string - args: string[] -} +export type SlashInvocation = SlashInput export interface WorkflowSlashLaunch { kind: 'workflow' @@ -35,16 +33,11 @@ export interface CommandSlashLaunch { export type SlashLaunch = WorkflowSlashLaunch | CommandSlashLaunch | null /** - * Split a prompt into a slash id + positional args. Returns null when the - * prompt is not a slash invocation (or is just a bare "/"). + * Split a prompt into a slash id, its tokenized args and the raw remainder. + * Returns null when the prompt is not a slash invocation (or is a bare "/"). */ export function parseSlashInvocation(prompt: string): SlashInvocation | null { - const trimmed = prompt.trim() - if (!trimmed.startsWith('/')) return null - const parts = trimmed.slice(1).split(/\s+/) - const id = parts[0] - if (!id) return null - return { id, args: parts.slice(1) } + return parseSlashInput(prompt) } /** @@ -71,37 +64,7 @@ export function workflowParamsFromArgs( return params } -/** Named template placeholders ({{name}}) in order of first occurrence, deduplicated. */ -export function extractTemplateParams(template: string): string[] { - const seen: string[] = [] - const regex = /\{\{(\w+)\}\}/g - let match: RegExpExecArray | null - while ((match = regex.exec(template)) !== null) { - const key = match[1]! - if (!seen.includes(key)) seen.push(key) - } - return seen -} - -/** - * Substitute positional args into a command's prompt template. Args map to - * placeholders by order of appearance. Returns the expanded prompt plus the - * placeholders that remain unfilled (so callers can degrade gracefully). - */ -export function expandCommandPrompt(template: string, args: string[]): { prompt: string; unfilledParams: string[] } { - const paramNames = extractTemplateParams(template) - const named: Record = {} - paramNames.forEach((name, index) => { - const value = args[index] - if (value !== undefined) named[name] = value - }) - let prompt = template - for (const [key, value] of Object.entries(named)) { - prompt = prompt.replaceAll(`{{${key}}}`, value) - } - const unfilledParams = paramNames.filter((name) => !(name in named)) - return { prompt, unfilledParams } -} +export { expandCommandPrompt, extractTemplateParams } from '../../shared/slash-args.js' /** * Resolve a prompt against the available workflows and commands. Returns null @@ -138,7 +101,7 @@ export async function resolveSlashLaunch( const command = findCommandById(invocation.id, commands) if (!command) return null - const expanded = expandCommandPrompt(command.prompt, invocation.args) + const expanded = expandCommandPrompt(command.prompt, invocation.args, invocation.rest) if (expanded.unfilledParams.length > 0) return null return { diff --git a/src/shared/slash-args.test.ts b/src/shared/slash-args.test.ts new file mode 100644 index 00000000..fcf5d811 --- /dev/null +++ b/src/shared/slash-args.test.ts @@ -0,0 +1,199 @@ +import { describe, it, expect } from 'vitest' +import { + ARGUMENTS_PARAM, + applyTemplateParams, + expandCommandPrompt, + extractTemplateParams, + parseSlashInput, + positionalTemplateParams, + resolveTemplateParams, + templateParamHints, + tokenizeArgs, +} from './slash-args.js' + +describe('tokenizeArgs', () => { + it('splits on whitespace', () => { + expect(tokenizeArgs('crash src/a.ts')).toEqual(['crash', 'src/a.ts']) + }) + + it('collapses runs of whitespace', () => { + expect(tokenizeArgs(' a b ')).toEqual(['a', 'b']) + }) + + it('returns nothing for an empty line', () => { + expect(tokenizeArgs('')).toEqual([]) + expect(tokenizeArgs(' ')).toEqual([]) + }) + + it('keeps a double-quoted run as one token and strips the quotes', () => { + expect(tokenizeArgs('foo.ts "gestion des erreurs"')).toEqual(['foo.ts', 'gestion des erreurs']) + }) + + it('keeps a single-quoted run as one token', () => { + expect(tokenizeArgs("'two words' tail")).toEqual(['two words', 'tail']) + }) + + it('treats the other quote character as literal inside a quoted run', () => { + expect(tokenizeArgs(`"it's fine"`)).toEqual(["it's fine"]) + expect(tokenizeArgs(`'say "hi"'`)).toEqual(['say "hi"']) + }) + + it('honours backslash escapes inside double quotes only', () => { + expect(tokenizeArgs('"a \\"quoted\\" word"')).toEqual(['a "quoted" word']) + expect(tokenizeArgs("'a \\ b'")).toEqual(['a \\ b']) + }) + + it('joins quoted and unquoted fragments that touch', () => { + expect(tokenizeArgs('src/"my file".ts')).toEqual(['src/my file.ts']) + }) + + it('swallows the rest of the line on an unterminated quote', () => { + expect(tokenizeArgs('a "unterminated rest')).toEqual(['a', 'unterminated rest']) + }) + + it('preserves an empty quoted argument', () => { + expect(tokenizeArgs('a "" b')).toEqual(['a', '', 'b']) + }) +}) + +describe('parseSlashInput', () => { + it('returns null for non-slash prompts', () => { + expect(parseSlashInput('just some text')).toBeNull() + expect(parseSlashInput('')).toBeNull() + expect(parseSlashInput(' ')).toBeNull() + }) + + it('treats a lone slash as invalid', () => { + expect(parseSlashInput('/')).toBeNull() + expect(parseSlashInput('/ ')).toBeNull() + }) + + it('parses a bare id with no args', () => { + expect(parseSlashInput('/lint')).toEqual({ id: 'lint', args: [], rest: '' }) + }) + + it('parses args and keeps the raw remainder', () => { + expect(parseSlashInput(' /fixme crash src/a.ts ')).toEqual({ + id: 'fixme', + args: ['crash', 'src/a.ts'], + rest: 'crash src/a.ts', + }) + }) + + it('keeps quotes in the raw remainder but strips them from tokens', () => { + expect(parseSlashInput('/revue foo.ts "gestion des erreurs"')).toEqual({ + id: 'revue', + args: ['foo.ts', 'gestion des erreurs'], + rest: 'foo.ts "gestion des erreurs"', + }) + }) + + it('keeps slashes inside args (e.g. file paths)', () => { + expect(parseSlashInput('/run vitest run src/x.test.ts')?.args).toEqual(['vitest', 'run', 'src/x.test.ts']) + }) +}) + +describe('extractTemplateParams', () => { + it('returns placeholders in order of first occurrence, deduplicated', () => { + expect(extractTemplateParams('{{b}} then {{a}} then {{b}}')).toEqual(['b', 'a']) + }) + + it('returns nothing when there are no placeholders', () => { + expect(extractTemplateParams('plain prompt')).toEqual([]) + }) +}) + +describe('positionalTemplateParams', () => { + it('excludes the whole-line placeholder', () => { + expect(positionalTemplateParams('{{file}} — {{ARGUMENTS}}')).toEqual(['file']) + }) + + it('does not treat a lowercase lookalike as special', () => { + expect(positionalTemplateParams('{{arguments}}')).toEqual(['arguments']) + }) +}) + +describe('templateParamHints', () => { + it('keeps positional order and moves ARGUMENTS last', () => { + expect(templateParamHints('{{ARGUMENTS}} after {{file}} and {{angle}}')).toEqual(['file', 'angle', ARGUMENTS_PARAM]) + }) + + it('omits ARGUMENTS when the template does not use it', () => { + expect(templateParamHints('{{a}} {{b}}')).toEqual(['a', 'b']) + }) +}) + +describe('resolveTemplateParams', () => { + it('fills positional placeholders in order of appearance', () => { + const { params, unfilledParams } = resolveTemplateParams('Fix {{issue}} in {{file}}', ['crash', 'a.ts']) + expect(params).toEqual({ issue: 'crash', file: 'a.ts' }) + expect(unfilledParams).toEqual([]) + }) + + it('reports placeholders with nothing to fill them', () => { + const { params, unfilledParams } = resolveTemplateParams('Fix {{issue}} in {{file}}', ['crash']) + expect(params).toEqual({ issue: 'crash' }) + expect(unfilledParams).toEqual(['file']) + }) + + it('gives ARGUMENTS the raw remainder without consuming a positional slot', () => { + const { params, unfilledParams } = resolveTemplateParams( + 'Review {{file}}: {{ARGUMENTS}}', + ['a.ts', 'be', 'harsh'], + 'a.ts be harsh', + ) + expect(params).toEqual({ file: 'a.ts', [ARGUMENTS_PARAM]: 'a.ts be harsh' }) + expect(unfilledParams).toEqual([]) + }) + + it('reports ARGUMENTS as unfilled when nothing was typed after the id', () => { + const { params, unfilledParams } = resolveTemplateParams('Do {{ARGUMENTS}}', [], '') + expect(params).toEqual({}) + expect(unfilledParams).toEqual([ARGUMENTS_PARAM]) + }) + + it('maps positionally as before when no ARGUMENTS placeholder is present', () => { + const { params } = resolveTemplateParams('{{a}} {{b}}', ['1', '2'], '1 2') + expect(params).toEqual({ a: '1', b: '2' }) + }) +}) + +describe('applyTemplateParams', () => { + it('replaces every occurrence of each placeholder', () => { + expect(applyTemplateParams('{{a}} and {{a}} and {{b}}', { a: 'x', b: 'y' })).toBe('x and x and y') + }) + + it('leaves placeholders without a value untouched', () => { + expect(applyTemplateParams('{{a}} {{b}}', { a: 'x' })).toBe('x {{b}}') + }) +}) + +describe('expandCommandPrompt', () => { + it('expands a multi-word quoted argument into one placeholder', () => { + const input = parseSlashInput('/revue foo.ts "gestion des erreurs"')! + const { prompt, unfilledParams } = expandCommandPrompt( + 'Relis {{fichier}} en te concentrant sur {{angle}}.', + input.args, + input.rest, + ) + expect(prompt).toBe('Relis foo.ts en te concentrant sur gestion des erreurs.') + expect(unfilledParams).toEqual([]) + }) + + it('expands ARGUMENTS with everything typed after the id', () => { + const input = parseSlashInput('/note remember to rerun the flaky test')! + const { prompt, unfilledParams } = expandCommandPrompt('Add a note: {{ARGUMENTS}}', input.args, input.rest) + expect(prompt).toBe('Add a note: remember to rerun the flaky test') + expect(unfilledParams).toEqual([]) + }) + + it('leaves an unfilled placeholder in place and reports it', () => { + const { prompt, unfilledParams } = expandCommandPrompt('Fix {{issue}} in {{file}}', ['crash'], 'crash') + expect(prompt).toBe('Fix crash in {{file}}') + expect(unfilledParams).toEqual(['file']) + }) + + it('works with no arguments at all', () => { + expect(expandCommandPrompt('plain prompt', [], '')).toEqual({ prompt: 'plain prompt', unfilledParams: [] }) + }) +}) diff --git a/src/shared/slash-args.ts b/src/shared/slash-args.ts new file mode 100644 index 00000000..faf56ddd --- /dev/null +++ b/src/shared/slash-args.ts @@ -0,0 +1,166 @@ +/** + * Slash-command argument parsing. + * + * Shared by the chat composer (web) and the task board's server-side slash + * resolution, so a command typed in chat and the same command seeded into a + * task expand identically. + * + * Two argument forms: + * - positional — `{{name}}` placeholders filled by order of first appearance + * - whole-line — `{{ARGUMENTS}}` receives everything typed after the id, + * verbatim, and never consumes a positional slot + */ + +/** Placeholder that captures the raw remainder of the line. */ +export const ARGUMENTS_PARAM = 'ARGUMENTS' + +export interface SlashInput { + id: string + /** Tokenized arguments, quotes honoured and stripped. */ + args: string[] + /** Everything after the id, verbatim and trimmed. Feeds `{{ARGUMENTS}}`. */ + rest: string +} + +/** + * Split a command line into tokens, treating a `"…"` or `'…'` run as one + * token so multi-word arguments survive. Inside double quotes a backslash + * escapes the next character; single quotes are literal, as in POSIX shells. + * An unterminated quote swallows the rest of the line rather than failing — + * a half-typed argument should still do something sensible. + */ +export function tokenizeArgs(line: string): string[] { + const tokens: string[] = [] + let current = '' + let started = false + let quote: '"' | "'" | null = null + + for (let i = 0; i < line.length; i++) { + const char = line[i]! + + if (quote === '"' && char === '\\' && i + 1 < line.length) { + current += line[++i]! + continue + } + + if (quote) { + if (char === quote) quote = null + else current += char + continue + } + + if (char === '"' || char === "'") { + quote = char + started = true + continue + } + + if (/\s/.test(char)) { + if (started) { + tokens.push(current) + current = '' + started = false + } + continue + } + + current += char + started = true + } + + if (started) tokens.push(current) + return tokens +} + +/** + * Split a prompt into a slash id, its tokenized args, and the raw remainder. + * Returns null when the prompt is not a slash invocation (or is a bare "/"). + */ +export function parseSlashInput(prompt: string): SlashInput | null { + const trimmed = prompt.trim() + if (!trimmed.startsWith('/')) return null + + const body = trimmed.slice(1) + const separator = body.search(/\s/) + const id = separator === -1 ? body : body.slice(0, separator) + if (!id) return null + + const rest = separator === -1 ? '' : body.slice(separator).trim() + return { id, args: tokenizeArgs(rest), rest } +} + +/** Named template placeholders (`{{name}}`) in order of first occurrence, deduplicated. */ +export function extractTemplateParams(template: string): string[] { + const seen: string[] = [] + const regex = /\{\{(\w+)\}\}/g + let match: RegExpExecArray | null + while ((match = regex.exec(template)) !== null) { + const key = match[1]! + if (!seen.includes(key)) seen.push(key) + } + return seen +} + +/** Placeholders that take a positional argument — every one except `{{ARGUMENTS}}`. */ +export function positionalTemplateParams(template: string): string[] { + return extractTemplateParams(template).filter((name) => name !== ARGUMENTS_PARAM) +} + +/** + * Resolve a template's placeholders from a parsed invocation. Positional + * placeholders take tokens in order; `{{ARGUMENTS}}` takes `rest` verbatim. + * A placeholder with nothing to fill it is reported rather than substituted, + * so callers can prompt for it instead of shipping `{{name}}` to the model. + */ +export function resolveTemplateParams( + template: string, + args: string[], + rest = '', +): { params: Record; unfilledParams: string[] } { + const names = extractTemplateParams(template) + const params: Record = {} + + names + .filter((name) => name !== ARGUMENTS_PARAM) + .forEach((name, index) => { + const value = args[index] + if (value !== undefined) params[name] = value + }) + + if (names.includes(ARGUMENTS_PARAM) && rest) params[ARGUMENTS_PARAM] = rest + + return { params, unfilledParams: names.filter((name) => !(name in params)) } +} + +/** + * Placeholder names for inline composer hints: positional slots in the order + * they must be typed, then `{{ARGUMENTS}}` last since it soaks up whatever + * follows them. + */ +export function templateParamHints(template: string): string[] { + const names = extractTemplateParams(template) + const positional = names.filter((name) => name !== ARGUMENTS_PARAM) + return names.includes(ARGUMENTS_PARAM) ? [...positional, ARGUMENTS_PARAM] : positional +} + +/** Replace `{{name}}` placeholders with the supplied values. */ +export function applyTemplateParams(template: string, params: Record): string { + let prompt = template + for (const [key, value] of Object.entries(params)) { + prompt = prompt.replaceAll(`{{${key}}}`, value) + } + return prompt +} + +/** + * Expand a command template against an invocation's arguments, reporting any + * placeholder left unfilled. + */ +export function expandCommandPrompt( + template: string, + args: string[], + rest = '', +): { prompt: string; unfilledParams: string[] } { + const { params, unfilledParams } = resolveTemplateParams(template, args, rest) + return { prompt: applyTemplateParams(template, params), unfilledParams } +} diff --git a/web/src/components/plan/ChatInput.slash.test.tsx b/web/src/components/plan/ChatInput.slash.test.tsx index 2d10401a..6f0e220d 100644 --- a/web/src/components/plan/ChatInput.slash.test.tsx +++ b/web/src/components/plan/ChatInput.slash.test.tsx @@ -27,12 +27,17 @@ describe('parseSlashCommand', () => { it('parses /pr-review 157 into workflow and params', () => { const result = parseSlashCommand('/pr-review 157', workflows) - expect(result).toEqual({ workflowId: 'pr-review', params: { pr_number: '157' } }) + expect(result).toEqual({ workflowId: 'pr-review', params: { pr_number: '157' }, args: ['157'], rest: '157' }) }) it('maps positional args by parameter position', () => { const result = parseSlashCommand('/pr-review 42 fix-bug', workflows) - expect(result).toEqual({ workflowId: 'pr-review', params: { pr_number: '42', pr_title: 'fix-bug' } }) + expect(result).toEqual({ + workflowId: 'pr-review', + params: { pr_number: '42', pr_title: 'fix-bug' }, + args: ['42', 'fix-bug'], + rest: '42 fix-bug', + }) }) it('returns null for non-slash input', () => { @@ -49,12 +54,17 @@ describe('parseSlashCommand', () => { it('handles workflow without parameter definitions', () => { const result = parseSlashCommand('/simple foo bar', workflows) - expect(result).toEqual({ workflowId: 'simple', params: { '0': 'foo', '1': 'bar' } }) + expect(result).toEqual({ + workflowId: 'simple', + params: { '0': 'foo', '1': 'bar' }, + args: ['foo', 'bar'], + rest: 'foo bar', + }) }) it('handles extra args beyond defined parameters', () => { const result = parseSlashCommand('/pr-review 42', workflows) - expect(result).toEqual({ workflowId: 'pr-review', params: { pr_number: '42' } }) + expect(result).toEqual({ workflowId: 'pr-review', params: { pr_number: '42' }, args: ['42'], rest: '42' }) }) }) @@ -93,18 +103,38 @@ describe('parseSlashCommand with commands', () => { it('matches a command by ID', () => { const result = parseSlashCommand('/review arg1 arg2', workflows, commands) - expect(result).toEqual({ commandId: 'review', params: { '0': 'arg1', '1': 'arg2' } }) + expect(result).toEqual({ + commandId: 'review', + params: { '0': 'arg1', '1': 'arg2' }, + args: ['arg1', 'arg2'], + rest: 'arg1 arg2', + }) }) it('returns null for unknown command', () => { expect(parseSlashCommand('/nonexistent', workflows, commands)).toBeNull() }) + it('keeps a quoted multi-word argument as one token', () => { + const result = parseSlashCommand('/review src/a.ts "gestion des erreurs"', workflows, commands) + expect(result).toEqual({ + commandId: 'review', + params: { '0': 'src/a.ts', '1': 'gestion des erreurs' }, + args: ['src/a.ts', 'gestion des erreurs'], + rest: 'src/a.ts "gestion des erreurs"', + }) + }) + + it('exposes the raw remainder for {{ARGUMENTS}}', () => { + const result = parseSlashCommand('/summarize the whole thread please', workflows, commands) + expect(result?.rest).toBe('the whole thread please') + }) + it('workflow takes priority over command with same ID', () => { const wf: WorkflowInfo[] = [{ id: 'review', name: 'Review WF', scope: 'builtin' }] const cmds: CommandInfo[] = [{ id: 'review', name: 'Review CMD' }] const result = parseSlashCommand('/review arg', wf, cmds) - expect(result).toEqual({ workflowId: 'review', params: { '0': 'arg' } }) + expect(result).toEqual({ workflowId: 'review', params: { '0': 'arg' }, args: ['arg'], rest: 'arg' }) }) }) @@ -324,6 +354,33 @@ describe('ChatInput slash command integration', () => { expect(mockLaunchWorkflow).not.toHaveBeenCalled() }) + it('feeds a quoted multi-word argument into the command template', async () => { + await commandsResource.refresh('/tmp') + const setInput = vi.fn() + const onSendCommand = vi.fn() + renderChatInput({ input: '/review "157 et 158"', setInput, onSendCommand }) + + fireEvent.click(screen.getByTestId('chat-send-button')) + + await waitFor(() => { + expect(onSendCommand).toHaveBeenCalledWith('Please review PR 157 et 158', 'builder') + }) + expect(mockSendMessage).not.toHaveBeenCalled() + }) + + it('leaves an unfilled placeholder in place so the params modal can ask for it', async () => { + await commandsResource.refresh('/tmp') + const setInput = vi.fn() + const onSendCommand = vi.fn() + renderChatInput({ input: '/review', setInput, onSendCommand }) + + fireEvent.click(screen.getByTestId('chat-send-button')) + + await waitFor(() => { + expect(onSendCommand).toHaveBeenCalledWith('Please review PR {{pr_number}}', 'builder') + }) + }) + it('shows error for missing required params', () => { const setInput = vi.fn() const setErrorMessage = vi.fn() diff --git a/web/src/components/plan/ChatInput.tsx b/web/src/components/plan/ChatInput.tsx index ed9ad99a..b5e3f4bc 100644 --- a/web/src/components/plan/ChatInput.tsx +++ b/web/src/components/plan/ChatInput.tsx @@ -6,7 +6,8 @@ import { useResource } from '../../hooks/useResource' import { useWorkflows } from '../../hooks/useWorkflows' import { commandsResource, commandResource } from '../../lib/resources' import { authFetch } from '../../lib/api' -import { parseSlashCommand, extractTemplateParams } from '../../lib/parse-slash-command' +import { parseSlashCommand, tokenizeArgs } from '../../lib/parse-slash-command' +import { expandCommandPrompt } from '@shared/slash-args.js' import { insertSuggestionAtCursor, focusTextareaAt, resolveSlashParamIds } from '../../lib/composer-utils' import { resolveWorkflowForLaunch } from '../../lib/workflow-scope' import { dedupById } from '../../lib/modal-utils' @@ -439,18 +440,10 @@ export function ChatInput({ // Fetch command, resolve params, send as message commandResource.refresh(slashResult.commandId, workdir).then((full) => { if (full) { - // Map positional args to named params by order of appearance in the prompt - const paramNames = extractTemplateParams(full.prompt) - const namedParams: Record = {} - for (const [posKey, value] of Object.entries(slashResult.params)) { - const idx = parseInt(posKey, 10) - const name = paramNames[idx] - if (name) namedParams[name] = value - } - let prompt = full.prompt - for (const [key, value] of Object.entries(namedParams)) { - prompt = prompt.replaceAll(`{{${key}}}`, value) - } + // Positional args fill {{name}} by order of appearance; {{ARGUMENTS}} + // takes the raw remainder. Anything left unfilled reaches + // onSendCommand as a placeholder, which opens the params modal. + const { prompt } = expandCommandPrompt(full.prompt, slashResult.args, slashResult.rest) onSendCommand(prompt, full.metadata.agentMode) clearInput() } @@ -656,10 +649,9 @@ export function ChatInput({ /> {activeSlashParams.length > 0 && (() => { - // Count space-separated args after the last /command + // Count args typed after the /command, quoted runs counting once const match = input.match(/\/(\w+)\s+(.*)$/) - const args = match ? match[2]!.trim().split(/\s+/) : [] - const filledCount = args.filter(Boolean).length + const filledCount = match ? tokenizeArgs(match[2]!).length : 0 const nextParam = activeSlashParams[filledCount] if (!nextParam) return null return ( diff --git a/web/src/components/settings/CommandsModal.tsx b/web/src/components/settings/CommandsModal.tsx index 588cf693..af9dda11 100644 --- a/web/src/components/settings/CommandsModal.tsx +++ b/web/src/components/settings/CommandsModal.tsx @@ -422,6 +422,12 @@ export function CommandsModal({ isOpen, onClose, initialEditId, projectDir }: Co })} className="h-80 w-full px-3 py-2 bg-bg-tertiary border border-border rounded text-sm font-mono resize-y focus:outline-none focus:ring-1 focus:ring-accent-primary" /> +

+ {t({ + en: 'Arguments: {{name}} takes one value in order (/cmd first second), {{ARGUMENTS}} takes everything typed after the command. Quote a value to keep it whole: /cmd "two words". Anything left unfilled is asked for before sending.', + fr: 'Arguments : {{nom}} prend une valeur dans l’ordre (/cmd premier second), {{ARGUMENTS}} prend tout ce qui suit la commande. Mettez une valeur entre guillemets pour la garder entière : /cmd « deux mots ». Ce qui reste vide vous sera demandé avant l’envoi.', + })} +

diff --git a/web/src/lib/parse-slash-command.ts b/web/src/lib/parse-slash-command.ts index bed44379..6d31695f 100644 --- a/web/src/lib/parse-slash-command.ts +++ b/web/src/lib/parse-slash-command.ts @@ -1,4 +1,7 @@ import type { WorkflowParameter, WorkflowScope } from '@shared/types.js' +import { parseSlashInput } from '@shared/slash-args.js' + +export { ARGUMENTS_PARAM, extractTemplateParams, tokenizeArgs } from '@shared/slash-args.js' export interface WorkflowInfo { id: string @@ -18,32 +21,17 @@ export interface SlashCommandResult { workflowId?: string commandId?: string params: Record -} - -/** - * Extract template parameter placeholders ({{name}}) from a template string. - * Returns them in order of first occurrence, deduplicated. - */ -export function extractTemplateParams(template: string): string[] { - const seen = new Set() - const result: string[] = [] - const regex = /\{\{(\w+)\}\}/g - let match: RegExpExecArray | null - while ((match = regex.exec(template)) !== null) { - const key = match[1]! - if (!seen.has(key)) { - seen.add(key) - result.push(key) - } - } - return result + /** Tokenized arguments, quoted runs kept whole. */ + args: string[] + /** Everything typed after the id, verbatim — feeds `{{ARGUMENTS}}`. */ + rest: string } /** * Legacy alias — use extractTemplateParams instead. * @deprecated */ -export const extractPositionalParams = extractTemplateParams +export { extractTemplateParams as extractPositionalParams } from '@shared/slash-args.js' /** * Parse a slash command from chat input. @@ -54,14 +42,10 @@ export function parseSlashCommand( workflows: WorkflowInfo[], commands?: CommandInfo[], ): SlashCommandResult | null { - const trimmed = input.trim() - if (!trimmed.startsWith('/')) return null - - const parts = trimmed.slice(1).split(/\s+/) - const id = parts[0] - if (!id) return null + const parsed = parseSlashInput(input) + if (!parsed) return null - const args = parts.slice(1) + const { id, args, rest } = parsed const params: Record = {} // Try workflow first @@ -79,7 +63,7 @@ export function parseSlashCommand( params[String(i)] = arg }) } - return { workflowId: id, params } + return { workflowId: id, params, args, rest } } // Then try command @@ -89,7 +73,7 @@ export function parseSlashCommand( args.forEach((arg, i) => { params[String(i)] = arg }) - return { commandId: id, params } + return { commandId: id, params, args, rest } } }