Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 22 additions & 15 deletions docs/control-plane.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,37 @@ The control server runs inside the daemon process alongside channels, the
inbound/outbound workers, and the scheduler registry. No separate process, no
TCP port.

## Socket activation
## Socket lifecycle

In production (systemd template pair):
Systemd manages the socket directory via `RuntimeDirectory=porter-%i` in the
service unit. The daemon binds the socket itself -- systemd socket activation
(`Bun.serve({ fd })`) is not used because the feature never shipped in Bun
(see [oven-sh/bun#2852](https://github.com/oven-sh/bun/pull/2852)).
Comment thread
dougEfresh marked this conversation as resolved.

```ini
# porter@.socket
ListenStream=%t/porter/porter-%i.sock
SocketMode=0600
# porter@.service
Environment=PORTER_SOCKET=%t/porter-%i/porter.sock
RuntimeDirectory=porter-%i
RuntimeDirectoryMode=0700
```

Systemd creates the socket before the service starts and passes it as fd 3.
The daemon detects `LISTEN_FDS=1` and calls `Bun.serve({ fd: 3 })`.
`RemoveOnStop=yes` handles cleanup.
Each template instance gets its own directory:

In development (no socket unit, `bun run`):
```
porter@projects-me-pi-porter → /run/user/1000/porter-projects-me-pi-porter/porter.sock
porter-dev → /run/user/1000/porter-dev/porter.sock
```

In development (`bun run`, no systemd):

```bash
# Daemon binds directly:
bun run runtime/src/index.ts --serve
# Socket created at $XDG_RUNTIME_DIR/porter/porter.sock (fallback: ~/.local/state/porter/porter.sock)
# Falls back to $XDG_RUNTIME_DIR/porter/porter.sock
# or ~/.local/state/porter/porter.sock
```

The CLI locates the socket via `PORTER_SOCKET` env var with a fallback to
`$XDG_RUNTIME_DIR/porter/porter.sock`.
The daemon prefers `PORTER_SOCKET` from the environment; the CLI uses the
same variable with the same fallback.

## API Reference

Expand Down Expand Up @@ -196,8 +203,8 @@ Worker states: `booting` (initializing Pi session), `ready` (idle), `busy`
## CLI

```bash
# Set socket path for template instances:
export PORTER_SOCKET=$XDG_RUNTIME_DIR/porter/porter-projects-me-pi-porter.sock
# Socket path is set by the systemd service unit; export manually for dev:
export PORTER_SOCKET=$XDG_RUNTIME_DIR/porter-projects-me-pi-porter/porter.sock

porter task list
porter task get morning-brief
Expand Down
3 changes: 3 additions & 0 deletions resources/systemd/porter-dev.service
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ EnvironmentFile=%h/workspace/.env
# Dev unit: run the locally built binary. PATH is for agent subprocesses (bun, tools), not ExecStart.
ExecStart=%h/.local/bin/porter --serve
Environment=PATH=%h/.local/bin:/usr/local/bin:/usr/bin:/bin
Environment=PORTER_SOCKET=%t/porter-dev/porter.sock
RuntimeDirectory=porter-dev
RuntimeDirectoryMode=0700
Restart=on-failure
RestartSec=10s
RestartSteps=4
Expand Down
3 changes: 2 additions & 1 deletion resources/systemd/porter@.service
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ EnvironmentFile=%h/%i/.env
# Prod unit: packaged install at a fixed path. PATH only affects subprocesses, not ExecStart.
ExecStart=/usr/bin/porter --serve
Environment=PATH=%h/.local/bin:/usr/local/bin:/usr/bin:/bin
RuntimeDirectory=porter
Environment=PORTER_SOCKET=%t/porter-%i/porter.sock
RuntimeDirectory=porter-%i
RuntimeDirectoryMode=0700
Restart=on-failure
RestartSec=5
Expand Down
145 changes: 60 additions & 85 deletions runtime/src/control-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ import { parseSessionKey } from './routing/session-key.js';
import type { SchedulerRegistry } from './scheduler/registry.js';
import type { NewScheduledTask } from './scheduler/types.js';

/** Bun extends Request with route params. Use `any` because TS Record access is `string | undefined`. */
// eslint-disable-next-line @typescript-eslint/no-explicit-any
type RoutedRequest = Request & { params: any };

// ---- Types ----

export type ControlServerOptions = {
Expand Down Expand Up @@ -55,19 +51,6 @@ function errorJson(message: string, status: number): Response {
return json({ error: message }, status);
}

/**
* Detect systemd socket activation.
*
* systemd sets LISTEN_FDS=1 and LISTEN_PID=<our pid>. The first
* (and typically only) socket fd is 3 (SD_LISTEN_FDS_START).
*/
function systemdFd(): number | undefined {
if (process.env.LISTEN_PID !== String(process.pid)) return undefined;
const count = Number.parseInt(process.env.LISTEN_FDS ?? '0', 10);
if (count < 1) return undefined;
return 3;
}

// ---- ControlServer ----

export class ControlServer {
Expand All @@ -86,66 +69,67 @@ export class ControlServer {
}

async start(): Promise<void> {
const fd = systemdFd();

const routes = {
'/api/health': {
GET: () => this.handleHealth(),
},
'/api/scheduled-tasks': {
GET: () => this.handleListTasks(),
POST: (req: Request) => this.handleCreateTask(req),
},
'/api/scheduled-tasks/:id': {
GET: (req: Request) => this.handleGetTask(req),
DELETE: (req: Request) => this.handleDeleteTask(req),
},
'/api/scheduled-tasks/:id/pause': {
POST: (req: Request) => this.handlePauseTask(req),
},
'/api/scheduled-tasks/:id/resume': {
POST: (req: Request) => this.handleResumeTask(req),
},
'/api/scheduled-tasks/:id/fire': {
POST: (req: Request) => this.handleFireTask(req),
},
'/api/scheduled-tasks/:id/runs': {
GET: (req: Request) => this.handleGetTaskRuns(req),
},
'/api/workers': {
GET: () => this.handleWorkers(),
},
} as const;
const fetchHandler: (req: Request) => Response | Promise<Response> = (req) => {
const url = new URL(req.url);
const id = url.pathname.match(/^\/api\/scheduled-tasks\/([^/]+)/)?.[1];

// Health
if (url.pathname === '/api/health' && req.method === 'GET') return this.handleHealth();

// Scheduled tasks — collection
if (url.pathname === '/api/scheduled-tasks') {
if (req.method === 'GET') return this.handleListTasks();
if (req.method === 'POST') return this.handleCreateTask(req);
}

// Scheduled tasks — single
if (id && url.pathname === `/api/scheduled-tasks/${id}`) {
if (req.method === 'GET') return this.handleGetTask(id);
if (req.method === 'DELETE') return this.handleDeleteTask(id);
}
if (id && url.pathname === `/api/scheduled-tasks/${id}/pause` && req.method === 'POST')
return this.handlePauseTask(id);
if (id && url.pathname === `/api/scheduled-tasks/${id}/resume` && req.method === 'POST')
return this.handleResumeTask(id);
if (id && url.pathname === `/api/scheduled-tasks/${id}/fire` && req.method === 'POST')
return this.handleFireTask(id);
if (id && url.pathname === `/api/scheduled-tasks/${id}/runs` && req.method === 'GET') {
const limit = Number.parseInt(url.searchParams.get('limit') ?? '50', 10);
return this.handleGetTaskRuns(id, limit);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reject negative limit values instead of normalizing them to the default.

The PR contract says negatives should be rejected, but limit=-1 currently falls through to handleGetTaskRuns() and comes back as 50. That makes bad input look valid and hides caller bugs.

Possible localized fix
       if (id && url.pathname === `/api/scheduled-tasks/${id}/runs` && req.method === 'GET') {
         const limit = Number.parseInt(url.searchParams.get('limit') ?? '50', 10);
+        if (Number.isFinite(limit) && limit < 0) {
+          return errorJson('limit must be non-negative', 400);
+        }
         return this.handleGetTaskRuns(id, limit);
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (id && url.pathname === `/api/scheduled-tasks/${id}/runs` && req.method === 'GET') {
const limit = Number.parseInt(url.searchParams.get('limit') ?? '50', 10);
return this.handleGetTaskRuns(id, limit);
if (id && url.pathname === `/api/scheduled-tasks/${id}/runs` && req.method === 'GET') {
const limit = Number.parseInt(url.searchParams.get('limit') ?? '50', 10);
if (Number.isFinite(limit) && limit < 0) {
return errorJson('limit must be non-negative', 400);
}
return this.handleGetTaskRuns(id, limit);

}

// Workers
if (url.pathname === '/api/workers' && req.method === 'GET') return this.handleWorkers();

return errorJson('not found', 404);
};

const onError = (err: Error) => {
console.error('[control-server] unhandled error', { error: err });
return errorJson('internal server error', 500);
};

if (fd !== undefined) {
console.log('[control-server] binding on systemd socket activation fd', { fd });
// Bun types for `fd` + `routes` together are incomplete; cast through any.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
this.server = Bun.serve({ fd, routes, error: onError } as any);
} else {
const runtimeDir = process.env.XDG_RUNTIME_DIR;
const stateDir = process.env.PORTER_STATE_DIR || `${process.env.HOME}/.local/state/porter`;
const base = runtimeDir ? `${runtimeDir}/porter` : stateDir;
const socketPath = `${base}/porter.sock`;

// Clean up stale socket from a previous run.
try {
await Bun.file(socketPath).delete();
} catch (err) {
// ENOENT is expected; log anything unexpected.
console.warn('[control-server] stale socket cleanup failed', { path: socketPath, err });
}
const socketPath =
process.env.PORTER_SOCKET ||
(() => {
const runtimeDir = process.env.XDG_RUNTIME_DIR;
const stateDir = process.env.PORTER_STATE_DIR || `${process.env.HOME}/.local/state/porter`;
const base = runtimeDir ? `${runtimeDir}/porter` : stateDir;
return `${base}/porter.sock`;
})();
Comment on lines +68 to +75

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve the explicit unix override.

start() now ignores ControlServerOptions.unix and always recomputes the socket path from env/fallbacks. Any caller that passes a custom socket path will silently bind somewhere else.


this.socketPath = socketPath;
console.log('[control-server] binding on unix socket', { path: socketPath });
this.server = Bun.serve({ unix: socketPath, routes, error: onError });
// Clean up stale socket from a previous run.
try {
await Bun.file(socketPath).delete();
} catch (err) {
// ENOENT is expected; log anything unexpected.
console.warn('[control-server] stale socket cleanup failed', { path: socketPath, err });
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

this.socketPath = socketPath;
console.log('[control-server] binding on unix socket', { path: socketPath });
this.server = Bun.serve({ unix: socketPath, fetch: fetchHandler, error: onError });

console.log('[control-server] started');
}

Expand Down Expand Up @@ -185,8 +169,7 @@ export class ControlServer {
return json(tasks);
}

private async handleGetTask(req: Request): Promise<Response> {
const id = (req as RoutedRequest).params.id as string;
private async handleGetTask(id: string): Promise<Response> {
const task = await this.taskStore.getById(id);
if (!task) return errorJson('task not found', 404);
return json(task);
Expand Down Expand Up @@ -242,9 +225,7 @@ export class ControlServer {
}
}

private async handleDeleteTask(req: Request): Promise<Response> {
const id = (req as RoutedRequest).params.id as string;

private async handleDeleteTask(id: string): Promise<Response> {
const task = await this.taskStore.getById(id);
if (!task) return errorJson('task not found', 404);

Expand All @@ -258,8 +239,7 @@ export class ControlServer {
return new Response(null, { status: 204 });
}

private async handlePauseTask(req: Request): Promise<Response> {
const id = (req as RoutedRequest).params.id as string;
private async handlePauseTask(id: string): Promise<Response> {
const task = await this.taskStore.getById(id);
if (!task) return errorJson('task not found', 404);
if (task.status !== 'active') {
Expand All @@ -273,8 +253,7 @@ export class ControlServer {
return json({ id, status: 'paused' });
}

private async handleResumeTask(req: Request): Promise<Response> {
const id = (req as RoutedRequest).params.id as string;
private async handleResumeTask(id: string): Promise<Response> {
const task = await this.taskStore.getById(id);
if (!task) return errorJson('task not found', 404);
if (task.status !== 'paused') {
Expand All @@ -287,21 +266,17 @@ export class ControlServer {
return json({ id, status: 'active' });
}

private async handleFireTask(req: Request): Promise<Response> {
const id = (req as RoutedRequest).params.id as string;
private async handleFireTask(id: string): Promise<Response> {
const result = await this.scheduler.fireNow(id);
if (!result.ok) {
return errorJson(result.error ?? 'failed to fire task', result.error === 'task not found' ? 404 : 409);
}
return json({ id, fired: true });
}

private async handleGetTaskRuns(req: Request): Promise<Response> {
const id = (req as RoutedRequest).params.id as string;
const url = new URL(req.url);
const raw = Number.parseInt(url.searchParams.get('limit') ?? '50', 10);
const limit = Number.isFinite(raw) && raw > 0 ? Math.min(raw, 500) : 50;
const runs = await this.taskStore.getRuns(id, limit);
private async handleGetTaskRuns(id: string, limit: number): Promise<Response> {
const clamped = Number.isFinite(limit) && limit > 0 ? Math.min(limit, 500) : 50;
const runs = await this.taskStore.getRuns(id, clamped);
return json(runs);
}

Expand Down