fix: enable readfile liquid tag in workflow args #306
Merged
probelabs / Visor: architecture
succeeded
Jan 30, 2026 in 2m 53s
✅ Check Passed (Warnings Found)
architecture check passed. Found 5 warnings, but fail_if condition was not met.
Details
📊 Summary
- Total Issues: 7
- Warning Issues: 5
🔍 Failure Condition Results
Passed Conditions
- global_fail_if: Condition passed
Issues by Category
Architecture (7)
⚠️ src/workflow-registry.ts:144 - Duplicate 'already exists' error handling logic appears in both workflow-registry.ts and config.ts. The same pattern of checking if all errors contain 'already exists' is duplicated across two files, creating maintenance burden and potential for inconsistencies.⚠️ src/frontends/slack-frontend.ts:294 - Timeout detection logic is scattered across multiple files (slack-frontend.ts, routing.ts, level-dispatch.ts, stats-manager.ts, command-check-provider.ts). Each location implements slightly different patterns for detecting timeouts via ruleId.includes('timeout') or message text matching, creating inconsistency and maintenance burden.⚠️ src/state-machine/states/routing.ts:81 - The classifyFailure() function in routing.ts implements different timeout detection patterns than isExecutionFailureIssue() in slack-frontend.ts. routing.ts checks msgLower.includes('timed out') while slack-frontend.ts checks msgLower.includes('timed out') AND msg.includes('Command execution failed'). This inconsistency could lead to different failure classifications for the same issue.⚠️ src/workflow-registry.ts:144 - The importInternal() method handles both top-level imports and workflow-level imports in the same function, creating nested loops and complex control flow. The function processes topImports, then iterates workflows, then processes workflowImports, making it difficult to follow and maintain.- ℹ️ src/slack/socket-runner.ts:168 - The Slack socket filtering logic now contains extensive debug logging blocks (VISOR_DEBUG checks) that are repeated for every filter condition. While useful for debugging, this pattern creates code bloat and reduces readability by mixing logging concerns with business logic.
- ...and 2 more architecture issues
Powered by Visor from Probelabs
💡 TIP: You can chat with Visor using /visor ask <your question>
Annotations
Check warning on line 144 in src/workflow-registry.ts
probelabs / Visor: architecture
architecture Issue
Duplicate 'already exists' error handling logic appears in both workflow-registry.ts and config.ts. The same pattern of checking if all errors contain 'already exists' is duplicated across two files, creating maintenance burden and potential for inconsistencies.
Raw output
Extract the duplicate 'already exists' error detection logic into a shared utility function that can be reused by both WorkflowRegistry.importInternal() and ConfigManager.loadConfigFromObject(). This would centralize the business logic for handling duplicate workflow imports.
Check warning on line 294 in src/frontends/slack-frontend.ts
probelabs / Visor: architecture
architecture Issue
Timeout detection logic is scattered across multiple files (slack-frontend.ts, routing.ts, level-dispatch.ts, stats-manager.ts, command-check-provider.ts). Each location implements slightly different patterns for detecting timeouts via ruleId.includes('timeout') or message text matching, creating inconsistency and maintenance burden.Raw output
Centralize timeout detection logic into a single utility function (e.g., isTimeoutIssue(issue: any): boolean) that can be imported and reused across all components. This ensures consistent timeout classification and reduces code duplication.
Check warning on line 81 in src/state-machine/states/routing.ts
probelabs / Visor: architecture
architecture Issue
The classifyFailure() function in routing.ts implements different timeout detection patterns than isExecutionFailureIssue() in slack-frontend.ts. routing.ts checks msgLower.includes('timed out') while slack-frontend.ts checks msgLower.includes('timed out') AND msg.includes('Command execution failed'). This inconsistency could lead to different failure classifications for the same issue.Raw output
Unify failure classification logic by having both routing.ts and slack-frontend.ts use a shared utility function for detecting execution failures and timeouts. This ensures consistent behavior across the system.
Check warning on line 144 in src/workflow-registry.ts
probelabs / Visor: architecture
architecture Issue
The importInternal() method handles both top-level imports and workflow-level imports in the same function, creating nested loops and complex control flow. The function processes topImports, then iterates workflows, then processes workflowImports, making it difficult to follow and maintain.
Raw output
Consider refactoring import logic to separate top-level import processing from workflow-level import processing. This could be achieved by extracting workflow-level import handling into a separate method or by restructuring the control flow to be more linear and easier to understand.
Check notice on line 168 in src/slack/socket-runner.ts
probelabs / Visor: architecture
architecture Issue
The Slack socket filtering logic now contains extensive debug logging blocks (VISOR_DEBUG checks) that are repeated for every filter condition. While useful for debugging, this pattern creates code bloat and reduces readability by mixing logging concerns with business logic.
Raw output
Consider extracting the debug logging into a single conditional wrapper function or using a debug logging utility that conditionally logs based on the VISOR_DEBUG flag. This would reduce code duplication and improve readability of the core filtering logic.
Check warning on line 328 in src/utils/worktree-manager.ts
probelabs / Visor: architecture
architecture Issue
The worktree refresh logic is embedded within the existing worktree reuse path, creating a deeply nested control structure with try-catch blocks and conditional logic. The refresh operation (fetch, checkout, metadata update) is intertwined with the reuse logic, making it difficult to test and maintain independently.
Raw output
Extract the worktree refresh logic into a separate method (e.g., refreshWorktreeIfAdvanced()) that can be called from the reuse path. This would improve testability, reduce nesting, and make the refresh operation more explicit and easier to understand.
Check notice on line 30 in src/providers/workflow-check-provider.ts
probelabs / Visor: architecture
architecture Issue
The change from 'new Liquid()' to 'createExtendedLiquid()' is applied inconsistently across the codebase. While workflow-check-provider.ts, workflow-executor.ts, and test-runner/index.ts now use createExtendedLiquid(), other parts of the system may still be using plain Liquid instances, leading to inconsistent feature availability.
Raw output
Conduct a comprehensive audit of all Liquid instantiation across the codebase to ensure createExtendedLiquid() is used consistently wherever Liquid templates are rendered. Document the standard approach in a central location to prevent future inconsistencies.
Loading