feat: add healthy filter for workspace queries#21743
Conversation
Adds support for filtering workspaces by health status using healthy:true or healthy:false in the search query. The filter only applies to running workspaces (status=running or unset). A workspace is considered unhealthy if any of its agents are: - Disconnected - Timed out waiting to connect - Lost connection (no recent heartbeat) - In error or shutting down lifecycle states Fixes #21623
- healthy:true translates to has-agent:connected - healthy:false translates to has-agent:disconnected,timeout - Changed has-agent to accept multiple statuses (text array) - Removed duplicate healthy SQL block
Documentation CheckUpdates Needed
ContextThis PR adds support for filtering workspaces by health status using
The Automated review via Coder Tasks |
![coder-tasks[bot]](https://avatars.githubusercontent.com/in/2652121?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Reviewed PR #21743: "feat: add healthy filter for workspace queries"
The implementation looks good overall. The approach of aliasing healthy:true to has-agent:connected and healthy:false to has-agent:disconnected,timeout is clean and leverages the existing filtering infrastructure.
Key Observations
Strengths:
- ✅ Changes
has-agentfrom single string to array of statuses, which is more flexible - ✅ SQL query properly uses
= ANY(@has_agent_statuses :: text[])for array matching - ✅ Comprehensive test coverage for both
healthy:trueandhealthy:falsescenarios - ✅ Tests verify correct behavior with connected, disconnected, and timeout agent states
- ✅ The healthy filter is implemented as an append operation, allowing it to combine with explicit
has-agentfilters
Potential Issues:
-
Filter Combination Behavior (Minor): The current implementation appends statuses when
healthyis used. This means:has-agent:connected healthy:falsewould result in["connected", "disconnected", "timeout"]- effectively matching all workspaces- This might be confusing but is probably acceptable as an edge case
- Consider adding a test case to document this behavior or add validation to prevent conflicting filters
-
Missing Test Coverage: No test cases verify the interaction between explicit
has-agentfilters and the newhealthyfilter when used together
Minor Suggestions
The code follows Coder's coding standards well. The implementation is clean and maintainable.
| filter.HasAgentStatuses = append(filter.HasAgentStatuses, "connected") | ||
| } else { | ||
| filter.HasAgentStatuses = append(filter.HasAgentStatuses, "disconnected", "timeout") | ||
| } |
There was a problem hiding this comment.
Question about filter combination behavior:
When both has-agent and healthy filters are used together (e.g., has-agent:connecting healthy:true), they will be combined with append(), potentially creating conflicting filter conditions.
For example:
has-agent:connected healthy:false→["connected", "disconnected", "timeout"]
This would match workspaces in any state, which might not be the user's intent.
Consider either:
- Adding validation to prevent conflicting combinations
- Documenting this behavior explicitly
- Adding a test case that demonstrates the expected behavior when both filters are used
Which behavior is intended?
There was a problem hiding this comment.
If someone is mixing up things like this, just allow it all to be searched. Let's not try to guess what they really wanted.
| Name: "HealthyMissing", | ||
| Query: "", | ||
| Expected: database.GetWorkspacesParams{ | ||
| HasAgentStatuses: []string{}, |
There was a problem hiding this comment.
Suggestion: Add test case for filter combination
Consider adding a test case to verify behavior when both has-agent and healthy are used together:
{
Name: "HealthyAndHasAgent",
Query: "has-agent:connecting healthy:true",
Expected: database.GetWorkspacesParams{
HasAgentStatuses: []string{"connecting", "connected"},
},
},This would document the expected behavior when filters are combined.
Documentation CheckUpdates Needed
The documentation correctly describes the feature, but uses inconsistent naming. The filter name should be Current (incorrect): - `health` - Only applicable for workspaces in "start" transition...Should be: - `healthy` - Only applicable for workspaces in "start" transition...Automated review via Coder Tasks |
Emyrk
left a comment
Emyrk
left a comment
There was a problem hiding this comment.
LG, just add that test suggestion
| Name: "HealthyMissing", | ||
| Query: "", | ||
| Expected: database.GetWorkspacesParams{ | ||
| HasAgentStatuses: []string{}, |
2 participants
Adds support for filtering workspaces by health status using healthy:true or healthy:false in the search query.
This is done by changing
has-agentto accept a list of statuses and aliasinghealth:truetohas-agent:connectedandhealthy:falsetohas-agent:timeout,disconnected.Fixes #21623