Skip to content

Conversation

@jbsmith7741
Copy link
Collaborator

@jbsmith7741 jbsmith7741 commented Jan 8, 2026

PR Type

Enhancement, Tests


Description

  • Add backload form UI for creating historical tasks with date ranges

  • Implement hourly task breakdown chart on task dashboard

  • Extract reusable table sorting module for consistent sorting across pages

  • Enhance about page with dynamic uptime calculation and number formatting

  • Improve utility functions with better HTML escaping and event handling


Diagram Walkthrough

flowchart LR
  A["Backload Feature"] --> B["backload.js<br/>backload.tmpl"]
  C["Task Stats"] --> D["GetCountsWithHourlyFiltered<br/>method"]
  D --> E["Hourly Chart<br/>Rendering"]
  F["Table Sorting"] --> G["table-sort.js<br/>Reusable Module"]
  G --> H["alert.tmpl<br/>workflow.tmpl"]
  I["Utils Refactor"] --> J["escapeHtml<br/>escapeJsString"]
  J --> K["Better Security<br/>Event Handling"]
Loading

File Walkthrough

Relevant files
Enhancement
14 files
handler.go
Add backload template and route handler                                   
+77/-9   
stats.go
Add hourly task breakdown with filtering                                 
+93/-0   
workflow.go
Add method to get phases grouped by workflow                         
+16/-0   
backload.js
New backload form with task selection and preview               
+635/-0 
table-sort.js
Reusable table sorting module for all pages                           
+185/-0 
task.js
Add result filter and initialize filters early                     
+20/-23 
utils.js
Improve HTML escaping and event handling                                 
+47/-20 
style.css
Add backload form and hourly chart styles                               
+398/-0 
about.tmpl
Add dynamic uptime calculation and number formatting         
+43/-8   
alert.tmpl
Use reusable table-sort module and utils                                 
+12/-264
backload.tmpl
New backload form template with task selection UI               
+174/-0 
header.tmpl
Add backload navigation link to header                                     
+8/-0     
task.tmpl
Add hourly chart and result filter dropdown                           
+225/-24
workflow.tmpl
Use reusable table-sort module and utils                                 
+23/-266
Tests
1 files
handler_test.go
Add test for backload HTML generation                                       
+26/-0   
Bug fix
2 files
taskmaster.go
Fix cron parser initialization                                                     
+1/-1     
jobs.toml
Fix rule syntax from question mark to ampersand                   
+1/-1     

@qodo-code-review
Copy link

qodo-code-review bot commented Jan 8, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Cross-site scripting

Description: Unescaped template data (e.g., .Rule / .Template) is interpolated into inline
event-handler JavaScript strings (e.g., onclick="...('{{.Rule}}')"), enabling DOM-based
XSS if these values contain quotes/JS payloads (recommend using a proper JS-string escaper
like escapeJsString or avoid inline handlers).
workflow.tmpl [77-96]

Referred Code
<td class="rule-cell truncated copyable" 
    data-full-text="{{.Rule}}" 
    data-truncated-text="{{if ge (len .Rule) 120}}{{slice .Rule 0 120}}...{{else}}{{.Rule}}{{end}}"
    onclick="window.FlowlordUtils.toggleField(this, '{{.Rule}}')" 
    oncontextmenu="window.FlowlordUtils.showContextMenu(event, '{{.Rule}}')"
    ondblclick="window.FlowlordUtils.copyToClipboard('{{.Rule}}')"
    title="Click to expand/collapse, double-click or right-click to copy">
    {{if ge (len .Rule) 120}}{{slice .Rule 0 120}}...{{else}}{{.Rule}}{{end}}
</td>
<td class="depends-on-cell">{{.DependsOn}}</td>
<td class="retry-cell">{{.Retry}}</td>
<td class="template-cell truncated copyable" 
    data-full-text="{{.Template}}" 
    data-truncated-text="{{if ge (len .Template) 120}}{{slice .Template 0 120}}...{{else}}{{.Template}}{{end}}"
    onclick="window.FlowlordUtils.toggleField(this, '{{.Template}}')" 
    oncontextmenu="window.FlowlordUtils.showContextMenu(event, '{{.Template}}')"
    ondblclick="window.FlowlordUtils.copyToClipboard('{{.Template}}')"
    title="Click to expand/collapse, double-click or right-click to copy">
    {{if ge (len .Template) 120}}{{slice .Template 0 120}}...{{else}}{{.Template}}{{end}}
