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

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.

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

@rekram1-node

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

@aspiers

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

I just assign every issue to a maintainer

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

@rekram1-node

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

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.
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.
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.

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, 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:

Extract anything from the string which looks like it might be a file path (via regex and/or LLM)
Check whether that file path actually exists
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:

Adopt my regex approach as an imperfect but definitely helpful short-term improvement.
As a follow-up, introduce Markdown parsing, and combine it with regexp matching.

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

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

@aspiers

@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.

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.

fix: stop false positives for @file references (anomalyco#2198)

Navigation Menu

Repository navigation

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

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Activity

aspiers commented on Aug 23, 2025

aspiers commented on Aug 23, 2025

rekram1-node commented on Aug 23, 2025

rekram1-node commented on Aug 23, 2025

aspiers commented on Aug 23, 2025

art-shen commented on Aug 30, 2025

rekram1-node commented on Aug 30, 2025

aspiers commented on Aug 30, 2025

aspiers commented on Aug 30, 2025

rekram1-node commented on Aug 30, 2025

art-shen commented on Aug 30, 2025

aspiers commented on Aug 30, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions