refactor(auto-update): retire per-stack gate, drive auto-update from schedules only (#1233)

* refactor(auto-update): retire per-stack gate, drive auto-update from schedules only

The per-stack Auto-update toggle in the stack sidebar context menu wrote a
gate row to `stack_auto_update_settings`, but actual updates only ran when a
`scheduled_tasks` row with `action='update'` fired. On a fresh install the
toggle was inert: detection ran every 6h, nothing was applied.

The same context menu already exposes `Schedule task`, which opens
ScheduledOperationsView pre-filled for the stack where the user can pick
`Auto-update Stack` and any cron. Keeping the toggle alongside that flow
duplicated the same action and turned the gate table into a parallel store
of "is a covering schedule active" derivable from `scheduled_tasks` itself.

Drop the gate model entirely:
- Backend: remove the `stack_auto_update_settings` table and its four
  accessors, the three routes under /api/stacks/*/auto-update, the per-stack
  skip in /api/auto-update/execute and SchedulerService.executeUpdate's
  fleet branch, and the clearStackAutoUpdateSetting call on stack delete.
  Dashboard `autoUpdate` count derives from scheduled_tasks (action='update'
  rows pinned to the node, total/enabled split).
- Frontend: drop the Auto-update entry from the sidebar context menu and its
  optimistic toggle plumbing. Drop autoUpdateSettings state, the
  /stacks/auto-update-settings fetch, and the auto-update-settings-changed
  WebSocket branch. Slim useSidebarActivitySummary (just nextRunAt; no
  enabled/total counts). AutoUpdateReadinessView's per-card autoUpdateEnabled
  now means "a covering enabled action='update' schedule exists" (per-stack
  row or fleet row on this node, earliest next_run_at wins, per-stack row
  wins on ties), with the gate-fetch removed.
- New: scheduledTasksRouter broadcasts scope: 'scheduled-tasks' on POST,
  PUT, PATCH /toggle, and DELETE so useConfigurationStatus and
  useNextAutoUpdateRun refetch under the 250ms debounce instead of waiting
  for the 60s poll. The broadcast is wrapped so a broken subscriber socket
  cannot turn a successful mutation into a 500.
- Docs: rewrite the "Per-stack control" section of auto-update-policies.mdx
  to describe the schedule-based model; update the matching troubleshooting
  entry. The misleading fleet-update help text in ScheduledOperationsView
  is corrected to reflect that every stack on the node is covered.

Tier parity: the surviving auto-update path (Schedule task -> Auto-update
Stack / All Stacks) is gated `requirePaid + requireAdmin` backend and
`isPaid + isAdmin` frontend, matching the gate the deleted routes carried.
The pre-commit grep returns no tier-related diff outside this PR's scope.

No data migration is provided: greenfield rules apply, and the leftover
table on already-shipped instances is harmless because no code reads or
writes it after this PR.

* docs: sweep remaining references to the per-stack auto-update toggle

The previous commit retired the per-stack Auto-update gate in favor of
configuring auto-update purely through scheduled tasks. This commit
removes the now-stale mentions of that toggle across the operator docs:

- docs/features/sidebar.mdx: drop the Auto-update entry from the Inspect
  group description, the matching screenshot alt-text, and the Skipper
  Note that listed it. Schedule task now carries the cross-link to
  Auto-Update Policies.
- docs/features/stack-management.mdx: drop the Auto-update list item;
  refresh the Schedule task entry to mention the Auto-update Stack action.
- docs/features/dashboard.mdx: rename the Configuration Status row from
  "Auto-update stacks" to "Auto-update schedules" with the new value
  shape, and rewrite the troubleshooting accordion to describe the
  scheduled-tasks invalidation path.
- docs/features/scheduled-operations.mdx: rewrite the Auto-update All
  Stacks row and helper text to reflect that every stack on the node is
  covered (no per-stack opt-out from this surface anymore).
- docs/features/multi-node.mdx: rewrite the Updates column definition to
  derive the Auto/Off flag from enabled Auto-update Stack / Auto-update
  All Stacks schedules instead of the removed per-stack policy.

The auto-update-policies.mdx rewrite in the previous commit already
covered the main reference page. The sidebar-context-menu.png screenshot
will be refreshed on release once the new menu is live in production;
the alt text is updated in this commit so it accurately describes the
shipping state.

No website edits needed: the Auto-Update Policies feature card description
("Schedule automatic image pulls and redeployments per stack on your own
cadence") and the feature matrix labels ("Auto-update stack schedule",
"Auto-update all stacks schedule") remain accurate under the new model.

* fix(stacks): drop orphaned requireAdmin import after auto-update route removal

CI's backend lint step flagged this PR's earlier deletion of the three
/api/stacks/*/auto-update routes: those handlers were the only callers of
`requireAdmin` inside routes/stacks.ts, leaving the named import on line 15
unreferenced. `requirePaid` and `effectiveTier` from the same line are still
in use elsewhere in the file and stay.

tsc --noEmit does not flag unused named imports; ESLint's no-unused-vars
does. Local backend lint reproduces and now reports 0 errors against the
existing 334-warning baseline.
This commit is contained in:
Anso
2026-05-26 11:08:33 -04:00
committed by GitHub
parent 2a29fed117
commit adcd04b01a
36 changed files with 295 additions and 668 deletions
@@ -112,6 +112,77 @@ describe('GET /api/dashboard/configuration', () => {
expect(res.body.security.scanPolicies.locked).toBe(false);
});
it('counts autoUpdate as enabled action=update scheduled tasks targeting this node, not other actions', async () => {
const db = DatabaseService.getInstance();
const now = Date.now();
const nodeId = 1;
const baseTask = {
created_by: 'admin',
created_at: now,
updated_at: now,
last_run_at: null,
next_run_at: now + 3600_000,
last_status: null,
last_error: null,
prune_targets: null,
target_services: null,
prune_label_filter: null,
};
const idA = db.createScheduledTask({
...baseTask,
name: 'au-on',
target_type: 'stack',
target_id: 'app1',
node_id: nodeId,
action: 'update',
cron_expression: '0 3 * * *',
enabled: 1,
});
const idB = db.createScheduledTask({
...baseTask,
name: 'au-off',
target_type: 'stack',
target_id: 'app2',
node_id: nodeId,
action: 'update',
cron_expression: '0 3 * * *',
enabled: 0,
});
const idC = db.createScheduledTask({
...baseTask,
name: 'scan-row',
target_type: 'system',
target_id: null,
node_id: nodeId,
action: 'scan',
cron_expression: '0 3 * * *',
enabled: 1,
});
const idD = db.createScheduledTask({
...baseTask,
name: 'au-other-node',
target_type: 'stack',
target_id: 'app3',
node_id: 999,
action: 'update',
cron_expression: '0 3 * * *',
enabled: 1,
});
try {
const res = await request(app).get('/api/dashboard/configuration').set('Cookie', adminCookie);
expect(res.status).toBe(200);
// Two update rows on node 1 (one enabled, one disabled); the scan row
// and the update row on node 999 must not leak into the count.
expect(res.body.automation.autoUpdate.total).toBe(2);
expect(res.body.automation.autoUpdate.enabled).toBe(1);
} finally {
db.deleteScheduledTask(idA);
db.deleteScheduledTask(idB);
db.deleteScheduledTask(idC);
db.deleteScheduledTask(idD);
}
});
it('does not leak agent URLs, tokens, or other secret material in the response', async () => {
// Seed a node-1 agent with a Discord URL so the configuration path
// exercises the `configured` truthy branch. The URL must never appear
@@ -562,3 +562,80 @@ describe('PUT /api/scheduled-tasks/:id - stack target validation', () => {
expect(res.body.error).toMatch(/Invalid action/);
});
});
describe('scheduled-tasks state-invalidate broadcast', () => {
// Two frontend hooks (useConfigurationStatus, useNextAutoUpdateRun) refetch
// when this scope fires; locking it down prevents a silent UX regression
// where a successful mutation no longer triggers their fast-refresh path.
it('fires scope=scheduled-tasks on create, update, toggle, and delete', async () => {
const { NotificationService } = await import('../services/NotificationService');
const broadcastSpy = vi.spyOn(NotificationService.getInstance(), 'broadcastEvent').mockImplementation(() => undefined);
try {
const expectScheduledTasksBroadcast = () => {
expect(broadcastSpy).toHaveBeenCalledWith(expect.objectContaining({
type: 'state-invalidate',
scope: 'scheduled-tasks',
ts: expect.any(Number),
}));
};
const createRes = await request(app)
.post('/api/scheduled-tasks')
.set('Cookie', adminCookie)
.send({
name: 'broadcast-test',
target_type: 'system',
node_id: 1,
action: 'prune',
cron_expression: '0 4 * * *',
enabled: true,
prune_targets: ['images'],
});
expect(createRes.status).toBe(201);
const taskId = createRes.body.id as number;
expectScheduledTasksBroadcast();
broadcastSpy.mockClear();
const updateRes = await request(app)
.put(`/api/scheduled-tasks/${taskId}`)
.set('Cookie', adminCookie)
.send({ cron_expression: '0 5 * * *' });
expect(updateRes.status).toBe(200);
expectScheduledTasksBroadcast();
broadcastSpy.mockClear();
const toggleRes = await request(app)
.patch(`/api/scheduled-tasks/${taskId}/toggle`)
.set('Cookie', adminCookie);
expect(toggleRes.status).toBe(200);
expectScheduledTasksBroadcast();
broadcastSpy.mockClear();
const deleteRes = await request(app)
.delete(`/api/scheduled-tasks/${taskId}`)
.set('Cookie', adminCookie);
expect(deleteRes.status).toBe(200);
expectScheduledTasksBroadcast();
} finally {
broadcastSpy.mockRestore();
}
});
it('does not broadcast when validation rejects the request', async () => {
const { NotificationService } = await import('../services/NotificationService');
const broadcastSpy = vi.spyOn(NotificationService.getInstance(), 'broadcastEvent').mockImplementation(() => undefined);
try {
const res = await request(app)
.post('/api/scheduled-tasks')
.set('Cookie', adminCookie)
.send({ name: '', target_type: 'system', action: 'prune', cron_expression: '0 4 * * *' });
expect(res.status).toBe(400);
const scheduledBroadcasts = broadcastSpy.mock.calls.filter(
([event]) => (event as { scope?: string }).scope === 'scheduled-tasks',
);
expect(scheduledBroadcasts).toHaveLength(0);
} finally {
broadcastSpy.mockRestore();
}
});
});
@@ -21,7 +21,6 @@ const {
mockGetProxyTarget,
mockIsTrivyAvailable,
mockScanAllNodeImages,
mockGetStackAutoUpdateSettingsForNode,
mockDeleteScheduledTask,
mockGetMatchingPolicy,
mockRunCommand,
@@ -62,7 +61,6 @@ const {
severity: { critical: 0, high: 0, medium: 0, low: 0, unknown: 0 },
violations: [],
}),
mockGetStackAutoUpdateSettingsForNode: vi.fn().mockReturnValue({}),
mockDeleteScheduledTask: vi.fn(),
mockGetMatchingPolicy: vi.fn().mockReturnValue(null),
mockRunCommand: vi.fn().mockResolvedValue(undefined),
@@ -86,7 +84,6 @@ vi.mock('../services/DatabaseService', () => ({
clearStackUpdateStatus: mockClearStackUpdateStatus,
markStaleRunsAsFailed: mockMarkStaleRunsAsFailed,
deleteOldScans: mockDeleteOldScans,
getStackAutoUpdateSettingsForNode: mockGetStackAutoUpdateSettingsForNode,
deleteScheduledTask: mockDeleteScheduledTask,
getMatchingPolicy: mockGetMatchingPolicy,
}),
@@ -790,7 +787,7 @@ describe('SchedulerService - executeUpdate', () => {
expect(svc.isTaskRunning(999)).toBe(false);
});
it('fleet target updates all stacks whose policy allows it', async () => {
it('fleet target updates every stack discovered on the node', async () => {
mockGetScheduledTask.mockReturnValue({
id: 87,
name: 'fleet-update',
@@ -804,45 +801,13 @@ describe('SchedulerService - executeUpdate', () => {
last_status: null,
});
mockGetStacks.mockResolvedValue(['app1', 'app2', 'app3']);
// app2 explicitly disabled; app1 and app3 default to enabled
mockGetStackAutoUpdateSettingsForNode.mockReturnValue({ app2: false });
mockGetContainersByStack.mockResolvedValue([{ Id: 'c1', Image: 'nginx:latest' }]);
mockCheckImage.mockResolvedValue({ hasUpdate: true });
const svc = SchedulerService.getInstance();
await svc.triggerTask(87);
// Only app1 and app3 should be updated
expect(mockUpdateStack).toHaveBeenCalledTimes(2);
expect(mockUpdateScheduledTaskRun).toHaveBeenCalledWith(
1,
expect.objectContaining({
status: 'success',
output: expect.stringContaining('auto-updates disabled; skipped'),
})
);
});
it('fleet target with zero eligible stacks records success', async () => {
mockGetScheduledTask.mockReturnValue({
id: 88,
name: 'fleet-update-all-off',
action: 'update',
target_type: 'fleet',
cron_expression: '0 4 * * *',
enabled: true,
target_id: null,
node_id: 1,
created_by: 'admin',
last_status: null,
});
mockGetStacks.mockResolvedValue(['app1', 'app2']);
mockGetStackAutoUpdateSettingsForNode.mockReturnValue({ app1: false, app2: false });
const svc = SchedulerService.getInstance();
await svc.triggerTask(88);
expect(mockUpdateStack).not.toHaveBeenCalled();
expect(mockUpdateStack).toHaveBeenCalledTimes(3);
expect(mockUpdateScheduledTaskRun).toHaveBeenCalledWith(
1,
expect.objectContaining({ status: 'success' })
@@ -1,250 +0,0 @@
/**
* Tests for per-stack auto-update settings:
* - DatabaseService accessors (round-trip, defaults)
* - GET/PUT /api/stacks/auto-update-settings and /api/stacks/:name/auto-update
* - /api/auto-update/execute skips disabled stacks
*/
import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
import request from 'supertest';
import bcrypt from 'bcrypt';
import { setupTestDb, cleanupTestDb, loginAsTestAdmin } from './helpers/setupTestDb';
let tmpDir: string;
let app: import('express').Express;
let DatabaseService: typeof import('../services/DatabaseService').DatabaseService;
let adminCookie: string;
let viewerCookie: string;
beforeAll(async () => {
tmpDir = await setupTestDb();
({ DatabaseService } = await import('../services/DatabaseService'));
const { LicenseService } = await import('../services/LicenseService');
vi.spyOn(LicenseService.getInstance(), 'getTier').mockReturnValue('paid');
vi.spyOn(LicenseService.getInstance(), 'getVariant').mockReturnValue('admiral');
vi.spyOn(LicenseService.getInstance(), 'getSeatLimits').mockReturnValue({ maxAdmins: null, maxViewers: null });
({ app } = await import('../index'));
adminCookie = await loginAsTestAdmin(app);
const viewerHash = await bcrypt.hash('viewerpass2', 1);
DatabaseService.getInstance().addUser({ username: 'aus-viewer', password_hash: viewerHash, role: 'viewer' });
const viewerRes = await request(app).post('/api/auth/login').send({ username: 'aus-viewer', password: 'viewerpass2' });
const cookies = viewerRes.headers['set-cookie'] as string | string[];
viewerCookie = Array.isArray(cookies) ? cookies[0] : cookies;
});
afterAll(() => cleanupTestDb(tmpDir));
describe('DatabaseService - stack auto-update settings', () => {
it('returns true by default when no row exists', () => {
const db = DatabaseService.getInstance();
const result = db.getStackAutoUpdateEnabled(0, 'no-such-stack');
expect(result).toBe(true);
});
it('upsert → get round-trip (disable)', () => {
const db = DatabaseService.getInstance();
db.upsertStackAutoUpdateEnabled(0, 'test-stack', false);
expect(db.getStackAutoUpdateEnabled(0, 'test-stack')).toBe(false);
});
it('upsert → get round-trip (re-enable)', () => {
const db = DatabaseService.getInstance();
db.upsertStackAutoUpdateEnabled(0, 'test-stack', true);
expect(db.getStackAutoUpdateEnabled(0, 'test-stack')).toBe(true);
});
it('getStackAutoUpdateSettingsForNode only returns stacks with explicit rows', () => {
const db = DatabaseService.getInstance();
db.upsertStackAutoUpdateEnabled(0, 'explicit-stack', false);
const settings = db.getStackAutoUpdateSettingsForNode(0);
expect('explicit-stack' in settings).toBe(true);
expect(settings['explicit-stack']).toBe(false);
});
it('clearStackAutoUpdateSetting removes the row (reverts to default true)', () => {
const db = DatabaseService.getInstance();
db.upsertStackAutoUpdateEnabled(0, 'to-clear', false);
db.clearStackAutoUpdateSetting(0, 'to-clear');
expect(db.getStackAutoUpdateEnabled(0, 'to-clear')).toBe(true);
const settings = db.getStackAutoUpdateSettingsForNode(0);
expect('to-clear' in settings).toBe(false);
});
});
describe('GET /api/stacks/auto-update-settings', () => {
it('rejects unauthenticated requests with 401', async () => {
const res = await request(app).get('/api/stacks/auto-update-settings');
expect(res.status).toBe(401);
});
it('returns an object for authenticated admin', async () => {
const res = await request(app).get('/api/stacks/auto-update-settings').set('Cookie', adminCookie);
expect(res.status).toBe(200);
expect(typeof res.body).toBe('object');
});
it('returns an object for authenticated viewer (read-only)', async () => {
const res = await request(app).get('/api/stacks/auto-update-settings').set('Cookie', viewerCookie);
expect(res.status).toBe(200);
});
});
describe('GET /api/stacks/:stackName/auto-update', () => {
it('rejects unauthenticated requests with 401', async () => {
const res = await request(app).get('/api/stacks/mystack/auto-update');
expect(res.status).toBe(401);
});
it('returns enabled:true by default', async () => {
const res = await request(app).get('/api/stacks/nonexistent/auto-update').set('Cookie', adminCookie);
expect(res.status).toBe(200);
expect(res.body.enabled).toBe(true);
});
it('rejects invalid stack names with 400', async () => {
// Dots are rejected by isValidStackName; unlike path-traversal sequences
// they are not normalised away by Express routing.
const res = await request(app).get('/api/stacks/my.stack/auto-update').set('Cookie', adminCookie);
expect(res.status).toBe(400);
});
});
describe('PUT /api/stacks/:stackName/auto-update', () => {
it('rejects unauthenticated requests with 401', async () => {
const res = await request(app)
.put('/api/stacks/mystack/auto-update')
.send({ enabled: false });
expect(res.status).toBe(401);
});
it('rejects viewer with 403', async () => {
const res = await request(app)
.put('/api/stacks/mystack/auto-update')
.set('Cookie', viewerCookie)
.send({ enabled: false });
expect(res.status).toBe(403);
});
it('rejects Community tier with 403', async () => {
const { LicenseService } = await import('../services/LicenseService');
const spy = vi.spyOn(LicenseService.getInstance(), 'getTier').mockReturnValue('community');
try {
const res = await request(app)
.put('/api/stacks/mystack/auto-update')
.set('Cookie', adminCookie)
.send({ enabled: false });
expect(res.status).toBe(403);
} finally {
// Use mockReturnValue rather than mockRestore: restoring would bypass the
// beforeAll spy that sets the tier to 'paid' for the rest of the suite.
spy.mockReturnValue('paid');
}
});
it('rejects non-boolean enabled with 400', async () => {
const res = await request(app)
.put('/api/stacks/mystack/auto-update')
.set('Cookie', adminCookie)
.send({ enabled: 'yes' });
expect(res.status).toBe(400);
});
it('rejects invalid stack names with 400', async () => {
// Dots are rejected by isValidStackName; unlike path-traversal sequences
// they are not normalised away by Express routing.
const res = await request(app)
.put('/api/stacks/my.stack/auto-update')
.set('Cookie', adminCookie)
.send({ enabled: false });
expect(res.status).toBe(400);
});
it('accepts Skipper/Admiral admin and persists the setting', async () => {
const res = await request(app)
.put('/api/stacks/my-app/auto-update')
.set('Cookie', adminCookie)
.send({ enabled: false });
expect(res.status).toBe(200);
expect(res.body.enabled).toBe(false);
const db = DatabaseService.getInstance();
const node = db.getNodes().find(n => n.type === 'local');
expect(db.getStackAutoUpdateEnabled(node!.id, 'my-app')).toBe(false);
});
it('can re-enable a disabled stack', async () => {
await request(app)
.put('/api/stacks/my-app/auto-update')
.set('Cookie', adminCookie)
.send({ enabled: false });
const res = await request(app)
.put('/api/stacks/my-app/auto-update')
.set('Cookie', adminCookie)
.send({ enabled: true });
expect(res.status).toBe(200);
expect(res.body.enabled).toBe(true);
});
});
describe('POST /api/auto-update/execute - per-stack disable gate', () => {
it('skips stacks with auto-updates disabled', async () => {
const db = DatabaseService.getInstance();
const node = db.getNodes().find(n => n.type === 'local')!;
db.upsertStackAutoUpdateEnabled(node.id, 'disabled-stack', false);
// Mock FileSystemService so target='*' returns our test stack
const fsMod = await import('../services/FileSystemService');
const getStacksSpy = vi.spyOn(fsMod.FileSystemService.prototype, 'getStacks')
.mockResolvedValue(['disabled-stack']);
try {
const res = await request(app)
.post('/api/auto-update/execute')
.set('Cookie', adminCookie)
.send({ target: '*' });
expect(res.status).toBe(200);
expect(res.body.result).toContain('auto-updates disabled; skipped');
} finally {
getStacksSpy.mockRestore();
db.clearStackAutoUpdateSetting(node.id, 'disabled-stack');
}
});
it('skips a named disabled stack', async () => {
const db = DatabaseService.getInstance();
const node = db.getNodes().find(n => n.type === 'local')!;
db.upsertStackAutoUpdateEnabled(node.id, 'named-disabled', false);
try {
const res = await request(app)
.post('/api/auto-update/execute')
.set('Cookie', adminCookie)
.send({ target: 'named-disabled' });
expect(res.status).toBe(200);
expect(res.body.result).toContain('auto-updates disabled; skipped');
} finally {
db.clearStackAutoUpdateSetting(node.id, 'named-disabled');
}
});
it('allows enabled stacks to proceed (may fail at image check, but not at disable gate)', async () => {
const db = DatabaseService.getInstance();
const node = db.getNodes().find(n => n.type === 'local')!;
db.upsertStackAutoUpdateEnabled(node.id, 'enabled-stack', true);
try {
const res = await request(app)
.post('/api/auto-update/execute')
.set('Cookie', adminCookie)
.send({ target: 'enabled-stack' });
expect(res.status).toBe(200);
// Should NOT contain "auto-updates disabled" in result
expect(res.body.result).not.toContain('auto-updates disabled; skipped');
} finally {
db.clearStackAutoUpdateSetting(node.id, 'enabled-stack');
}
});
});