</td>
Inline JS injection

Description: Unescaped data (e.g., .TaskID / .Msg) is embedded into inline JavaScript event handler
attributes (e.g., onclick="...('{{.Msg}}')"), which can break out of the quoted string and
execute attacker-controlled script, resulting in XSS.
alert.tmpl [41-57]

Referred Code
<tr>
    <td class="id-cell id-column truncated copyable" 
        data-full-text="{{.TaskID}}" 
        onclick="window.FlowlordUtils.toggleField(this, '{{.TaskID}}')" 
        oncontextmenu="window.FlowlordUtils.showContextMenu(event, '{{.TaskID}}')"
        ondblclick="window.FlowlordUtils.copyToClipboard('{{.TaskID}}')"
        title="Click to expand/collapse, double-click or right-click to copy">
        {{.TaskID}}
    </td>
    <td class="type-cell type-column">{{.Type}}</td>
    <td class="job-cell job-column">{{.Job}}</td>
    <td class="message-cell message-column truncated copyable" 
        data-full-text="{{.Msg}}" 
        onclick="window.FlowlordUtils.toggleField(this, '{{.Msg}}')" 
        oncontextmenu="window.FlowlordUtils.showContextMenu(event, '{{.Msg}}')"
        ondblclick="window.FlowlordUtils.copyToClipboard('{{.Msg}}')"
        title="Click to expand/collapse, double-click or right-click to copy">
Unsafe script embedding

Description: The backload page injects JSON into a <script> context using template.JS(phasesJSON), which
disables auto-escaping and can permit script-breaking sequences (e.g., </script> within
workflow-derived strings) to become an XSS vector unless the JSON is safely encoded for
script embedding.
handler.go [758-763]

Referred Code
phasesJSON, _ := json.Marshal(allPhases)

