anomalyco/opencode
1
0
Fork 0
mirror of https://github.com/anomalyco/opencode.git synced 2026-07-22 02:05:26 -04:00
Code Issues 5.9k Packages Projects Releases 19 Wiki Activity

fileRegex for @ file references within custom commands is far too inclusive #1454

New Issue
Closed
opened 2026-02-16 17:31:03 -05:00 by yindo · 12 comments
yindo commented 2026-02-16 17:31:03 -05:00
Owner
Copy Link
Copy Source

Originally created by @aspiers on GitHub (Aug 23, 2025).

Originally assigned to: @thdxr on GitHub.

I'm trying to write a custom /commit command, and I have as part of it:

   - Adds a "Co-authored-by:" footer which clarifies which AI agent
     helped create this commit, using an appropriate `noreply@...`
     email address.

This causes opencode to attempt to find a file called ... which is clearly wrong! I see that the regex used doesn't take into account what precedes the regex:

https://github.com/sst/opencode/blob/3706b2bca7bb2e813391dad35a795fd03e36f905/packages/opencode/src/session/index.ts#L1170

I think a reasonable solution is to use a negative look-behind assertion: https://regex101.com/r/F6KAgH/1

I will submit a PR shortly.

Originally created by @aspiers on GitHub (Aug 23, 2025). Originally assigned to: @thdxr on GitHub. I'm trying to write a custom `/commit` command, and I have as part of it: ``` - Adds a "Co-authored-by:" footer which clarifies which AI agent helped create this commit, using an appropriate `noreply@...` email address. ``` This causes opencode to attempt to find a file called `...` which is clearly wrong! I see that the regex used doesn't take into account what precedes the regex: https://github.com/sst/opencode/blob/3706b2bca7bb2e813391dad35a795fd03e36f905/packages/opencode/src/session/index.ts#L1170 I think a reasonable solution is to use a negative look-behind assertion: https://regex101.com/r/F6KAgH/1 I will submit a PR shortly.
yindo closed this issue 2026-02-16 17:31:03 -05:00
yindo commented 2026-02-16 17:31:03 -05:00
Author
Owner
Copy Link
Copy Source

@aspiers commented on GitHub (Aug 23, 2025):

Found a bunch of other problems - latest version of the regex: https://regex101.com/r/F6KAgH/

@aspiers commented on GitHub (Aug 23, 2025): Found a bunch of other problems - latest version of the regex: https://regex101.com/r/F6KAgH/
yindo commented 2026-02-16 17:31:03 -05:00
Author
Owner
Copy Link
Copy Source

@aspiers commented on GitHub (Aug 23, 2025):

@rekram1-node I'm already working on this as mentioned here and in Discord - not sure why you've self-assigned this?

@aspiers commented on GitHub (Aug 23, 2025): @rekram1-node I'm already working on this as mentioned here and in Discord - not sure why you've self-assigned this?
yindo commented 2026-02-16 17:31:03 -05:00
Author
Owner
Copy Link
Copy Source

@rekram1-node commented on GitHub (Aug 23, 2025):

@aspiers I was just going to review it preliminarily then send to Dax if it looks good

@rekram1-node commented on GitHub (Aug 23, 2025): @aspiers I was just going to review it preliminarily then send to Dax if it looks good
yindo commented 2026-02-16 17:31:03 -05:00
Author
Owner
Copy Link
Copy Source

@rekram1-node commented on GitHub (Aug 23, 2025):

I just assign every issue to a maintainer

@rekram1-node commented on GitHub (Aug 23, 2025): I just assign every issue to a maintainer
yindo commented 2026-02-16 17:31:03 -05:00
Author
Owner
Copy Link
Copy Source

@aspiers commented on GitHub (Aug 23, 2025):

Oh OK thanks! I've just submitted #2203 as a draft.

@aspiers commented on GitHub (Aug 23, 2025): Oh OK thanks! I've just submitted #2203 as a draft.
yindo commented 2026-02-16 17:31:03 -05:00
Author
Owner
Copy Link
Copy Source

@art-shen commented on GitHub (Aug 30, 2025):

Copying from my issue (if it would be closed as duplicate):

  1. I think the current behavior of silent failure with only a cryptic message shown is a really bad experience. I propose to improve the failure information and clearly state in the conversation that the command failed to start because of the error. And specify that the specific file mentioned in the command wasn't loaded.

  2. I propose to add support for launching comments even when some mentioned file was not loaded. I think in many cases this would be a desirable behavior, but probably this should be optional.

  3. And finally, the problem itself specific to my case was that OpenCode is trying to interpret parts of the command prompt that it shouldn’t actually interpret in any way. I mean the code blocks and inline code in Markdown.

@rekram1-node, I honestly believe that regexes are a dead end here, and Markdown should be properly parsed to avoid such errors.

