Improve Exie result presentation - #2482
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a70d7d1eef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (prepareAssistantContext) { | ||
| prepareAssistantContext(); | ||
| } else if (stack) { | ||
| assistantPageContext.setPageStack(stack); |
There was a problem hiding this comment.
When this card is rendered inside stack-detail-sheet.svelte for a stack with a latest event, handleEventLoaded has already installed that event as the overlay context, and AssistantPageContext.getContext prioritizes overlays over page state. Calling setPageStack here therefore cannot replace the event context, so clicking the stack-level Fix button submits a stack prompt with an event path and causes Exie to follow its current-event rule. Pass an overlay-aware preparation callback through this flow, as the removed sheet-header action did.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 324afad. I restored the sheet-owned stack preparation callback and threaded it through StackDetails and EventsOverview to the shared StackCard. Clicking the stack-level Fix action now replaces the sheet's event overlay with the same owner's stack overlay before asking Exie, so no event ID leaks into the stack prompt. The overlay regression, 13 focused assistant tests, full frontend validation, and the production frontend build all pass.
|
|
||
| return replaceOutsideProtectedMarkdown(content, (text) => { | ||
| const linksByLabel = new Map(links.map((link) => [link.label, link])); | ||
| const linkPattern = new RegExp(links.map((link) => escapeRegularExpression(link.label)).join('|'), 'g'); |
There was a problem hiding this comment.
When a returned project or stack has a short/common name that occurs inside other text, this unbounded alternation rewrites every substring rather than only a mention of that resource. For example, a project named API turns APIClient into [API](... )Client, and similarly can corrupt an unprotected Markdown URL. Match label boundaries or parsed Markdown text nodes before inserting the link.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 324afad. Resource labels now require Unicode-aware standalone boundaries, and bare absolute plus /next/... URLs are protected from replacement. Added regressions confirming APIClient is untouched, standalone API is linked, and labels inside both URL forms remain unchanged. The 13 focused assistant tests, full frontend validation, and the production frontend build all pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 324afadabb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| function replaceOutsideProtectedMarkdown(content: string, replace: (text: string) => string): string { | ||
| const protectedMarkdown = /(```[^\n]*\n[\s\S]*?```|~~~[^\n]*\n[\s\S]*?~~~|`+[^`\n]*`+|!?\[[^\]\n]*\]\([^\n)]*\)|https?:\/\/[^\s<]+|\/next(?:\/[^\s<]*)?)/g; |
There was a problem hiding this comment.
When Exie emits a valid reference-style link such as [Timeout expired][stack], this protection regex does not recognize it, so the subsequent replacement produces malformed nested Markdown like [[Timeout expired](/next/stack/new)][stack] and overrides/corrupts the existing link. The fresh evidence beyond the earlier link-boundary issue is that the new protection handles only inline [...]() links; reference and shortcut links remain exposed to rewriting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 4688c9a. The Markdown protection now preserves full-reference, collapsed-reference, and shortcut links before resource-label insertion, with a regression covering all three forms. The 14 focused assistant tests, full frontend validation, and production frontend build all pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4688c9a20d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (latestUserMessage is null || String.IsNullOrWhiteSpace(latestUserMessage.Content)) | ||
| return false; |
There was a problem hiding this comment.
When a suggested action has an innocuous label but a mutating prompt (for example, “Inspect details” paired with “Discard this stack”), the UI displays only the label and submits the hidden prompt, and this removed IsSuggestedAction guard now lets that prompt immediately authorize the write. This can arise from an erroneous or prompt-injected model response and causes a click that appears read-only to mutate stack data without confirmation; require the visible suggestion itself to disclose and match the exact write, or retain a confirmation boundary for suggested mutations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 48637e6. Suggested-action clicks now send the visible label with the hidden prompt, and the server requires both to independently match the same write operation and arguments before authorizing it. A missing or innocuous label fails closed and returns to the confirmation boundary. The regression covers the Inspect details/snooze mismatch and missing-label clients; 93 focused authorization tests, 4 OpenAPI snapshot tests, 18 focused frontend tests, the backend build, full frontend validation, and the production frontend build all pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48637e6d79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return false; | ||
| } | ||
|
|
||
| string visibleRequest = $"{label.Trim()} this stack"; |
There was a problem hiding this comment.
When a model-supplied action label names a different target—for example, visible label Discard stack B, hidden prompt Discard this stack, while viewing stack A—this line appends this stack, and the visible-label check validates only the operation and arguments, not the named target. The hidden prompt then passes the full target checks and stack A is mutated even though the user clicked an action naming B. Fresh evidence after the earlier visible-label fix is that this synthetic target bypasses HasAmbiguousStackTargets and the exclusion checks; validate the label's own target against the tool arguments and current path instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in fc61986. The visible label and hidden prompt now go through the same stack-target validation. A label that explicitly names another stack is rejected unless it matches the tool arguments or current route; only labels with no named target may inherit the current stack context. Added regressions for both Discard stack other-stack (rejected) and Discard stack current-stack (authorized). The backend build and all 95 focused AssistantService authorization tests pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc61986c53
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return false; | ||
| } | ||
|
|
||
| string visibleRequest = $"{label.Trim()} this stack"; |
There was a problem hiding this comment.
When a suggested write label identifies another target without the exact stack <id> syntax—such as visible label Discard the other stack or Discard other-stack, hidden prompt Please discard this stack, and tool arguments for the current stack—GetNamedStackTargets finds no conflicting target, and appending this stack makes the label authorize discarding the current stack. Fresh evidence beyond the earlier target finding is that the new target parser only recognizes identifiers following stack or issue, leaving these natural label forms able to authorize a mutation different from what the user clicked; require the label itself to unambiguously identify the current target instead of synthesizing one.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in d6c6aba. Only canonical operation-only labels such as Mark as fixed or Snooze for 7 days may now inherit the current-stack context. Labels with extra target wording must explicitly match the current/tool target; regression coverage rejects both Discard the other stack and Discard other-stack while preserving compact legitimate actions. The backend build and all 98 focused AssistantServiceTests pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6c6aba9a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| && (referencesCurrentStack || (allowImplicitCurrentStack && !namesStackTarget)); | ||
| } | ||
|
|
||
| return ContainsExactToken(message, requestedStackId) |
There was a problem hiding this comment.
When a suggested reference-link action includes the current stack ID inside its URL, this accepts that incidental occurrence as proof that the visible label targets the current stack. For example, a label like Remove reference https://example.test/<current-stack-id> from the other stack, paired with a hidden current-stack prompt and matching tool arguments, passes the URL and operation checks and removes the link from the current stack despite the visible label naming another target. The fresh evidence after the prior target-label fixes is that ContainsExactToken searches the entire label, including the URL; require a semantic stack reference rather than any token occurrence.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 0847b64. Stack-target validation now removes absolute URLs before checking target IDs, current-stack phrases, ambiguity, and exclusions. An ID embedded in the reference URL can no longer authorize the current stack, while the canonical compact Remove reference <url> action still inherits current-stack context. Added regressions for both cases; the backend build and all 100 focused AssistantServiceTests pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0847b640d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| && (referencesCurrentStack || (allowImplicitCurrentStack && !namesStackTarget)); | ||
| } | ||
|
|
||
| return ContainsExactToken(targetText, requestedStackId) |
There was a problem hiding this comment.
When a model-supplied label names another target but also contains the requested ID elsewhere, this unconditional token match authorizes the write. For example, Discard stack X 0123456789abcdef01234567 fits the 40-character label limit and passes for tool arguments targeting that ObjectId, even though the visible command names stack X; a matching hidden prompt then discards the current stack. Fresh evidence after the URL-only fix is that incidental IDs outside URLs still bypass the named-target check, so require every explicitly named stack target to match the tool/current target rather than accepting any occurrence of the requested ID.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 7e9627b. Every explicitly named stack target must now match the tool/current target before a write can be authorized. Noncanonical visible labels also require a semantic target (this/current stack or stack <matching-id>), so a stray matching ID cannot override stack X; natural-language exclusions such as other stack fail closed as well. Added regressions for both the conflicting named-target case and the attempted other stack <matching-id> variant. The backend build and all 102 focused AssistantServiceTests pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e9627b028
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| string[] namedTargets = GetNamedStackTargets(targetText).Distinct(StringComparer.OrdinalIgnoreCase).ToArray(); | ||
| return namedTargets.Length > 0 | ||
| ? namedTargets.All(target => String.Equals(target, expectedStackId, StringComparison.OrdinalIgnoreCase)) | ||
| : !String.IsNullOrWhiteSpace(currentStackId) && ReferencesCurrentStackByPhrase(targetText); |
There was a problem hiding this comment.
When a model-supplied action label uses the current-stack phrase possessively—for example, visible label Discard this stack's duplicate, hidden prompt Please discard this stack, and arguments targeting the current stack—this branch treats the substring this stack as an explicit matching target. The status check then accepts the label and discards the current stack even though the clicked action names its duplicate. Fresh evidence beyond the earlier target-label cases is that possessive or extended current-stack phrases bypass the new semantic target checks; require the phrase to denote the mutation's direct target rather than merely occur in the label.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in ea06afd. Noncanonical visible labels can no longer use a generic this/current stack substring as their target; they must explicitly name the matching stack ID. Current-stack phrase recognition also rejects possessive and hyphen-extended forms, protecting both suggested-action and direct-message authorization paths. Regressions cover this stack's duplicate and this stack-related duplicate in both paths. The backend build and all 106 focused AssistantServiceTests pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea06afd820
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private static IEnumerable<string> GetNamedStackTargets(string message) | ||
| => Regex.Matches(message, @"\b(?:stack|issue)\s+(?<target>[A-Za-z0-9][A-Za-z0-9_-]*)\b", RegexOptions.IgnoreCase) | ||
| .Select(match => match.Groups["target"].Value.ToLowerInvariant()) | ||
| .Where(target => target is not ("this" or "current" or "the" or "fixed" or "ignored" or "discarded" or "open" or "critical" or "not" or "as" or "to" or "with" or "for" or "until")); |
There was a problem hiding this comment.
When a suggested label names the current 24-character ID possessively, such as Discard stack 0123456789abcdef01234567's (exactly the 40-character label limit), this regex extracts the ID and ContainsExactToken accepts the apostrophe as a token boundary. With a hidden prompt such as Discard this stack and matching arguments, the label therefore authorizes discarding the stack even though its visible target is possessive rather than the stack itself. Fresh evidence beyond the earlier current-stack possessive fix is that its suffix guard applies only to this/current stack, not named IDs; reject possessive or extended suffixes here as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 8d368f8. Named-target parsing and exact stack-ID matching now reject straight or curly apostrophe suffixes, so a possessive ID cannot be treated as the stack itself. The regression uses the exact 24-character ID / 40-character label case from the finding. The backend build and all 107 focused AssistantServiceTests pass.
| const linkPattern = new RegExp( | ||
| `(?<![\\p{L}\\p{N}_])(?:${links.map((link) => escapeRegularExpression(link.label)).join('|')})(?![\\p{L}\\p{N}_])`, | ||
| 'gu' |
There was a problem hiding this comment.
When a valid project or stack label contains only punctuation, these letter/number lookarounds provide no boundary protection. For example, a completed tool result for a project named - causes every Markdown list marker and every hyphen in a table separator to be replaced with a resource link, corrupting the rendered response. The fresh case not covered by the earlier boundary fix is that punctuation-only labels have no word characters on which those boundaries can operate; exclude such labels or match them only as explicit resource mentions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 8d368f8. Resource auto-link candidates must now contain at least one Unicode letter or number, so punctuation-only names are ignored instead of rewriting Markdown structure. Added a regression covering both a list marker and table separator. The 11 focused assistant-link tests and full frontend validation pass with zero errors or warnings.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d368f8867
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (latestUserMessage.IsSuggestedAction == true | ||
| && !HasVisibleSuggestedActionWriteIntent(latestUserMessage.SuggestedActionLabel, toolName, root, requestedStackId, currentStackId)) |
There was a problem hiding this comment.
When a write suggestion is generated on stack A and the user navigates to stack B before clicking it, assistant-panel.svelte retains the conversation across same-organization navigation but sends the current path. A canonical action such as Mark as fixed with the hidden prompt Please mark this stack fixed can then make the model omit stackId, and this branch validates both strings against B and authorizes mutating B even though the suggestion arose from A. Capture and validate the suggestion's creation target, or invalidate/reconfirm write suggestions when the page context changes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 641958a. Suggested actions are now stamped with the exact request path where they were created; clicking one sends that source path, and server-side write authorization requires it to equal the current request path. A suggestion created on stack A therefore cannot authorize a write after navigation to stack B. Added frontend capture/serialization coverage, a backend mismatched-path regression, updated the OpenAPI snapshot, and reran the focused tests plus full frontend validation.
| } | ||
|
|
||
| private static IEnumerable<string> GetNamedStackTargets(string message) | ||
| => Regex.Matches(message, @"\b(?:stack|issue)\s+(?<target>[A-Za-z0-9][A-Za-z0-9_-]*)\b(?!['’])", RegexOptions.IgnoreCase) |
There was a problem hiding this comment.
Fresh evidence after the possessive-target fix is that a visible label such as Discard stack 0123456789abcdef01234567/x—exactly the 40-character label limit—still passes because this regex treats / as the end of the named ID, while ContainsExactToken also accepts it as a boundary. With a hidden Discard this stack prompt and matching tool arguments, clicking this slash-qualified target discards the bare current stack; require the named ID to be a complete direct target rather than accepting identifier suffixes through other separators.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 641958a. Named-target parsing and exact-token matching now reject slash- and backslash-qualified suffixes in addition to possessive and hyphen suffixes. The regression uses an exact 24-character stack ID with /x, while remaining within the visible-label length limit. Focused assistant tests and full frontend validation pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 641958adb7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| private static IEnumerable<string> GetNamedStackTargets(string message) | ||
| => Regex.Matches(message, @"\b(?:stack|issue)\s+(?<target>[A-Za-z0-9][A-Za-z0-9_-]*)\b(?!['’/\\])", RegexOptions.IgnoreCase) |
There was a problem hiding this comment.
When a suggested label is Discard stack 0123456789abcdef01234567.x—which fits the 40-character limit—this regex extracts the bare ID, and ContainsExactToken also accepts . as a boundary. With a hidden Discard this stack prompt and matching tool arguments, the label therefore authorizes discarding the current stack even though the visible target is the qualified ...67.x. The fresh evidence beyond the existing slash-qualified case is that the new suffix guards still permit dot-qualified identifiers; require the named ID to terminate at whitespace or safe sentence punctuation rather than accepting arbitrary suffix separators.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in c360d8f. Named stack targets and exact stack-ID references now use a fail-closed terminator rule instead of a growing suffix blacklist: the target must end, be followed by whitespace, or be followed by sentence punctuation that itself ends the token. This rejects .x, /x, \x, possessive, and other qualified suffixes while preserving normal punctuation such as Discard stack <id>. Added both rejection and positive regressions; all 112 focused AssistantServiceTests pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c360d8f245
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| function replaceOutsideProtectedMarkdown(content: string, replace: (text: string) => string): string { | ||
| const protectedMarkdown = | ||
| /(```[^\n]*\n[\s\S]*?```|~~~[^\n]*\n[\s\S]*?~~~|`+[^`\n]*`+|!?\[[^\]\n]*\]\([^\n)]*\)|!?\[[^\]\n]*\]\s*\[[^\]\n]*\]|!?\[[^\]\n]*\]|https?:\/\/[^\s<]+|\/next(?:\/[^\s<]*)?)/g; |
There was a problem hiding this comment.
When an existing Markdown link has a valid destination containing balanced parentheses before a resource label, this alternative stops at the first ) and exposes the rest of the destination to replacement. For example, [docs](https://example.test/(guide)/API) with a returned resource named API becomes [docs](https://example.test/(guide)/[API](/next/project/api)), corrupting the existing link. Parse Markdown nodes or otherwise protect the complete balanced destination before inserting resource links.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 9c21e19. Inline Markdown links and images are now scanned with balanced label/destination depth (including escaped characters) and their complete ranges are protected before resource-link insertion. This preserves the exact [docs](https://example.test/(guide)/API) case instead of exposing the destination tail. The focused link suite passes all 12 tests, and full frontend validation passes with zero errors or warnings.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c21e1911a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| function replaceOutsideProtectedMarkdown(content: string, replace: (text: string) => string): string { | ||
| const protectedMarkdown = | ||
| /(```[^\n]*\n[\s\S]*?```|~~~[^\n]*\n[\s\S]*?~~~|`+[^`\n]*`+|!?\[[^\]\n]*\]\s*\[[^\]\n]*\]|!?\[[^\]\n]*\]|https?:\/\/[^\s<]+|\/next(?:\/[^\s<]*)?)/g; |
There was a problem hiding this comment.
When a response uses a valid four-space-indented code block containing a returned resource label, this protection regex does not recognize the block. For example, an indented API.connect() snippet with a project named API is rewritten to [API](/next/project/api).connect(); Markdown then displays that replacement literally as code, corrupting snippets users may copy. Protect indented code blocks as well, or perform replacements on parsed Markdown text nodes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 43cc25d. The protected Markdown ranges now include contiguous four-space- or tab-indented code lines, so resource labels inside indented snippets are never rewritten. The regression keeps API.connect() unchanged while still linking a later prose mention. All 14 focused link tests and full frontend validation pass.
| return replaceOutsideProtectedMarkdown(content, (text) => { | ||
| const linksByLabel = new Map(links.map((link) => [link.label, link])); | ||
| const linkPattern = new RegExp( | ||
| `(?<![\\p{L}\\p{N}_])(?:${links.map((link) => escapeRegularExpression(link.label)).join('|')})(?![\\p{L}\\p{N}_])`, |
There was a problem hiding this comment.
Fresh evidence after the earlier boundary fix is that the current lookarounds treat punctuation such as @ as a standalone boundary. If a returned project is named API, text such as API@example.com becomes [API](/next/project/api)@example.com, replacing the email's local part with an unrelated internal project link and preventing the address from behaving as a complete email link. Exclude email and punctuation-qualified identifiers, or insert links only into parsed text nodes that are semantic resource mentions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 43cc25d. Complete email-address ranges are now protected before resource-link insertion, preventing both local-part and domain labels from being rewritten. The regression preserves API@example.com while still linking a separate prose API mention. All 14 focused link tests and full frontend validation pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43cc25dff7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| function replaceOutsideProtectedMarkdown(content: string, replace: (text: string) => string): string { | ||
| const protectedMarkdown = | ||
| /(^(?:(?: {4}|\t)[^\r\n]*(?:\r?\n|$))+|```[^\n]*\n[\s\S]*?```|~~~[^\n]*\n[\s\S]*?~~~|`+[^`\n]*`+|!?\[[^\]\n]*\]\s*\[[^\]\n]*\]|!?\[[^\]\n]*\]|[A-Za-z0-9.!#$%&'*+/=?^_`{|}~-]+@[A-Za-z0-9-]+(?:\.[A-Za-z0-9-]+)*|https?:\/\/[^\s<]+|\/next(?:\/[^\s<]*)?)/gm; |
There was a problem hiding this comment.
When a valid Markdown code span uses a longer delimiter to contain a backtick, this alternative ends at the embedded backtick and exposes the remainder to resource replacement. For example, with a project named API, the span code API`` `` becomes `` ``code API , corrupting a snippet the user may copy. Scan code spans using their opening delimiter length so only a matching run closes the protected range.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 4ca9557. Inline code spans are now scanned by the length of their opening backtick run and protected only through a matching run, so shorter embedded runs remain inside the protected span. The exact code ` API case is covered while a later prose API is still linked. All 15 focused link tests and full frontend validation pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ca955700b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| function replaceOutsideProtectedMarkdown(content: string, replace: (text: string) => string): string { | ||
| const protectedMarkdown = | ||
| /(^(?:(?: {4}|\t)[^\r\n]*(?:\r?\n|$))+|```[^\n]*\n[\s\S]*?```|~~~[^\n]*\n[\s\S]*?~~~|!?\[[^\]\n]*\]\s*\[[^\]\n]*\]|!?\[[^\]\n]*\]|[A-Za-z0-9.!#$%&'*+/=?^_`{|}~-]+@[A-Za-z0-9-]+(?:\.[A-Za-z0-9-]+)*|https?:\/\/[^\s<]+|\/next(?:\/[^\s<]*)?)/gm; |
There was a problem hiding this comment.
When a fenced code block contains its three-character delimiter mid-line, this regex treats that occurrence as the closing fence even though Markdown only recognizes a closing fence at the start of a line. For example, in a block containing const marker = "```"; followed by API.connect(), a returned project named API causes the latter line to be rewritten as a link; the Markdown renderer still considers it code, so users see and copy the injected link syntax. Scan fenced blocks using line-aligned closing delimiters of the opening fence's length rather than this unanchored fixed-delimiter match.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in a0437ca. Fenced blocks are now scanned from a line-aligned opening fence to a line-aligned closing fence using the same delimiter character and at least the opening run length. Mid-line delimiter text therefore stays inside the protected block. The exact const marker = "```"; followed by API.connect() regression is covered; all 16 focused link tests and full frontend validation pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0437cae2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| function findFencedCodeRanges(content: string): Array<{ end: number; start: number }> { | ||
| const ranges: Array<{ end: number; start: number }> = []; | ||
| const openingFence = /^ {0,3}(?<delimiter>`{3,}|~{3,})[^\r\n]*(?:\r?\n|$)/gm; |
There was a problem hiding this comment.
When a response contains a fenced block inside a blockquote, such as > ```ts followed by > API.connect(), the Markdown renderer treats the contents as code after removing the > container markers, but this expression only recognizes fences preceded by zero to three spaces. A returned resource named API is therefore rewritten inside the code block, corrupting code users may copy. Make the protection container-aware or operate on parsed Markdown nodes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 5fba28b. Fenced-code scanning now recognizes Markdown blockquote container prefixes, requires the closing fence at the same blockquote depth, and preserves the entire quoted code block from resource-link insertion. The exact > ```ts / > API.connect() case is covered while a later prose API is still linked. All 17 focused assistant-link tests and full frontend validation pass with zero errors or warnings.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fba28b0ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| function findFencedCodeRanges(content: string): Array<{ end: number; start: number }> { | ||
| const ranges: Array<{ end: number; start: number }> = []; | ||
| const openingFence = /^(?<container>(?: {0,3}>[\t ]?)* {0,3})(?<delimiter>`{3,}|~{3,})[^\r\n]*(?:\r?\n|$)/gm; |
There was a problem hiding this comment.
After the blockquote-specific fix, fenced blocks inside Markdown lists remain unprotected because this opening pattern recognizes only indentation and > prefixes. For example, - ~~~ts\n API.connect()\n ~~~ is rendered as a fenced code block, but with a returned project named API, the replacement pass injects [API](/next/project/api) into the snippet because tilde fences are not rescued by the backtick-span scanner. Recognize list-container prefixes as well, or perform insertion on parsed Markdown text nodes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in b41b1c5. Fenced-code scanning now recognizes unordered and ordered list markers in addition to blockquote containers, derives the list content indentation for the closing fence, and protects the complete nested block from resource-link insertion. The exact - ~~~ts / API.connect() case is covered while a later prose API is still linked. All 18 focused assistant-link tests and full frontend validation pass with zero errors or warnings.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b41b1c5dc0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| function replaceOutsideProtectedMarkdown(content: string, replace: (text: string) => string): string { | ||
| const protectedMarkdown = | ||
| /(^(?:(?: {4}|\t)[^\r\n]*(?:\r?\n|$))+|!?\[[^\]\n]*\]\s*\[[^\]\n]*\]|!?\[[^\]\n]*\]|[A-Za-z0-9.!#$%&'*+/=?^_`{|}~-]+@[A-Za-z0-9-]+(?:\.[A-Za-z0-9-]+)*|https?:\/\/[^\s<]+|\/next(?:\/[^\s<]*)?)/gm; |
There was a problem hiding this comment.
Fresh evidence beyond the earlier top-level indented-code fix is that blockquote containers remain unhandled: with a project named API, valid Markdown such as > API.connect() is rendered as an indented code block after the > prefix is removed, but this regex does not protect it and rewrites the snippet to > [API](/next/project/api).connect(). Users therefore see and copy injected Markdown rather than the original code; make indented-code detection container-aware.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in b849f34. Indented-code protection now recognizes Markdown blockquote prefixes on each line, so > API.connect() remains an indented code block and cannot receive an injected resource link. The exact regression is covered while a later prose API is still linked. All 19 focused assistant-link tests and full frontend validation pass with zero errors or warnings.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b849f34ac5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| `(?<![\\p{L}\\p{N}_])(?:${links.map((link) => escapeRegularExpression(link.label)).join('|')})(?![\\p{L}\\p{N}_])`, | ||
| 'gu' |
There was a problem hiding this comment.
When a returned resource has a short name used as a hostname segment, the Unicode word boundaries still treat dots as standalone boundaries, and the protected ranges cover only absolute URLs and email addresses. For example, with a project named API, API.example.com becomes [API](/next/project/api).example.com, breaking the hostname and linking its first segment to an unrelated project. Protect bare hostnames and similar punctuation-qualified identifiers, or perform insertion only on parsed Markdown text nodes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 1f31077. Bare DNS hostnames are now protected before resource-link insertion, so a label such as API inside API.example.com remains part of the hostname while a separate prose mention is still linked. The exact regression is covered. All 20 focused assistant-link tests and full frontend validation pass with zero errors or warnings.
|
1 participant
Summary
Why
The generic Markdown table theme imposed a minimum width, controls, and both wrapper and container borders. Important values overflowed horizontally, while stack names were plain text when the model failed to reproduce a tool result's
webUrl.The Fix with Exie action was mounted only in detail-sheet headers, so it disappeared on direct stack and event routes and competed with sheet-level navigation controls.
Suggested-action clicks were marked as assistant suggestions and then rejected unconditionally by the write authorization guard, even though the user had deliberately clicked the visible action and submitted its explicit command.
Impact
Assistant results are denser and readable without horizontal scrolling. Returned Exceptionless resources are directly navigable, while ambiguous, nested, external, existing Markdown, and code content remain untouched.
The compact Fix action now stays with the stack controls in both overlay and full-page views while preserving event-aware prompts when an event is selected.
External references now occupy the same label/value columns as Project, Tags, and status metadata. Multiple links stack cleanly in the value column, each with a single 14px open icon and an adjacent square 24px delete action.
Clicked write suggestions can execute without a redundant confirmation turn. The existing affirmative-intent, current-target, and exact-argument checks still reject investigation wording, ambiguity, negation, and mismatched mutations.
Verification
npm run validatenpm run test:unit -- src/lib/features/assistant/assistant-links.test.ts src/lib/features/assistant/components/assistant-fix-button.svelte.test.ts src/lib/features/assistant/components/assistant-message.svelte.test.ts src/lib/features/shared/components/ai-elements/response/response.svelte.test.ts— 14 passednpm run builddotnet test --project tests/Exceptionless.Tests/Exceptionless.Tests.csproj -- --filter-class Exceptionless.Tests.Assistant.AssistantServiceTests— 91 passedBreaking changes
None.