Warn against semicolons in categorisation rules editor - #724
Warn against semicolons in categorisation rules editor#724AbdulGoodLooks wants to merge 2 commits into
Conversation
Workaround to attempt to prevent creating rules that break aw-server like this https://forum.activitywatch.net/t/invalid-syntax-for-value-to-assign-out-of-nowhere/4627/3
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 93f4484 in 1 minute and 31 seconds. Click for details.
- Reviewed
15lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/util/validate.ts:6
- Draft comment:
Document the rationale further: consider adding the forum post URL in the comment so future maintainers understand why semicolons break aw-server. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to document the rationale further by adding a URL to a forum post. This is a request for additional documentation, which is not allowed according to the rules. The comment does not provide a specific code suggestion or ask for a test to be written, so it should be removed.
Workflow ID: wflow_E2xAh4bMlQ1LhDNM
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #724 +/- ##
==========================================
- Coverage 26.30% 26.29% -0.02%
==========================================
Files 29 29
Lines 1684 1685 +1
Branches 301 302 +1
==========================================
Hits 443 443
+ Misses 1219 1180 -39
- Partials 22 62 +40 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@TimeToBuildBob Investigate this, shouldn't it get escaped in the query and be supported? Does it break aw-server-python or aw-server-rust or both? |
|
Investigated both servers. Short answer: semicolons should be supported and don't need to be blocked. Both servers handle them correctly when the regex is embedded as a string literal in the AQL query (which is what aw-webui does via aw-server-python: Was broken — the old query2.py split statements on all semicolons without tracking string context. Fixed in ActivityWatch/aw-core#131 (merged 2026-02-27). The aw-server-rust: Never broken. The plex-based lexer in How aw-webui builds queries: Recommendation: This PR isn't needed. The original bug (forum post) was in an older aw-server-python before the aw-core fix. The right fix was aw-core#131, which is already merged. Blocking semicolons in the UI unnecessarily restricts valid regex patterns (e.g. alternation with Could close this PR as not needed, or the contributor could be asked to add a test to aw-core instead verifying that semicolons in categorization rules work end-to-end. |
|
Investigation complete — root cause found, fix in #918. Short answer: yes, semicolons should be supported. Both servers handle them correctly — the bug is in the webui's client-side query splitter, not in aw-server-python or aw-server-rust. What breaks: A plain Both servers are fine: I checked Fix: PR #918 replaces the naive split with a string-context-aware parser that only treats |
querystr_to_array() was naively splitting on all ';' characters, which
shredded category rules whose regex contained a semicolon. For example,
a rule with regex "foo;bar" gets embedded into the query as:
events = categorize(events, [["Work", {"type":"regex","regex":"foo;bar"}]]);
A plain .split(';') would cut this into two broken fragments before the
query ever reaches aw-server, causing a parse error on the server side.
Both aw-server-python and aw-server-rust handle semicolons correctly
inside quoted strings — the bug is purely in the client-side splitter.
The fix tracks string-literal context (double-quoted, with backslash
escape support) and only treats ';' as a statement separator when it
appears outside a string.
Also exports querystr_to_array for direct unit testing and adds 4 unit
tests covering: basic split, semicolon-in-string (the regression case),
escaped quote handling, and whitespace filtering.
Closes ActivityWatch#724 (the right fix is here, not a UI warning/block).
querystr_to_array() was naively splitting on all ';' characters, which
shredded category rules whose regex contained a semicolon. For example,
a rule with regex "foo;bar" gets embedded into the query as:
events = categorize(events, [["Work", {"type":"regex","regex":"foo;bar"}]]);
A plain .split(';') would cut this into two broken fragments before the
query ever reaches aw-server, causing a parse error on the server side.
Both aw-server-python and aw-server-rust handle semicolons correctly
inside quoted strings — the bug is purely in the client-side splitter.
The fix tracks string-literal context (double-quoted, with backslash
escape support) and only treats ';' as a statement separator when it
appears outside a string.
Also exports querystr_to_array for direct unit testing and adds 4 unit
tests covering: basic split, semicolon-in-string (the regression case),
escaped quote handling, and whitespace filtering.
Closes ActivityWatch#724 (the right fix is here, not a UI warning/block).
* fix(queries): don't split on semicolons inside string literals
querystr_to_array() was naively splitting on all ';' characters, which
shredded category rules whose regex contained a semicolon. For example,
a rule with regex "foo;bar" gets embedded into the query as:
events = categorize(events, [["Work", {"type":"regex","regex":"foo;bar"}]]);
A plain .split(';') would cut this into two broken fragments before the
query ever reaches aw-server, causing a parse error on the server side.
Both aw-server-python and aw-server-rust handle semicolons correctly
inside quoted strings — the bug is purely in the client-side splitter.
The fix tracks string-literal context (double-quoted, with backslash
escape support) and only treats ';' as a statement separator when it
appears outside a string.
Also exports querystr_to_array for direct unit testing and adds 4 unit
tests covering: basic split, semicolon-in-string (the regression case),
escaped quote handling, and whitespace filtering.
Closes #724 (the right fix is here, not a UI warning/block).
* fix(lint): rename escape→inEscape to avoid shadowing global; fix prettier
- `escape` is a reserved JS global; rename to `inEscape` to silence no-shadow
- collapse two-line const assignment in test to satisfy prettier line-length rule
* fix(views): use querystr_to_array in all query-splitting call sites
Seven views were still doing a naive .split(';') on query strings before
sending them to aw-client. Category rules with semicolons in their regex
(e.g. "foo;bar") embed as JSON string literals inside the AQL query, so a
plain split shreds them before they reach the server.
Replace every .split(';').map(s => s.trim() + ';') chain (and the two
longer filter-variant chains in Timeline and Trends) with querystr_to_array,
which correctly skips semicolons inside double-quoted string literals.
Workaround to attempt to prevent creating rules that break aw-server like this https://forum.activitywatch.net/t/invalid-syntax-for-value-to-assign-out-of-nowhere/4627/3
Important
Update
validateRegexto flag semicolons in regex as invalid to prevent server errors.validateRegexinvalidate.tsto returnfalseif regex contains semicolons, preventing server errors.aw-server.This description was created by
for 93f4484. You can customize this summary. It will automatically update as commits are pushed.