data := map[string]interface{}{
	"PhasesByWorkflow": phasesByWorkflow,
	"PhasesJSON":       template.JS(phasesJSON),
	"CurrentPage":      "backload",
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Ignored marshal error: The new code ignores the json.Marshal error and proceeds, which can lead to invalid/empty
PhasesJSON and hard-to-debug failures.

Referred Code
phasesJSON, _ := json.Marshal(allPhases)

data := map[string]interface{}{
	"PhasesByWorkflow": phasesByWorkflow,
	"PhasesJSON":       template.JS(phasesJSON),
	"CurrentPage":      "backload",

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Raw error to user: The new backload page renderer returns err.Error() directly in the HTTP response, which
can expose internal implementation details to end users.

Referred Code
// Parse and execute template using the shared funcMap
tmpl, err := template.New("backload").Funcs(getBaseFuncMap()).Parse(HeaderTemplate + BackloadTemplate)
if err != nil {
	return []byte("Error parsing template: " + err.Error())
}

var buf bytes.Buffer
if err := tmpl.Execute(&buf, data); err != nil {
	return []byte("Error executing template: " + err.Error())
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Backload access logging: The new backload page handler (GET /web/backload) does not emit any audit log entry, and
the diff does not show whether the corresponding backload execution endpoint logs
user/action/outcome for task creation.

Referred Code
// htmlBackload handles GET /web/backload - displays the backload form
func (tm *taskMaster) htmlBackload(w http.ResponseWriter, r *http.Request) {
	w.WriteHeader(http.StatusOK)
	w.Header().Set("Content-Type", "text/html")
	w.Write(backloadHTML(tm.taskCache))
}

// backloadHTML renders the backload form HTML page
func backloadHTML(tCache *sqlite.SQLite) []byte {
	// Get all phases grouped by workflow file
	phasesByWorkflow := tCache.GetAllPhasesGrouped()

	// Create flat list of phases for JSON encoding
	type phaseJSON struct {
		Workflow  string `json:"workflow"`
		Task      string `json:"task"`
		Job       string `json:"job"`
		Template  string `json:"template"`
		Rule      string `json:"rule"`
		DependsOn string `json:"dependsOn"`
	}


 ... (clipped 36 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Unstructured request logs: The updated task page log line is unstructured and logs user-supplied filter fields (e.g.,
filter.ID), which may be sensitive depending on task identifier semantics.

Referred Code
// Single consolidated log with all metrics
log.Printf("Task page: date=%s filters=[id=%q type=%q job=%q result=%q] total=%d filtered=%d page=%d/%d query=%v render=%v size=%.2fMB",
	date.Format("2006-01-02"), filter.ID, filter.Type, filter.Job, filter.Result,
	unfilteredCounts.Total, totalCount, filter.Page, totalPages,
	queryTime, renderTime, float64(htmlSize)/(1024*1024))

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Unsafe JS injection risk: The code injects marshaled JSON into templates using template.JS(...), which bypasses
template auto-escaping and requires verification that the JSON encoding is always safe for
inline script contexts.

Referred Code
data := map[string]interface{}{
	"PhasesByWorkflow": phasesByWorkflow,
	"PhasesJSON":       template.JS(phasesJSON),
	"CurrentPage":      "backload",
	"PageTitle":        "Backload Tasks",
	"isLocal":          isLocal,
	"DatesWithData":    []string{}, // Backload page doesn't use date picker with highlights
}

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

qodo-code-review bot commented Jan 8, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Set headers before WriteHeader

In the htmlBackload function, move the w.Header().Set("Content-Type",
"text/html") call to be before w.WriteHeader(http.StatusOK) to ensure the header
is correctly sent.

apps/flowlord/handler.go [725-729]

 func (tm *taskMaster) htmlBackload(w http.ResponseWriter, r *http.Request) {
+  w.Header().Set("Content-Type", "text/html")
   w.WriteHeader(http.StatusOK)
-  w.Header().Set("Content-Type", "text/html")
   w.Write(backloadHTML(tm.taskCache))
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that HTTP headers must be set before WriteHeader is called, which is a fundamental concept in Go's net/http package and fixes a bug.

Medium
Create output directory first

In TestBackloadHTML, before writing the backload_preview.html file, ensure the
handler/ directory exists by calling os.MkdirAll to prevent the test from
failing.

apps/flowlord/handler_test.go [607-608]

 outputFile := "handler/backload_preview.html"
+if err := os.MkdirAll(filepath.Dir(outputFile), 0755); err != nil {
+  t.Fatalf("Failed to create output directory: %v", err)
+}
 err := os.WriteFile(outputFile, html, 0644)
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion improves the robustness of a test by ensuring the output directory exists before writing a file, preventing test failures in environments where the directory is not pre-existing.

Low
High-level
Extract large inline chart-drawing JavaScript

The suggestion proposes extracting over 160 lines of complex JavaScript for
chart rendering from the task.tmpl file into a separate, dedicated .js file.
This change would improve code organization and maintainability.

Examples:

apps/flowlord/handler/task.tmpl [219-380]
        function renderHourlyChart() {
            const canvas = document.getElementById('hourlyChart');
            if (!canvas) return;

            const ctx = canvas.getContext('2d');
            
            // Get hourly data from template
            const hourlyData = [
                {{range $i, $stats := .HourlyStats}}
                {

 ... (clipped 152 lines)

Solution Walkthrough:

Before:

<!-- apps/flowlord/handler/task.tmpl -->
...
<canvas id="hourlyChart"></canvas>
...
<script>
    document.addEventListener('DOMContentLoaded', function() {
        ...
        renderHourlyChart();
        ...
    });

    function renderHourlyChart() {
        const canvas = document.getElementById('hourlyChart');
        const ctx = canvas.getContext('2d');
        const hourlyData = [ {{.HourlyStats}} ];
        // ... ~160 lines of canvas drawing logic ...
        ctx.fillText('Hour of Day', ...);
        ctx.fillText('Number of Tasks', ...);
    }
</script>
</body>

After:

<!-- apps/flowlord/handler/task.tmpl -->
...
<canvas id="hourlyChart"></canvas>
...
<script src="/static/chart.js"></script>
<script>
    document.addEventListener('DOMContentLoaded', function() {
        const hourlyData = [ {{.HourlyStats}} ];
        window.FlowlordCharts.renderHourlyTaskChart('hourlyChart', hourlyData);
    });
</script>
</body>

// New file: apps/flowlord/handler/static/chart.js
(function() {
  function renderHourlyTaskChart(canvasId, data) {
    // ... ~160 lines of canvas drawing logic ...
  }
  window.FlowlordCharts = { renderHourlyTaskChart };
})();

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a large block of complex inline JavaScript in task.tmpl and proposes extracting it, which significantly improves code organization and maintainability, aligning with best practices seen elsewhere in the PR.

Low
Possible issue
Handle potential JSON marshaling error

Handle the potential error returned by json.Marshal in backloadHTML. If
marshaling fails, log the error and return an error response to prevent
rendering a broken page.

apps/flowlord/handler.go [758-760]

-	phasesJSON, _ := json.Marshal(allPhases)
+	phasesJSON, err := json.Marshal(allPhases)
+	if err != nil {
+		log.Printf("Error marshaling phases to JSON: %v", err)
+		return []byte("Error preparing backload data: " + err.Error())
+	}
 
 	data := map[string]interface{}{
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies unhandled error from json.Marshal, which is a valid concern for application robustness, and proposes a standard way to handle it.

Low
  • Update

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enhances the Flowlord UI with a backload feature for creating historical tasks, implements hourly task breakdown charts, extracts reusable table sorting functionality, and improves the about page with dynamic uptime calculation and number formatting.

Changes:

  • Adds a new backload form UI with task selection, date ranges, meta fields, and preview/execute functionality
  • Implements hourly task breakdown chart with filtering on the task dashboard
  • Extracts table sorting logic into a reusable module used across alert and workflow pages
  • Enhances about page with live uptime calculation and formatted row counts
  • Fixes cron parser initialization and rule syntax in configuration

Reviewed changes

Copilot reviewed 17 out of 19 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
handler.go Adds backload route, template handling, and hourly stats integration
handler_test.go Adds test coverage for backload HTML generation
stats.go Implements GetCountsWithHourlyFiltered method for task breakdown by hour
workflow.go Adds GetAllPhasesGrouped method for backload form data
taskmaster.go Fixes cron parser initialization to use cronParser
jobs.toml Fixes rule syntax from question mark to ampersand delimiter
backload.js New 644-line module for backload form with search, preview, and execution
table-sort.js New reusable table sorting module with URL persistence
task.js Adds result filter dropdown and initializes filters earlier
utils.js Improves HTML escaping and refactors event handling
backload.tmpl New template for backload form UI
header.tmpl Adds backload navigation link and mobile menu toggle
task.tmpl Adds hourly chart rendering and result filter dropdown
workflow.tmpl Refactored to use table-sort.js and utils.js modules
alert.tmpl Refactored to use table-sort.js and utils.js modules
about.tmpl Adds dynamic uptime calculation and number formatting
style.css Adds 493 lines of styles for backload, charts, and responsive navigation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const startTimeStr = document.getElementById('startTime').value;
if (!startTimeStr) return;

const startTime = new Date(startTimeStr);
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No validation is performed to check if startTime is a valid date after parsing. If the date string is malformed, the code will continue with an invalid date object. Consider adding a check: if (isNaN(startTime.getTime())) return; after creating the Date object.

Suggested change
const startTime = new Date(startTimeStr);
const startTime = new Date(startTimeStr);
if (isNaN(startTime.getTime())) return;

Copilot uses AI. Check for mistakes.
Comment on lines +374 to +376
if err != nil {
continue
}
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Errors from GetPhasesForWorkflow are silently ignored. Consider logging these errors to help with debugging workflow loading issues, as silent failures could make it difficult to diagnose why certain workflows aren't appearing in the backload form.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants