Skip to content

Commit 2b43fed

Browse files
committed
fix: address PR review findings for optional Actions tab
- Add enableActions to GET_CONFIG relay response - Handle disabled sentinel in WebSocketDataSource.getFailingActions - Add enableActions prop to PersonalSummaryStrip - Set hasPRActivity for PushEvent in parseRepoEvents - Extract isActionsBasedTab to shared schemas.ts - Remove unused actionsMonitoringDisabled from DashboardSummary - Add enableActions gating tests for MCP data-source and tools - Add custom tab reset and dropdown filtering tests - Update USER_GUIDE.md with Actions toggle documentation
1 parent 895ce34 commit 2b43fed

18 files changed

Lines changed: 213 additions & 34 deletions

File tree

docs/USER_GUIDE.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,8 @@ Sort by: Repo, Title, Author, Checks, Review, Size, Created, Updated (default: U
240240

241241
## Actions Tab
242242

243+
The Actions tab is enabled by default. You can disable it in **Settings > GitHub Actions > Show Actions tab**. Disabling it hides the tab, skips all workflow run API calls (saving REST API rate limit budget), suppresses workflow run notifications, and hides the "actions running" count from the summary strip. Custom tabs based on Actions are also hidden when disabled. Re-enabling restores the tab immediately; workflow run data refreshes on the next poll cycle.
244+
243245
### Workflow Grouping
244246

245247
Workflow runs are grouped first by repository, then by workflow name. Each workflow group shows its most recent runs up to the configured limit (default: 3 runs per workflow, up to 5 workflows per repo).
@@ -575,12 +577,13 @@ Settings are saved automatically to `localStorage` and persist across sessions.
575577
|---------|---------|-------------|
576578
| Refresh interval | 5 minutes | How often to poll GitHub for new data. Options: 1, 2, 5, 10, 15, 30 minutes, or Off. |
577579
| CI status refresh (hot poll interval) | 30 seconds | How often to re-check in-flight CI checks and workflow runs. Range: 10–120 seconds. |
578-
| Max workflows per repo | 5 | Number of active workflows to track per repository. Range: 1–20. |
579-
| Max runs per workflow | 3 | Number of recent runs to show per workflow. Range: 1–10. |
580+
| Show Actions tab | On | Show the Actions tab and track workflow runs. Disable to skip all workflow run API calls and simplify the dashboard. |
581+
| Max workflows per repo | 5 | Number of active workflows to track per repository. Range: 1–20. Disabled when Actions is off. |
582+
| Max runs per workflow | 3 | Number of recent runs to show per workflow. Range: 1–10. Disabled when Actions is off. |
580583
| Notifications enabled | Off | Master toggle for browser push notifications. |
581584
| Notify: Issues | On | Notify when new issues open (requires notifications enabled). |
582585
| Notify: Pull Requests | On | Notify when PRs are opened or updated (requires notifications enabled). |
583-
| Notify: Workflow Runs | On | Notify when workflow runs complete (requires notifications enabled). |
586+
| Notify: Workflow Runs | On | Notify when workflow runs complete (requires notifications enabled). Disabled when Actions is off. |
584587
| Theme | Auto | UI color theme. Auto follows system dark/light preference (Corporate for light, Dim for dark). |
585588
| View density | Comfortable | Spacing between list items. Options: Comfortable, Compact. |
586589
| Items per page | 25 | Number of items per page in each tab. Options: 10, 25, 50, 100. |

mcp/src/data-source.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@ export class OctokitDataSource implements DataSource {
455455

456456
const actionsEnabled = _cachedConfig?.enableActions !== false;
457457
if (repos.length === 0) {
458-
return { openPRCount: 0, openIssueCount: 0, failingRunCount: actionsEnabled ? 0 : null, needsReviewCount: 0, approvedUnmergedCount: 0, actionsMonitoringDisabled: !actionsEnabled };
458+
return { openPRCount: 0, openIssueCount: 0, failingRunCount: actionsEnabled ? 0 : null, needsReviewCount: 0, approvedUnmergedCount: 0 };
459459
}
460460

461461
const repoFilter = repos.map((r) => `repo:${r.owner}/${r.name}`).join("+");
@@ -507,7 +507,7 @@ export class OctokitDataSource implements DataSource {
507507
}
508508
}
509509

510-
return { openPRCount, openIssueCount, failingRunCount: finalFailingRunCount, needsReviewCount, approvedUnmergedCount, actionsMonitoringDisabled: !actionsEnabled };
510+
return { openPRCount, openIssueCount, failingRunCount: finalFailingRunCount, needsReviewCount, approvedUnmergedCount };
511511
}
512512