For example, the #2203 from @aspiers would fail again with the same problem on my example command from #2327 (would help with the first occurrence but still fail for the second). Because by design, regexes aren't suitable to properly handle languages like Markdown or any other complex syntax.

@art-shen commented on GitHub (Aug 30, 2025): Copying from my [issue](https://github.com/sst/opencode/issues/2327#issue-3368986303) (if it would be closed as duplicate): 1. I think the current behavior of silent failure with only a cryptic message shown is a really bad experience. I propose to improve the failure information and clearly state in the conversation that the command failed to start because of the error. And specify that the specific file mentioned in the command wasn't loaded. 2. I propose to add support for launching comments even when some mentioned file was not loaded. I think in many cases this would be a desirable behavior, but probably this should be optional. 3. And finally, the problem itself specific to my case was that OpenCode is trying to interpret parts of the command prompt that it shouldn’t actually interpret in any way. I mean the code blocks and inline code in Markdown. @rekram1-node, I honestly believe that **regexes are a dead end here**, and Markdown should be properly parsed to avoid such errors. For example, the #2203 from @aspiers would fail again with the same problem on my example command from #2327 (would help with the first occurrence but still fail for the second). Because by design, regexes aren't suitable to properly handle languages like Markdown or any other complex syntax.
yindo commented 2026-02-16 17:31:03 -05:00
Author
Owner
Copy Link
Copy Source

@rekram1-node commented on GitHub (Aug 30, 2025):

I was thinkijg the BE should just do a check on parsed files to see if they actually exist before trying to read them, if they dont exist we can toast an error or warning and handle gracefully or something

@rekram1-node commented on GitHub (Aug 30, 2025): I was thinkijg the BE should just do a check on parsed files to see if they actually exist before trying to read them, if they dont exist we can toast an error or warning and handle gracefully or something
yindo commented 2026-02-16 17:31:03 -05:00
Author
Owner
Copy Link
Copy Source

@aspiers commented on GitHub (Aug 30, 2025):

@rekram1-node, I honestly believe that regexes are a dead end here, and Markdown should be properly parsed to avoid such errors.

Parsing the Markdown into an AST is a nice idea which I support. However I disagree that regexes are a dead end - even if we have an AST, it is not sufficient for deciding what constitutes a reference to a file. In fact your second example demonstrates exactly this:

For example, the #2203 from @aspiers would fail again with the same problem on my example command from #2327 (would help with the first occurrence but still fail for the second). Because by design, regexes aren't suitable to properly handle languages like Markdown or any other complex syntax.

Your second example was:

Shows: `PR# ✨/📝 Title(80ch) @author branch(30ch) 💬comments/🧵reviews +adds/-dels date`

Even if we have an AST for parsed Markdown, there is no information in the AST which can reliably tell us whether @author is supposed to be a reference to a file or not, because the entire contents inside the ... will be treated as a single leaf in the tree, of which @author is just a substring. We could use an LLM to guess, but likely that's way overkill and there will be no solution better than rough heuristics:

  1. Extract anything from the string which looks like it might be a file path (via regex and/or LLM)
  2. Check whether that file path actually exists
  3. If it does, assume that it was intended to be a reference to a file

For bonus points, allow step 1 to be configurable by the user.

Given that using an LLM is going to be extremely heavyweight and probably won't significantly outperform regex matching, I have a pretty strong belief that it makes sense to use regexen.

Additionally, I postulate that my current PR is a significant improvement on the breakage currently in the code base - as we both experienced, the status quo is literally that custom commands can completely break opencode. So I would recommend the following:

  1. Adopt my regex approach as an imperfect but definitely helpful short-term improvement.
  2. As a follow-up, introduce Markdown parsing, and combine it with regexp matching.
@aspiers commented on GitHub (Aug 30, 2025): > [@rekram1-node](https://github.com/rekram1-node), I honestly believe that **regexes are a dead end here**, and Markdown should be properly parsed to avoid such errors. Parsing the Markdown into an AST is a nice idea which I support. However I disagree that regexes are a dead end - even if we have an AST, it is not sufficient for deciding what constitutes a reference to a file. In fact your second example demonstrates exactly this: > For example, the [#2203](https://github.com/sst/opencode/pull/2203) from [@aspiers](https://github.com/aspiers) would fail again with the same problem on my example command from [#2327](https://github.com/sst/opencode/issues/2327) (would help with the first occurrence but still fail for the second). Because by design, regexes aren't suitable to properly handle languages like Markdown or any other complex syntax. Your second example was: Shows: `PR# ✨/📝 Title(80ch) @author branch(30ch) 💬comments/🧵reviews +adds/-dels date` Even if we have an AST for parsed Markdown, there is no information in the AST which can reliably tell us whether `@author` is supposed to be a reference to a file or not, because the entire contents inside the `...` will be treated as a single leaf in the tree, of which `@author` is just a substring. We could use an LLM to guess, but likely that's way overkill and there will be no solution better than rough heuristics: 1. Extract anything from the string which looks like it might be a file path (via regex and/or LLM) 2. Check whether that file path actually exists 3. If it does, assume that it was intended to be a reference to a file For bonus points, allow step 1 to be configurable by the user. Given that using an LLM is going to be extremely heavyweight and probably won't significantly outperform regex matching, I have a pretty strong belief that it makes sense to use regexen. Additionally, I postulate that my current PR is a significant improvement on the breakage currently in the code base - as we both experienced, the status quo is literally that custom commands can completely break opencode. So I would recommend the following: 1. Adopt my regex approach as an imperfect but definitely helpful short-term improvement. 2. As a follow-up, introduce Markdown parsing, and combine it with regexp matching.
yindo commented 2026-02-16 17:31:03 -05:00
Author
Owner
Copy Link
Copy Source

@aspiers commented on GitHub (Aug 30, 2025):

I was thinkijg the BE should just do a check on parsed files to see if they actually exist before trying to read them, if they dont exist we can toast an error or warning and handle gracefully or something

It certainly can't be an error, as the @author example above demonstrates. A warning might make sense as long as there was an option like "I know that's not a valid @ reference to a file, and I don't care so don't warn me again" for the user to dismiss the warning. Given that custom command prompts don't change very often, this feels like about the right DWIM balance between asking the user annoying questions and accidentally failing to include files through typos etc.

@aspiers commented on GitHub (Aug 30, 2025): > I was thinkijg the BE should just do a check on parsed files to see if they actually exist before trying to read them, if they dont exist we can toast an error or warning and handle gracefully or something It certainly can't be an error, as the `@author` example above demonstrates. A warning might make sense as long as there was an option like "I know that's not a valid @ reference to a file, and I don't care so don't warn me again" for the user to dismiss the warning. Given that custom command prompts don't change very often, this feels like about the right DWIM balance between asking the user annoying questions and accidentally failing to include files through typos etc.
yindo commented 2026-02-16 17:31:03 -05:00
Author
Owner
Copy Link
Copy Source

@rekram1-node commented on GitHub (Aug 30, 2025):

I shouldve worded better typed that right when I woke up, but I generally agree w u @aspiers

@rekram1-node commented on GitHub (Aug 30, 2025): I shouldve worded better typed that right when I woke up, but I generally agree w u @aspiers
yindo commented 2026-02-16 17:31:04 -05:00
Author
Owner
Copy Link
Copy Source

@art-shen commented on GitHub (Aug 30, 2025):

@aspiers

Even if we have an AST for parsed Markdown, there is no information in the AST

On the contrary, in my example the full leaf with code block and/or inline code would be skipped entirely.

But I agree that even after parsing, the final filtration could still be efficiently done in two stages.
First with RegExp, then with actually checking if the file exists.

@art-shen commented on GitHub (Aug 30, 2025): @aspiers > Even if we have an AST for parsed Markdown, there is no information in the AST On the contrary, in my example the full leaf with code block and/or inline code would be skipped entirely. But I agree that even after parsing, the final filtration could still be efficiently done in two stages. First with RegExp, then with actually checking if the file exists.
yindo commented 2026-02-16 17:31:04 -05:00
Author
Owner
Copy Link
Copy Source

@aspiers commented on GitHub (Aug 30, 2025):

On the contrary, in my example the full leaf with code block and/or inline code would be skipped entirely.

Yes but that might not always be the desired behaviour. Even if it is for backticks, the @file reference might be wrapped in bold or italics, in which case it's very likely that extraction would be desired, and there you still need a regexp.

Please can we just agree to take this first step in the right direction, which solves a serious crash, does the right thing in many more cases than before, and allows a reasonable path open for further improvements? Let's not make the quest for perfection become the enemy of the good.

@aspiers commented on GitHub (Aug 30, 2025): > On the contrary, in my example the full leaf with code block and/or inline code would be skipped entirely. Yes but that might not always be the desired behaviour. Even if it is for backticks, the @file reference might be wrapped in bold or italics, in which case it's very likely that extraction would be desired, and there you still need a regexp. Please can we just agree to take this first step in the right direction, which solves a serious crash, does the right thing in many more cases than before, and allows a reasonable path open for further improvements? Let's not make the quest for perfection become the enemy of the good.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
acp
bad
bug
discussion
docs
documentation
enhancement
extension
good first issue
growth
help-wanted
mcp
model-problem
needs:compliance
nix
opentui
perf
pull-request
Mirrored from GitHub Pull Request
 tui
web
windows
zen
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: anomalyco/opencode#1454
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?