Improve terminal input handling and task loading#235
Improve terminal input handling and task loading#235DeliLevente99 wants to merge 12 commits intosubsy:mainfrom
Conversation
Enhances prompt input by filtering out mouse tracking and escape codes, adds real-time input filtering for text prompts, and disables mouse tracking in the terminal. Also updates the resume command to load tasks from the tracker and passes them to the TUI. Expands user story header parsing to support up to four hash marks.
|
@DeliLevente99 is attempting to deploy a commit to the plgeek Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughResume path now loads tracker tasks and passes them into the TUI; Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/setup/prompts.ts`:
- Around line 136-142: The cleanup() function currently calls
process.stdin.removeAllListeners('data') which removes every 'data' listener;
instead remove only the specific listener you originally registered for stdin by
keeping a reference to that callback (the function you passed to
process.stdin.on('data', ...)) and calling process.stdin.removeListener('data',
yourCallback). Update the code to store the handler in a const (e.g.,
dataHandler) where you attach it, use process.stdin.on('data', dataHandler), and
replace removeAllListeners('data') in cleanup() with
process.stdin.removeListener('data', dataHandler) while keeping the existing
pause() and setRawMode(false) logic.
- Around line 47-75: The regex literals in stripEscapeCodes that include control
characters should be replaced with RegExp constructors to satisfy Biome's rule:
replace the /\x1b\[[0-9;?]*[a-zA-Z]/g literal with new
RegExp('\\x1b\\[[0-9;?]*[a-zA-Z]', 'g'), replace the
/\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)/g literal with new
RegExp('\\x1b\\][^\\x07\\x1b]*(?:\\x07|\\x1b\\\\)', 'g'), and replace
/[\x00-\x1F\x7F]/g with new RegExp('[\\x00-\\x1F\\x7F]', 'g'); keep the same
flags and assign results back to cleaned exactly as before so stripEscapeCodes
behavior is unchanged.
🧹 Nitpick comments (2)
src/commands/resume.tsx (2)
612-620: Avoid loading tracker tasks when TUI is disabled.
Headless runs still callgetTasks, which can add latency or fail unnecessarily. Consider guarding the fetch behind the TUI branch.♻️ Suggested adjustment
- // Load tasks from tracker - let tasks: any[] = []; - try { - const trackerRegistry = getTrackerRegistry(); - const tracker = await trackerRegistry.getInstance(config.tracker); - tasks = await tracker.getTasks({ status: ['open', 'in_progress', 'completed'] }); - } catch (error) { - console.error('Warning: Failed to load tasks:', error); - } + // Load tasks from tracker (TUI only) + let tasks: any[] = []; + if (!headless && config.showTui) { + try { + const trackerRegistry = getTrackerRegistry(); + const tracker = await trackerRegistry.getInstance(config.tracker); + tasks = await tracker.getTasks({ status: ['open', 'in_progress', 'completed'] }); + } catch (error) { + console.error('Warning: Failed to load tasks:', error); + } + }
287-294: ChangeinitialTasksparameter toTrackerTask[]and add the necessary import.
TheRunAppcomponent expectsinitialTasks?: TrackerTask[], so acceptingany[]creates a type mismatch. ImportTrackerTaskfromsrc/plugins/trackers/types.tsand update the function signature to use the concrete type for better type safety and editor support.
| /** | ||
| * Strip mouse tracking and other escape codes from input string | ||
| */ | ||
| function stripEscapeCodes(input: string): string { | ||
| // Step 1: Remove ALL mouse tracking patterns first (before they get fragmented) | ||
| // Pattern: any sequence of digits and semicolons followed by 'M' or 'm' | ||
| // This catches: 35;106;28M, 100;200M, etc. | ||
| let cleaned = input.replace(/\d+(?:;\d+)*[Mm]/g, ''); | ||
|
|
||
| // Step 2: Remove any remaining standalone semicolons (fragments from mouse codes) | ||
| cleaned = cleaned.replace(/;+/g, ''); | ||
|
|
||
| // Step 3: Remove escape sequences | ||
| cleaned = cleaned.replace(/\x1b\[[0-9;?]*[a-zA-Z]/g, ''); | ||
| cleaned = cleaned.replace(/\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)/g, ''); | ||
|
|
||
| // Step 4: Remove any control characters (except space, tab, newline) | ||
| cleaned = cleaned.replace(/[\x00-\x1F\x7F]/g, (char) => { | ||
| // Keep space (0x20 is already outside this range) | ||
| // Keep tab (0x09) and newline (0x0A) | ||
| if (char === '\t' || char === '\n') return char; | ||
| return ''; | ||
| }); | ||
|
|
||
| // Step 5: Clean up any resulting multiple spaces | ||
| cleaned = cleaned.replace(/\s+/g, ' '); | ||
|
|
||
| return cleaned; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file
cd /repo && git ls-files | grep -E "src/setup/prompts\.ts"Repository: subsy/ralph-tui
Length of output: 114
🏁 Script executed:
# Check if file exists and examine the relevant lines
if [ -f "src/setup/prompts.ts" ]; then
echo "=== File exists, checking lines 40-80 ==="
sed -n '40,80p' src/setup/prompts.ts | cat -n
fiRepository: subsy/ralph-tui
Length of output: 1808
🏁 Script executed:
# Search for Biome configuration
find . -maxdepth 2 -type f \( -name "biome.json*" -o -name ".biomerc*" \) 2>/dev/nullRepository: subsy/ralph-tui
Length of output: 71
🏁 Script executed:
# Check for any linting configuration files
git ls-files | grep -E "(biome|eslint|tsconfig)" | head -20Repository: subsy/ralph-tui
Length of output: 137
🏁 Script executed:
# Examine the Biome configuration
cat biome.jsonRepository: subsy/ralph-tui
Length of output: 1833
🏁 Script executed:
# Also check if there's more context about the project structure
git ls-files | head -30Repository: subsy/ralph-tui
Length of output: 735
🌐 Web query:
Biome linter rules control characters regex literals
💡 Result:
Biome has a dedicated rule for control characters in regex patterns:
lint/suspicious/noControlCharactersInRegex(noControlCharactersInRegex, enabled by default) — disallows ASCII control chars (0–31) inside regex patterns, including escapes like\x00–\x1F,\u0000–\u001F,\u{0}–\u{1F}, and raw U+0000–U+001F. It allows common control escapes like\tand\n. (biomejs.dev)
Configure in biome.json:
{
"linter": {
"rules": {
"suspicious": {
"noControlCharactersInRegex": "error"
}
}
}
}Related regex-literal rules you may also want:
lint/complexity/useRegexLiterals(useRegexLiterals) — prefers/.../overnew RegExp("...")when possible (has a safe fix). (biomejs.dev)lint/complexity/noUselessEscapeInRegex(noUselessEscapeInRegex) — flags unnecessary escapes inside regex literals (has a safe fix). (biomejs.dev)
Citations:
- 1: https://biomejs.dev/linter/rules/no-control-characters-in-regex/?utm_source=openai
- 2: https://biomejs.dev/linter/rules/no-control-characters-in-regex/?utm_source=openai
- 3: https://biomejs.dev/linter/rules/use-regex-literals/?utm_source=openai
- 4: https://biomejs.dev/linter/rules/no-useless-escape-in-regex/?utm_source=openai
🌐 Web query:
Biome regex control character rule no-regex-spaces
💡 Result:
In Biome, the ESLint-style regex rules you’re referencing map to these Biome linter rules:
no-regex-spaces(ESLint) →lint/complexity/noMultipleSpacesInRegex(also seenoAdjacentSpacesInRegex; both list their source asno-regex-spaces). It reports “unclear uses of consecutive spaces” in regex literals and suggests replacing them with a quantifier (safe fix). [1] [2]no-control-regex(ESLint) →lint/suspicious/noControlCharactersInRegex. It disallows ASCII control characters (U+0000–U+001F) in regexes, including escape forms like\x00–\x1F,\u0000–\u001F, and\u{0}–\u{1F}; it allows\tand\n. [3]
Example Biome config:
{
"linter": {
"rules": {
"complexity": {
"noMultipleSpacesInRegex": "error"
},
"suspicious": {
"noControlCharactersInRegex": "error"
}
}
}
}Sources: Biome rule docs for noAdjacentSpacesInRegex, noMultipleSpacesInRegex, and noControlCharactersInRegex. [1] [2] [3]
References
- Biome:
noAdjacentSpacesInRegex[1] - Biome:
noMultipleSpacesInRegex[2] - Biome:
noControlCharactersInRegex[3]
🏁 Script executed:
# Check if there's any indication that this code currently fails Biome checks
# by looking at CI workflow
cat .github/workflows/ci.ymlRepository: subsy/ralph-tui
Length of output: 10842
🏁 Script executed:
# Check the file header for ABOUTME comment
head -20 src/setup/prompts.tsRepository: subsy/ralph-tui
Length of output: 556
Replace control-character regex literals to satisfy Biome.
The regex patterns on lines 21–22 and 25 contain control characters (\x1b, \x07, \x00–\x1F), which violate Biome's noControlCharactersInRegex rule and will fail CI. Use RegExp constructors with escaped strings instead.
Suggested fix
- cleaned = cleaned.replace(/\x1b\[[0-9;?]*[a-zA-Z]/g, '');
- cleaned = cleaned.replace(/\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)/g, '');
+ const CSI_ESCAPE_RE = new RegExp('\\x1b\\[[0-9;?]*[a-zA-Z]', 'g');
+ const OSC_ESCAPE_RE = new RegExp('\\x1b\\][^\\x07\\x1b]*(?:\\x07|\\x1b\\\\)', 'g');
+ cleaned = cleaned.replace(CSI_ESCAPE_RE, '');
+ cleaned = cleaned.replace(OSC_ESCAPE_RE, '');
- cleaned = cleaned.replace(/[\x00-\x1F\x7F]/g, (char) => {
+ const CONTROL_CHAR_RE = new RegExp('[\\x00-\\x1F\\x7F]', 'g');
+ cleaned = cleaned.replace(CONTROL_CHAR_RE, (char) => {🧰 Tools
🪛 Biome (2.1.2)
[error] 60-60: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 61-61: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 61-61: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 61-61: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 61-61: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 61-61: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 64-64: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 64-64: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🤖 Prompt for AI Agents
In `@src/setup/prompts.ts` around lines 47 - 75, The regex literals in
stripEscapeCodes that include control characters should be replaced with RegExp
constructors to satisfy Biome's rule: replace the /\x1b\[[0-9;?]*[a-zA-Z]/g
literal with new RegExp('\\x1b\\[[0-9;?]*[a-zA-Z]', 'g'), replace the
/\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)/g literal with new
RegExp('\\x1b\\][^\\x07\\x1b]*(?:\\x07|\\x1b\\\\)', 'g'), and replace
/[\x00-\x1F\x7F]/g with new RegExp('[\\x00-\\x1F\\x7F]', 'g'); keep the same
flags and assign results back to cleaned exactly as before so stripEscapeCodes
behavior is unchanged.
…orted by coderabbitai Enhances the TUI session to better track active tasks by subscribing to engine events for task activation and completion, ensuring session state is updated and persisted accordingly. Refactors prompt cleanup to remove only the relevant data listener, and updates escape code stripping to use RegExp constructors for better linter compatibility. Also refines historical output loading logic in RunApp to allow showing previous output when resuming in-progress tasks.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/setup/prompts.ts`:
- Around line 164-167: The code builds a dynamic RegExp from options.pattern
inside the prompt validation (when options.pattern && value) without error
handling; wrap the RegExp construction in a try/catch around new
RegExp(options.pattern) (referencing options.pattern and value) so invalid
patterns don’t throw: on catch, log or print a clear warning (using the same
colors/logger used in this module), and either treat the input as invalid (ask
user to retry) or skip pattern validation to continue gracefully; ensure the
rest of the validation flow (the regex.test(value) branch) only runs when the
RegExp was successfully created.
- Around line 149-154: The onData handler's regex in src/setup/prompts.ts
currently matches numbers without semicolons and can drop legitimate input like
"10m"; update the regex used in onData to only match SGR mouse codes by
requiring at least one semicolon and anchoring to the end (i.e., require the
pattern to include `;` between numbers and end with `M` or `m`), so only true
mouse-tracking sequences are ignored and normal inputs are preserved.
…error reported by coderabbitai Refines the regex for filtering mouse tracking codes to avoid false positives and adds error handling for invalid pattern configurations in prompt input validation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/setup/prompts.ts`:
- Around line 51-58: The mouse-code stripping regex in stripEscapeCodes is too
permissive and removes valid inputs (e.g., "10m"); update the pattern to require
at least one semicolon to match the safer onData check by replacing
/\d+(?:;\d+)*[Mm]/g with /\d+(?:;\d+)+[Mm]/g so only sequences like "35;106;28M"
are removed; ensure this change applies where stripEscapeCodes is used by
promptBoolean, promptSelect, and promptNumber.
…e error reported by coderabbitai Refines the regex in stripEscapeCodes to avoid false positives when removing mouse tracking patterns. Adds an AnimatedProgressBar component to ChatView for simulating progress during loading states.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.54%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #235 +/- ##
==========================================
- Coverage 45.45% 45.15% -0.30%
==========================================
Files 84 84
Lines 24396 24558 +162
==========================================
Hits 11088 11088
- Misses 13308 13470 +162
🚀 New features to boost your workflow:
|
|
@DeliLevente99 thanks for this - looks good, but test coverage needs to be >50% for new/changed LoC - can you please add tests to meet the threshold. |
The parser now recognizes user stories defined with H4 headers (####) and 'Feature X.Y' formats, normalizing them to 'FEAT-X-Y' IDs. Tests have been added for all header and ID format combinations. The error message in the PRD chat app was updated to document all supported user story header formats. Utility functions for stripping escape codes and disabling mouse tracking are now exported and tested.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/setup/prompts.ts`:
- Around line 56-57: The current aggressive semicolon removal uses cleaned =
cleaned.replace(/;+/g, ''); which will strip valid semicolons from user input
(URLs, code, configs); either remove that replacement entirely or replace it
with a targeted removal that only deletes semicolons directly adjacent to
previously-removed mouse-code fragments (e.g., only where the mouse-code regex
matched boundaries), updating the logic around the variable cleaned in this
module (the cleaned variable post-mouse-code stripping) so legitimate semicolons
remain intact.
🧹 Nitpick comments (1)
src/prd/parser.test.ts (1)
1-4: Consider updating the ABOUTME comment to reflect expanded scope.The file now also tests header level support (H2–H4) and multiple ID formats (US-XXX, PREFIX-XXX, Feature X.Y), which extends beyond just description extraction.
📝 Suggested update
/** * ABOUTME: Tests for the PRD markdown parser. - * Covers user story description extraction with various LLM-generated formats. + * Covers header level support, ID format parsing, and description extraction + * with various LLM-generated formats. */
Enhanced the PRD parser to recognize user story headers with version-style IDs (e.g., US-2.1.1, US-1.2) and added a fallback mechanism to extract stories from any header with a colon in the User Stories section. Updated related tests and user guidance to reflect the new supported formats. Improved escape code stripping to preserve legitimate semicolons in user input.
…h-tui into Issue-223-fix
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/prd/parser.ts`:
- Around line 422-438: The parser currently only exits the User Stories block
for a fixed list of non-story headers; update the logic in the parsing loop that
uses inUserStoriesSection and currentStory so any new H1/H2 that is not a
recognized story header forces an exit. Replace the isNonStorySection check with
a condition that treats /^#{1,2}\s+/ lines as section boundaries unless they
match the story header pattern (the same pattern used to detect stories earlier
in this parser), then save currentStory (using
lines.slice(currentStory.startIndex, i) and push into sections) and break;
ensure the change references the same variables currentStory,
inUserStoriesSection, sections, and lines so behavior is consistent.
- Around line 42-50: USER_STORY_HEADER_PATTERN is too permissive because the
generic prefix `[A-Z]+-\d+` will also match US- variants like US-1 or US-9999;
update the regex to exclude the "US-" prefix from the generic case and keep the
intended US formats. Replace `[A-Z]+-\d+` with a negative-lookahead form like
`(?!US-)[A-Z]+-\d+` (or similar) so the overall constant
USER_STORY_HEADER_PATTERN still matches the explicit US formats (`US-\d{3}` and
`US-\d+(?:\.\d+)+`) but prevents the generic branch from capturing US-* IDs.
🧹 Nitpick comments (1)
src/setup/prompts.test.ts (1)
186-191: Test comment is misleading about which mouse code is matched.The comment states "Mouse code "35;106;28M" is removed", but the regex
/\d+;\d+(?:;\d+)*[Mm]/gwill actually match "135;106;28M" because\d+greedily matches the full number "135". The expected result is correct, but the comment should be updated for clarity.📝 Suggested comment fix
test('removes mouse codes but preserves legitimate semicolons', () => { const input = 'URL: http://test.com;a=135;106;28Mb'; const result = stripEscapeCodes(input); - // Mouse code "35;106;28M" is removed, leaving "URL: http://test.com;a=1b" + // Mouse code "135;106;28M" is removed, leaving "URL: http://test.com;a=b" expect(result).toBe('URL: http://test.com;a=b'); });
Enhanced the parser to better distinguish valid US-XXX formats, exclude US-1 and US-9999 from generic pattern matching, and add a 3-tier fallback strategy to always extract user stories from markdown. Added ultimate fallback to parse any H2/H3/H4 headers with colons, skipping common non-story headers. Updated and expanded tests to cover new parsing logic and edge cases.
…h-tui into Issue-223-fix
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/prd/parser.ts`:
- Around line 101-113: normalizeStoryId fails to normalize headers like "feature
1.1" because the Feature regex is case-sensitive; update the feature matcher in
normalizeStoryId to be case-insensitive (use the /i flag) and trim rawId first,
then build the normalized ID as `FEAT-{major}-{minor}` (use featureMatch[1] and
featureMatch[2]) to ensure spaces/casing are normalized consistently.
🧹 Nitpick comments (1)
src/prd/parser.ts (1)
470-481: Extract the valid‑ID regex to a shared constant.
validIdPatternis duplicated here and in the ultimate fallback; hoisting it avoids drift and repeated compilation inside loops.Suggested refactor
+const VALID_STORY_ID_PATTERN = + /^US-\d{3}$|^US-\d+(?:\.\d+)+$|^(?!US-)[A-Z]+-\d+$|^Feature\s+\d+\.\d+$/i;- const validIdPattern = /^US-\d{3}$|^US-\d+(?:\.\d+)+$|^(?!US-)[A-Z]+-\d+$|^Feature\s+\d+\.\d+$/i; - if (validIdPattern.test(prefix)) { + if (VALID_STORY_ID_PATTERN.test(prefix)) { id = normalizeStoryId(prefix); } else { id = `STORY-${String(storyCounter).padStart(3, '0')}`; }Apply the same replacement in
findUserStorySectionsUltimate.
Updated the user story header and normalization patterns to handle 'Feature X.Y' identifiers in a case-insensitive manner. This improves flexibility when parsing user story headers with different casing.
|
Thank you for this contribution! I've merged these changes with the current main branch (resolving a conflict in RunApp.tsx) and created #265 to bring this forward. Your improved historical output loading logic was kept in the conflict resolution - it correctly handles the resume scenario by only loading historical data when no current iteration exists. All changes reviewed and confirmed still needed. 🙏 |
|
Closing in favor of #265, which includes all your changes merged with latest main. What was kept:
What was simplified:
Thank you for the contribution! 🙏 |
Improve terminal input handling and task loading (supersedes #235)
Improve terminal input handling and task loading (supersedes subsy#235)
Enhances prompt input by filtering out mouse tracking and escape codes, adds real-time input filtering for text prompts, and disables mouse tracking in the terminal. Also updates the resume command to load tasks from the tracker and passes them to the TUI. Expands user story header parsing to support up to four hash marks.
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.