513513
async getConfig(): Promise<CachedConfig | null> {
@@ -536,7 +536,11 @@ export class WebSocketDataSource implements DataSource {
536536
}
537537

538538
async getFailingActions(repo?: string): Promise<WorkflowRun[]> {
539-
return sendRelayRequest(METHODS.GET_FAILING_ACTIONS, { repo }) as Promise<WorkflowRun[]>;
539+
const result = await sendRelayRequest(METHODS.GET_FAILING_ACTIONS, { repo });
540+
if (result !== null && typeof result === "object" && !Array.isArray(result) && "disabled" in (result as Record<string, unknown>)) {
541+
return [];
542+
}
543+
return result as WorkflowRun[];
540544
}
541545

542546
async getPRDetails(repo: string, number: number): Promise<PullRequest | null> {

mcp/tests/data-source.test.ts

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,14 @@ describe("OctokitDataSource", () => {
117117
trackedUsers: [],
118118
upstreamRepos: [],
119119
monitoredRepos: [],
120+
enableActions: true,
120121
});
121122
});
122123

123124
afterEach(() => {
124125
vi.restoreAllMocks();
125126
// Clear cached config
126-
setCachedConfig({ selectedRepos: [], trackedUsers: [], upstreamRepos: [], monitoredRepos: [] });
127+
setCachedConfig({ selectedRepos: [], trackedUsers: [], upstreamRepos: [], monitoredRepos: [], enableActions: true });
127128
});
128129

