Repository navigation
fix: align leading-whitespace policy between slash_command compiler and runtime - #50928
Conversation
The compiler-generated activation conditions use startsWith(body, '/command') which requires the command at character zero. But check_command_position.cjs was calling text.trim() before matching, and slash_command_matcher.cjs also trimmed inside parseSlashCommand(). This caused a mismatch: a comment like " /reproduce args" would pass the runtime helper's check but the pre_activation/activation job conditions would skip the job entirely (no denial summary, no reaction). Fix: adopt a consistent character-zero policy end-to-end. - Remove trim() from parseSlashCommand in slash_command_matcher.cjs - Remove text.trim() from check_command_position.cjs - Update the test that expected leading whitespace to pass: it now fails - Add a compiler regression test that documents the character-zero contract Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Use trimStart() for the firstWord diagnostic variable so that when a comment has leading whitespace the log message shows the actual first token rather than an empty string. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Aligns slash-command matching on a character-zero policy across compiler and runtime checks.
Changes:
- Removes leading-whitespace normalization from slash-command matching.
- Improves rejection diagnostics while preserving raw matching.
- Adds runtime and compiler regression coverage.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/command_precision_test.go |
Tests raw-body compiler conditions. |
actions/setup/js/slash_command_matcher.cjs |
Enforces character-zero matching. |
actions/setup/js/check_command_position.test.cjs |
Updates leading-whitespace expectations. |
actions/setup/js/check_command_position.cjs |
Matches against untrimmed event bodies. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
| */ | ||
| function parseSlashCommand(text) { | ||
| const match = /^\/([a-zA-Z0-9][a-zA-Z0-9._-]*)(?=$|\s)/.exec(String(text).trim()); | ||
| const match = /^\/([a-zA-Z0-9][a-zA-Z0-9._-]*)(?=$|\s)/.exec(String(text)); |
|
@copilot Please address the remaining unresolved review thread (#50928 (comment)), refresh the branch if needed, and run the
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed the failing CJS assertion in d0da66f. The updated router test now expects leading-whitespace commands to be rejected, matching the character-zero runtime policy.
Addressed the unresolved parser regression in d0da66f. The router test now explicitly requires the command at character zero; targeted and required local validation pass. |
|
🎉 This pull request is included in a new release. Release: |
The compiler generates
startsWith(body, '/command...')conditions requiring the command at position zero, butcheck_command_position.cjscalledtext.trim()before matching. A comment like" /reproduce args"would pass the runtime helper but be silently rejected by the pre_activation/activation job condition — no agent ran, no denial summary was produced.Changes
slash_command_matcher.cjs— Remove.trim()fromparseSlashCommand; command must appear at character zero in the raw body.check_command_position.cjs— Removetext.trim()so matching uses the raw body, consistent with the compiled conditions. UsetrimStart()only for thefirstWorddiagnostic log variable so rejection messages remain readable.check_command_position.test.cjs— Update the leading-whitespace test to expectcommand_position_ok: "false"under the new policy.command_precision_test.go— Add a compiler regression test documenting the character-zero contract and asserting no body transform (e.g.trim() appears in generated conditions.The chosen policy is character-zero end-to-end: both the compiled activation predicate and the runtime helper now require the slash command to start at the first character of the comment body.
slash_commandjob gate rejects leading whitespace accepted by runtime matcher #50922