129130
describe("getOpenPRs", () => {
@@ -189,7 +190,7 @@ describe("OctokitDataSource", () => {
189190

190191
it("accepts explicit repo parameter and skips cached config", async () => {
191192
// Clear cached config to verify explicit param works without it
192-
setCachedConfig({ selectedRepos: [], trackedUsers: [], upstreamRepos: [], monitoredRepos: [] });
193+
setCachedConfig({ selectedRepos: [], trackedUsers: [], upstreamRepos: [], monitoredRepos: [], enableActions: true });
193194

194195
const responses = new Map([
195196
["GET /user", makeUserResponse()],
@@ -208,7 +209,7 @@ describe("OctokitDataSource", () => {
208209

209210
it("returns empty array when config has no repos and no explicit repo", async () => {
210211
// setCachedConfig with empty selectedRepos → resolveRepos returns []
211-
setCachedConfig({ selectedRepos: [], trackedUsers: [], upstreamRepos: [], monitoredRepos: [] });
212+
setCachedConfig({ selectedRepos: [], trackedUsers: [], upstreamRepos: [], monitoredRepos: [], enableActions: true });
212213
const responses = new Map([["GET /user", makeUserResponse()]]);
213214
const octokit = makeMockOctokit(responses);
214215
const ds = new OctokitDataSource(octokit);
@@ -433,11 +434,26 @@ describe("OctokitDataSource", () => {
433434
});
434435

435436
it("returns empty array when config has no repos and no explicit repo", async () => {
436-
setCachedConfig({ selectedRepos: [], trackedUsers: [], upstreamRepos: [], monitoredRepos: [] });
437+
setCachedConfig({ selectedRepos: [], trackedUsers: [], upstreamRepos: [], monitoredRepos: [], enableActions: true });
437438
const ds = new OctokitDataSource({ request: vi.fn() });
438439
const runs = await ds.getFailingActions();
439440
expect(runs).toEqual([]);
440441
});
442+
443+
it("returns empty array when enableActions is false", async () => {
444+
setCachedConfig({
445+
selectedRepos: [{ owner: "owner", name: "repo", fullName: "owner/repo" }],
446+
trackedUsers: [],
447+
upstreamRepos: [],
448+
monitoredRepos: [],
449+
enableActions: false,
450+
});
451+
const requestMock = vi.fn();
452+
const ds = new OctokitDataSource({ request: requestMock });
453+
const runs = await ds.getFailingActions();
454+
expect(runs).toEqual([]);
455+
expect(requestMock).not.toHaveBeenCalled();
456+
});
441457
});
442458

443459
describe("getPRDetails", () => {
@@ -502,7 +518,7 @@ describe("OctokitDataSource", () => {
502518

503519
describe("getDashboardSummary", () => {
504520
it("returns zero counts when no repos are configured", async () => {
505-
setCachedConfig({ selectedRepos: [], trackedUsers: [], upstreamRepos: [], monitoredRepos: [] });
521+
setCachedConfig({ selectedRepos: [], trackedUsers: [], upstreamRepos: [], monitoredRepos: [], enableActions: true });
506522
const octokit = makeMockOctokit(new Map([["GET /user", makeUserResponse()]]));
507523
const ds = new OctokitDataSource(octokit);
508524
const summary = await ds.getDashboardSummary("involves_me");
@@ -514,6 +530,16 @@ describe("OctokitDataSource", () => {
514530
expect(summary.approvedUnmergedCount).toBe(0);
515531
});
516532

533+
it("returns failingRunCount=null in early return when no repos and enableActions is false", async () => {
534+
setCachedConfig({ selectedRepos: [], trackedUsers: [], upstreamRepos: [], monitoredRepos: [], enableActions: false });
535+
const octokit = makeMockOctokit(new Map([["GET /user", makeUserResponse()]]));
536+
const ds = new OctokitDataSource(octokit);
537+
const summary = await ds.getDashboardSummary("involves_me");
538+
539+
expect(summary.failingRunCount).toBeNull();
540+
expect(summary.openPRCount).toBe(0);
541+
});
542+
517543
it("constructs involves_me query with user login", async () => {
518544
const requestMock = vi.fn().mockImplementation(async (route: string) => {
519545
if (route === "GET /user") return { data: { login: "testuser" }, headers: {} };
@@ -556,6 +582,30 @@ describe("OctokitDataSource", () => {
556582
expect(prCall).toBeDefined();
557583
expect(prCall![1].q).not.toContain("involves:");
558584
});
585+
586+
it("returns failingRunCount=null and skips actions API when enableActions is false", async () => {
587+
setCachedConfig({
588+
selectedRepos: [{ owner: "owner", name: "repo", fullName: "owner/repo" }],
589+
trackedUsers: [],
590+
upstreamRepos: [],
591+
monitoredRepos: [],
592+
enableActions: false,
593+
});
594+
const requestMock = vi.fn().mockImplementation(async (route: string) => {
595+
if (route === "GET /user") return { data: { login: "testuser" }, headers: {} };
596+
if (route === "GET /search/issues") return { data: { items: [], total_count: 0 }, headers: {} };
597+
throw new Error(`Unexpected: ${route}`);
598+
});
599+
600+
const ds = new OctokitDataSource({ request: requestMock });
601+
const summary = await ds.getDashboardSummary("involves_me");
602+
603+
expect(summary.failingRunCount).toBeNull();
604+
const actionsCalls = requestMock.mock.calls.filter(
605+
([route]: [string]) => route === "GET /repos/{owner}/{repo}/actions/runs"
606+
);
607+
expect(actionsCalls).toHaveLength(0);
608+
});
559609
});
560610

561611
describe("getConfig", () => {
@@ -565,6 +615,7 @@ describe("OctokitDataSource", () => {
565615
trackedUsers: [],
566616
upstreamRepos: [],
567617
monitoredRepos: [],
618+
enableActions: true,
568619
};
569620
setCachedConfig(config);
570621
const ds = new OctokitDataSource({ request: vi.fn() });
@@ -641,6 +692,7 @@ describe("CompositeDataSource", () => {
641692
trackedUsers: [],
642693
upstreamRepos: [],
643694
monitoredRepos: [],
695+
enableActions: true,
644696
});
645697
});
646698

@@ -755,4 +807,13 @@ describe("CompositeDataSource", () => {
755807
expect(octokitDs.getRateLimit).toHaveBeenCalled();
756808
expect(_mockSendRequest).not.toHaveBeenCalled();
757809
});
810+
811+
it("WebSocketDataSource.getFailingActions returns empty array for disabled sentinel", async () => {
812+
_mockIsConnected = true;
813+
_mockSendRequest = vi.fn().mockResolvedValue({ disabled: true, message: "Actions monitoring is disabled" });
814+
815+
const wsDs = new WebSocketDataSource();
816+
const result = await wsDs.getFailingActions();
817+
expect(result).toEqual([]);
818+
});
758819
});

mcp/tests/integration.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,7 @@ describe("Integration: Edge cases (with server)", () => {
439439
trackedUsers: [],
440440
upstreamRepos: [],
441441
monitoredRepos: [],
442+
enableActions: true,
442443
});
443444

444445
if (!wss) throw new Error("Server not started");
@@ -461,7 +462,7 @@ describe("Integration: Edge cases (with server)", () => {
461462
getRateLimit: vi.fn(),
462463
getConfig: vi.fn().mockResolvedValue({
463464
selectedRepos: [{ owner: "acme", name: "app", fullName: "acme/app" }],
464-
trackedUsers: [], upstreamRepos: [], monitoredRepos: [],
465+
trackedUsers: [], upstreamRepos: [], monitoredRepos: [], enableActions: true,
465466
}),
466467
getRepos: vi.fn().mockResolvedValue([{ owner: "acme", name: "app", fullName: "acme/app" }]),
467468
};

mcp/tests/resources.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ function makeConfig(overrides: Partial<CachedConfig> = {}): CachedConfig {
5858
trackedUsers: [],
5959
upstreamRepos: [],
6060
monitoredRepos: [],
61+
enableActions: true,
6162
...overrides,
6263
};
6364
}

mcp/tests/tools.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,22 @@ describe("get_dashboard_summary", () => {
125125
// isRelayConnected is mocked to return false, so staleness note should be present
126126
expect(result.content[0].text).toContain("data via GitHub API");
127127
});
128+
129+
it("shows actions disabled indicator when failingRunCount is null", async () => {
130+
const disabledSummary: DashboardSummary = {
131+
openPRCount: 2,
132+
openIssueCount: 1,
133+
failingRunCount: null,
134+
needsReviewCount: 0,
135+
approvedUnmergedCount: 0,
136+
};
137+
vi.mocked(ds.getDashboardSummary).mockResolvedValueOnce(disabledSummary);
138+
const result = await callTool(server, "get_dashboard_summary");
139+
expect(result.isError).toBeFalsy();
140+
const text = result.content[0].text;
141+
expect(text).toContain("Actions monitoring disabled");
142+
expect(text).not.toMatch(/Failing CI Runs:\s+\d/);
143+
});
128144
});
129145

130146
describe("get_open_prs", () => {
@@ -315,6 +331,41 @@ describe("get_failing_actions", () => {
315331
expect(result.isError).toBe(true);
316332
expect(result.content[0].text).toContain("Error fetching workflow runs");
317333
});
334+
335+
it("returns disabled message when enableActions is false", async () => {
336+
vi.mocked(ds.getConfig).mockResolvedValueOnce({
337+
selectedRepos: [{ owner: "owner", name: "repo", fullName: "owner/repo" }],
338+
trackedUsers: [],
339+
upstreamRepos: [],
340+
monitoredRepos: [],
341+
enableActions: false,
342+
});
343+
const result = await callTool(server, "get_failing_actions");
344+
expect(result.isError).toBeFalsy();
345+
expect(result.content[0].text).toContain("Actions monitoring is disabled");
346+
expect(ds.getFailingActions).not.toHaveBeenCalled();
347+
});
348+
349+
it("proceeds normally when enableActions is true", async () => {
350+
vi.mocked(ds.getConfig).mockResolvedValueOnce({
351+
selectedRepos: [{ owner: "owner", name: "repo", fullName: "owner/repo" }],
352+
trackedUsers: [],
353+
upstreamRepos: [],
354+
monitoredRepos: [],
355+
enableActions: true,
356+
});
357+
const result = await callTool(server, "get_failing_actions");
358+
expect(result.isError).toBeFalsy();
359+
expect(result.content[0].text).toContain("No failing or in-progress workflow runs found");
360+
expect(ds.getFailingActions).toHaveBeenCalled();
361+
});
362+
363+
it("proceeds normally when getConfig returns null", async () => {
364+
vi.mocked(ds.getConfig).mockResolvedValueOnce(null);
365+
const result = await callTool(server, "get_failing_actions");
366+
expect(result.isError).toBeFalsy();
367+
expect(ds.getFailingActions).toHaveBeenCalled();
368+
});
318369
});
319370

320371
describe("get_pr_details", () => {

src/app/components/dashboard/DashboardPage.tsx

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import IssuesTab from "./IssuesTab";
88
import PullRequestsTab from "./PullRequestsTab";
99
import TrackedTab from "./TrackedTab";
1010
import PersonalSummaryStrip from "./PersonalSummaryStrip";
11-
import { config, setConfig, getCustomTab, isBuiltinTab, updateJiraConfig, type TrackedUser } from "../../stores/config";
11+
import { config, setConfig, getCustomTab, isBuiltinTab, isActionsBasedTab, updateJiraConfig, type TrackedUser } from "../../stores/config";
1212
import { viewState, updateViewState, setSortPreference, pruneClosedTrackedItems, removeCustomTabState, untrackJiraItem, setTabFilter, IssueFiltersSchema, PullRequestFiltersSchema, ActionsFiltersSchema } from "../../stores/view";
1313
import type { SortOption } from "../shared/SortDropdown";
1414
import type { Issue, PullRequest, WorkflowRun } from "../../services/api";
@@ -530,15 +530,11 @@ export default function DashboardPage() {
530530
};
531531
}
532532

533-
function isActionsBasedTab(tab: TabId): boolean {
534-
return tab === "actions" || (!isBuiltinTab(tab) && config.customTabs.some((t) => t.id === tab && t.baseType === "actions"));
535-
}
536-
537533
function resolveInitialTab(): TabId {
538534
const tab = config.rememberLastTab ? viewState.lastActiveTab : config.defaultTab;
539535
if (tab === "tracked" && !config.enableTracking) return "issues";
540536
if (tab === "jiraAssigned" && !config.jira?.enabled) return "issues";
541-
if (!config.enableActions && isActionsBasedTab(tab)) return "issues";
537+
if (!config.enableActions && isActionsBasedTab(tab, config.customTabs)) return "issues";
542538
// Validate custom tab still exists; fall back to "issues" if stale
543539
if (!isBuiltinTab(tab) && !config.customTabs.some((t) => t.id === tab)) return "issues";
544540
return tab;
@@ -549,7 +545,7 @@ export default function DashboardPage() {
549545
function handleTabChange(tab: TabId) {
550546
// Reject invalid tab IDs to prevent persisting stale state
551547
if (!isBuiltinTab(tab) && !config.customTabs.some((t) => t.id === tab)) return;
552-
if (!config.enableActions && isActionsBasedTab(tab)) return;
548+
if (!config.enableActions && isActionsBasedTab(tab, config.customTabs)) return;
553549
setActiveTab(tab);
554550
updateViewState({ lastActiveTab: tab });
555551
}
@@ -579,7 +575,7 @@ export default function DashboardPage() {
579575

580576
// Redirect away from Actions tab (or actions-based custom tab) when Actions is disabled
581577
createEffect(() => {
582-
if (!config.enableActions && isActionsBasedTab(activeTab())) {
578+
if (!config.enableActions && isActionsBasedTab(activeTab(), config.customTabs)) {
583579
handleTabChange("issues");
584580
}
585581
});
@@ -1110,6 +1106,7 @@ export default function DashboardPage() {
11101106
pullRequests={visiblePullRequests()}
11111107
workflowRuns={visibleWorkflowRuns()}
11121108
userLogin={userLogin()}
1109+
enableActions={config.enableActions}
11131110
onTabChange={handleTabChange}
11141111
/>
11151112
<TabBar

src/app/components/dashboard/PersonalSummaryStrip.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ interface PersonalSummaryStripProps {
1717
pullRequests: PullRequest[];
1818
workflowRuns: WorkflowRun[];
1919
userLogin: string;
20+
enableActions?: boolean;
2021
onTabChange: (tab: TabId) => void;
2122
}
2223

@@ -142,7 +143,7 @@ export default function PersonalSummaryStrip(props: PersonalSummaryStripProps) {
142143
}));
143144
},
144145
});
145-
if (running > 0) items.push({
146+
if (running > 0 && props.enableActions !== false) items.push({
146147
label: running === 1 ? "action running" : "actions running",
147148
count: running,
148149
tab: "actions",

src/app/components/settings/SettingsPage.tsx

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { createSignal, createMemo, Show, For, onCleanup, onMount } from "solid-j
22
import * as Sentry from "@sentry/solid";
33
import { getRelayStatus } from "../../lib/mcp-relay";
44
import { useNavigate } from "@solidjs/router";
5-
import { config, updateConfig, updateJiraConfig, updateJiraCustomFields, updateJiraCustomScopes, setMonitoredRepo } from "../../stores/config";
5+
import { config, updateConfig, updateJiraConfig, updateJiraCustomFields, updateJiraCustomScopes, setMonitoredRepo, isActionsBasedTab } from "../../stores/config";
66
import type { Config } from "../../stores/config";
77
import { viewState, updateViewState } from "../../stores/view";
88
import { clearAuth, jiraAuth, setJiraAuth, clearJiraConfigFull, isJiraAuthenticated } from "../../stores/auth";
@@ -620,10 +620,8 @@ export default function SettingsPage() {
620620
checked={config.enableActions}
621621
onChange={(e) => {
622622
const val = e.currentTarget.checked;
623-
const isActionsCustomTab = (id: string) =>
624-
config.customTabs.some((t) => t.id === id && t.baseType === "actions");
625-
const needsDefaultReset = !val && (config.defaultTab === "actions" || isActionsCustomTab(config.defaultTab));
626-
const needsLastTabReset = !val && (viewState.lastActiveTab === "actions" || isActionsCustomTab(viewState.lastActiveTab));
623+
const needsDefaultReset = !val && isActionsBasedTab(config.defaultTab, config.customTabs);
624+
const needsLastTabReset = !val && isActionsBasedTab(viewState.lastActiveTab, config.customTabs);
627625
saveWithFeedback({
628626
enableActions: val,
629627
...(needsDefaultReset ? { defaultTab: "issues" as const } : {}),

0 commit comments

Comments
 (0)