Public: anyone can read this; the crew can change it.
A machine shell that prints output is not idle, and its idle stop is a setting with its reason #10594
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/10543-a-machine-shell-that-prints-output-is-no → main 4 commits fork 3 threads · 3 unresolved
7 files
changed +98
−19
docs/features/machines.md +3
| @@ -87,6 +87,9 @@ repository's machines: | ||
| 87 | 87 | output. The session ends when the shell exits, after 30 minutes with no |
| 88 | 88 | keystroke, or — checked every 5 seconds and when its last connection |
| 89 | 89 | closes — once its owner is suspended or can no longer write here. |
| 90 | - **Idle.** A shell with no keystroke and no output for the admin limit | |
| 91 | `shell_idle_minutes` (default 30, 5 to 1440, set at `/admin/limits`, read at | |
| 92 | every check) ends as `idle`; one that prints, like a build, is never idle. | |
| 90 | 93 | - **Records.** Each session is a row in `machine_sessions` (machine, owner, |
| 91 | 94 | start, end and why: `exit`, `idle`, `stopped`, `revoked`, `failed`). No |
| 92 | 95 | shell outlives its forge, so a row a stop or crash left open is closed as |
src/actions/limits.ts +2 −6
| @@ -13,9 +13,7 @@ import { | ||
| 13 | 13 | LIMIT_KEYS, |
| 14 | 14 | isLimitKey, |
| 15 | 15 | limitChanges, |
| 16 | limitDefault, | |
| 17 | limitIsSet, | |
| 18 | limitValue, | |
| 16 | limitState, | |
| 19 | 17 | setLimit, |
| 20 | 18 | type LimitChange, |
| 21 | 19 | type LimitDef, |
| @@ -45,9 +43,7 @@ export function listLimits( | ||
| 45 | 43 | limits: LIMIT_KEYS.map((key) => ({ |
| 46 | 44 | ...LIMITS[key], |
| 47 | 45 | key, |
| 48 | value: limitValue(app.db, key), | |
| 49 | isSet: limitIsSet(app.db, key), | |
| 50 | defaultValue: limitDefault(app.db, key), | |
| 46 | ...limitState(app.db, key), | |
| 51 | 47 | })), |
| 52 | 48 | changes: limitChanges(app.db), |
| 53 | 49 | }); |
src/http/server.ts +2 −4
| @@ -586,13 +586,11 @@ import { | ||
| 586 | 586 | withShells, |
| 587 | 587 | type ForgeSocket, |
| 588 | 588 | type ShellSocket, |
| 589 | shellIdleMs, | |
| 589 | 590 | } from "../machines/shell.ts"; |
| 590 | 591 | |
| 591 | 592 | // ---- auth ---- |
| 592 | 593 | |
| 593 | /** A machine's shell with no keystroke for this long ends (`machines/shell.ts`). */ | |
| 594 | const SHELL_IDLE_MS = 30 * 60_000; | |
| 595 | ||
| 596 | 594 | const NO_AUTH: Auth = { user: undefined, scope: undefined, via: "none" }; |
| 597 | 595 | |
| 598 | 596 | async function resolveAuth(app: App, request: Request): Promise<Auth> { |
| @@ -2610,7 +2608,7 @@ export async function serve(app: App): Promise<Servers> { | ||
| 2610 | 2608 | const cells = await cellsAvailable(); |
| 2611 | 2609 | if (shellOptions(app) === undefined) |
| 2612 | 2610 | setShellOptions(app, { |
| 2613 | idleMs: SHELL_IDLE_MS, | |
| 2611 | idleMs: () => shellIdleMs(app.db), | |
outdated
Untested wiring: add a test that fails if this reads the wrong key or loses the * 60_000, using setLimit. |
||
| 2614 | 2612 | contain: cells.ok ? "cell" : "refuse", |
| 2615 | 2613 | command: defaultCommand(), |
| 2616 | 2614 | }); |
src/machines/shell.ts +26 −6
| @@ -13,8 +13,8 @@ | ||
| 13 | 13 | * second tab, a reload) joins the session that is running and sees the same |
| 14 | 14 | * screen. Binary frames are keystrokes in and screen bytes out; a text frame |
| 15 | 15 | * is JSON and says only `{"type":"resize","cols":n,"rows":n}`. The session |
| 16 | * ends when its program exits or after `idleMs` with no keystroke, and each | |
| 17 | * one is a row in `machine_sessions`: machine, owner, start, end and why. | |
| 16 | * ends when its program exits or after the idle limit with no keystroke and | |
| 17 | * no output, and each one is a row in `machine_sessions`: machine, owner, start, end and why. | |
| 18 | 18 | * |
| 19 | 19 | * No text a person sends ever reaches a command line: the command is a |
| 20 | 20 | * constant, and the names in the path only pick a row and a directory after |
| @@ -36,6 +36,7 @@ import type { User } from "../core/users.ts"; | ||
| 36 | 36 | import { repoMachines } from "../crew/machines_read.ts"; |
| 37 | 37 | import { reachableApps, type Machine } from "../crew/machines.ts"; |
| 38 | 38 | import { ownApp, ownAppUrls } from "../crew/app_hosts.ts"; |
| 39 | import { limitValue } from "../settings/limits.ts"; | |
| 39 | 40 | import { id as newId } from "../kernel/id.ts"; |
| 40 | 41 | |
| 41 | 42 | // ---- the route ---- |
| @@ -246,13 +247,18 @@ interface Session { | ||
| 246 | 247 | /** The pty's size, as last set. */ |
| 247 | 248 | size: PtySize; |
| 248 | 249 | lastInput: number; |
| 250 | /** When the program last printed: a running build is not idle. */ | |
| 251 | lastOutput: number; | |
| 249 | 252 | ended: boolean; |
| 250 | 253 | readonly timer: ReturnType<typeof setInterval>; |
| 251 | 254 | } |
| 252 | 255 | |
| 253 | 256 | export interface ShellOptions { |
| 254 | /** No keystroke for this long ends the session. */ | |
| 255 | readonly idleMs: number; | |
| 257 | /** | |
| 258 | * No keystroke and no output for this long ends the session. A function is | |
| 259 | * asked at every check, so a changed setting governs the next one. | |
| 260 | */ | |
| 261 | readonly idleMs: number | (() => number); | |
| 256 | 262 | /** |
| 257 | 263 | * Whether this forge can hold a cell. Without one the shell is refused — |
| 258 | 264 | * except where a test says `uncontained`, on purpose, to run the bridge. |
| @@ -377,6 +383,16 @@ export function shellCell( | ||
| 377 | 383 | }; |
| 378 | 384 | } |
| 379 | 385 | |
| 386 | /** The idle limit an admin set at `/admin/limits`, in ms; read at every check. */ | |
| 387 | export const shellIdleMs = (db: Database): number => limitValue(db, "shell_idle_minutes") * 60_000; | |
| 388 | ||
| 389 | const idleLimit = (opts: ShellOptions): number => (typeof opts.idleMs === "function" ? opts.idleMs() : opts.idleMs); | |
| 390 | ||
| 391 | /** Idle only when both the last keystroke and the last output are older than the limit. */ | |
| 392 | export function idle(session: { lastInput: number; lastOutput: number }, limitMs: number, now = Date.now()): boolean { | |
| 393 | return now - session.lastInput >= limitMs && now - session.lastOutput >= limitMs; | |
| 394 | } | |
| 395 | ||
| 380 | 396 | function end(app: App, session: Session, reason: EndReason): void { |
| 381 | 397 | if (session.ended) return; |
| 382 | 398 | session.ended = true; |
| @@ -401,6 +417,7 @@ function start(app: App, grant: ShellGrant, opts: ShellOptions, cols: number, ro | ||
| 401 | 417 | let terminal: Bun.Terminal | undefined; |
| 402 | 418 | let cell: Cell | undefined; |
| 403 | 419 | let row: string | undefined; |
| 420 | let live: Session | undefined; | |
| 404 | 421 | try { |
| 405 | 422 | mkdirSync(join(dir, "home"), { recursive: true }); |
| 406 | 423 | row = recordStart(app.db, grant.repo.id, grant.machine.name, grant.user.id); |
| @@ -408,6 +425,7 @@ function start(app: App, grant: ShellGrant, opts: ShellOptions, cols: number, ro | ||
| 408 | 425 | cols, |
| 409 | 426 | rows, |
| 410 | 427 | data: (_t, chunk) => { |
| 428 | if (live !== undefined) live.lastOutput = Date.now(); | |
| 411 | 429 | for (const peer of peers) { |
| 412 | 430 | if (peer.readyState !== WebSocket.OPEN) continue; |
| 413 | 431 | // A peer that is not reading is closed, never buffered without end. |
| @@ -447,19 +465,21 @@ function start(app: App, grant: ShellGrant, opts: ShellOptions, cols: number, ro | ||
| 447 | 465 | peers, |
| 448 | 466 | size: { cols, rows }, |
| 449 | 467 | lastInput: Date.now(), |
| 468 | lastOutput: Date.now(), | |
| 450 | 469 | ended: false, |
| 451 | 470 | timer: setInterval( |
| 452 | 471 | () => { |
| 453 | if (Date.now() - session.lastInput >= opts.idleMs) end(app, session, "idle"); | |
| 472 | if (idle(session, idleLimit(opts))) end(app, session, "idle"); | |
| 454 | 473 | else if (!stillMay(app, session)) end(app, session, "revoked"); |
| 455 | 474 | }, |
| 456 | Math.max(10, Math.min(opts.idleMs, opts.checkMs ?? CHECK_MS)), | |
| 475 | Math.max(10, Math.min(idleLimit(opts), opts.checkMs ?? CHECK_MS)), | |
| 457 | 476 | ), |
| 458 | 477 | }; |
| 459 | 478 | void program.exited.then(() => { |
| 460 | 479 | term.close(); |
| 461 | 480 | end(app, session, "exit"); |
| 462 | 481 | }); |
| 482 | live = session; | |
| 463 | 483 | sessionsOf(app).set(session.key, session); |
| 464 | 484 | app.log.info("machine shell started", { |
| 465 | 485 | repo: `${grant.repo.ownerName}/${grant.repo.name}`, |
src/settings/limits.ts +24 −1
| @@ -50,7 +50,8 @@ export type LimitKey = | ||
| 50 | 50 | | "api.anonymous_per_source" |
| 51 | 51 | | "model_review.auto" |
| 52 | 52 | | "model_review.min_landed" |
| 53 | | "model_review.slack_pct"; | |
| 53 | | "model_review.slack_pct" | |
| 54 | | "shell_idle_minutes"; | |
| 54 | 55 | |
| 55 | 56 | export interface LimitDef { |
| 56 | 57 | readonly label: string; |
| @@ -294,6 +295,14 @@ export const LIMITS: Record<LimitKey, LimitDef> = { | ||
| 294 | 295 | max: 100, |
| 295 | 296 | reason: "Past this the weekly review undoes its own change, so quality is the bar the saving is held to.", |
| 296 | 297 | }, |
| 298 | shell_idle_minutes: { | |
| 299 | label: "Minutes a machine shell may be silent before it stops", | |
| 300 | unit: "minutes", | |
| 301 | default: 30, | |
| 302 | min: 5, | |
| 303 | max: 1440, | |
| 304 | reason: "A shell nobody types in and that prints nothing holds a process and memory for no reason; one that prints, like a build, is never stopped.", | |
| 305 | }, | |
| 297 | 306 | }; |
| 298 | 307 | |
| 299 | 308 | export const LIMIT_KEYS = Object.keys(LIMITS) as LimitKey[]; |
| @@ -368,6 +377,20 @@ export function limitIsSet(db: Database, key: LimitKey): boolean { | ||
| 368 | 377 | return stored(db, key) !== undefined; |
| 369 | 378 | } |
| 370 | 379 | |
| 380 | /** | |
| 381 | * Value, whether it is set and the default in one go: a stored limit is read | |
| 382 | * once, not once for the value and again for "is it set". | |
| 383 | */ | |
| 384 | export function limitState( | |
| 385 | db: Database, | |
| 386 | key: LimitKey, | |
| 387 | ): { value: number; isSet: boolean; defaultValue: number } { | |
| 388 | if (capacityKey(key) !== undefined) | |
| 389 | return { value: limitValue(db, key), isSet: limitIsSet(db, key), defaultValue: limitDefault(db, key) }; | |
| 390 | const set = stored(db, key); | |
| 391 | return { value: set ?? LIMITS[key].default, isSet: set !== undefined, defaultValue: LIMITS[key].default }; | |
| 392 | } | |
| 393 | ||
| 371 | 394 | export type LimitError = { |
| 372 | 395 | readonly kind: "out_of_bounds"; |
| 373 | 396 | readonly message: string; |
test/machine-shell.test.ts +41 −1
| @@ -18,7 +18,8 @@ import { createUser, type User } from "../src/core/users.ts"; | ||
| 18 | 18 | import { serve, type Servers } from "../src/http/server.ts"; |
| 19 | 19 | import { tokenFor } from "./helpers/tokens.ts"; |
| 20 | 20 | import { existsSync } from "node:fs"; |
| 21 | import { closeStaleSessions, recordStart, shellCell, workspace, MAX_SESSIONS } from "../src/machines/shell.ts"; | |
| 21 | import { setLimit } from "../src/settings/limits.ts"; | |
| 22 | import { idle, shellIdleMs, closeStaleSessions, recordStart, shellCell, workspace, MAX_SESSIONS } from "../src/machines/shell.ts"; | |
| 22 | 23 | import { BUILT_IN } from "../src/crew/machines.ts"; |
| 23 | 24 | import { |
| 24 | 25 | lastSessions, |
| @@ -215,6 +216,45 @@ test("a session with no keystroke past the idle limit ends, and says so", async | ||
| 215 | 216 | setShellOptions(app, { idleMs: 60_000, contain: "uncontained", command: "exec /bin/sh", handshakesPerMinute: 1000 }); |
| 216 | 217 | }); |
| 217 | 218 | |
| 219 | test("output with no input keeps a session alive past the idle limit", async () => { | |
| 220 | stopShells(app); | |
| 221 | setShellOptions(app, { idleMs: 300, contain: "uncontained", command: "while :; do echo tick; sleep 0.05; done", checkMs: 30, handshakesPerMinute: 1000 }); | |
| 222 | const { ws } = await open("alice"); | |
| 223 | await Bun.sleep(1_000); | |
| 224 | expect(liveSessions(app)).toBe(1); | |
| 225 | expect(ws.readyState).toBe(WebSocket.OPEN); | |
| 226 | stopShells(app); | |
| 227 | setShellOptions(app, { idleMs: 60_000, contain: "uncontained", command: "exec /bin/sh", handshakesPerMinute: 1000 }); | |
| 228 | }, 10_000); | |
| 229 | ||
| 230 | test("idle needs both the last input and the last output older than the limit", () => { | |
| 231 | expect(idle({ lastInput: 0, lastOutput: 0 }, 100, 100)).toBe(true); | |
| 232 | expect(idle({ lastInput: 0, lastOutput: 50 }, 100, 100)).toBe(false); | |
| 233 | expect(idle({ lastInput: 50, lastOutput: 0 }, 100, 100)).toBe(false); | |
| 234 | }); | |
| 235 | ||
| 236 | test("the idle limit the server installs is the shell_idle_minutes setting, in minutes", () => { | |
| 237 | expect(shellIdleMs(app.db)).toBe(30 * 60_000); | |
| 238 | expect(setLimit(app.db, "shell_idle_minutes", 7, "test").ok).toBe(true); | |
| 239 | expect(shellIdleMs(app.db)).toBe(7 * 60_000); | |
| 240 | expect(setLimit(app.db, "shell_idle_minutes", null, "test").ok).toBe(true); | |
| 241 | expect(shellIdleMs(app.db)).toBe(30 * 60_000); | |
| 242 | }); | |
| 243 | ||
| 244 | test("changing the setting changes the next check", async () => { | |
| 245 | stopShells(app); | |
| 246 | let ms = 60_000; | |
| 247 | setShellOptions(app, { idleMs: () => ms, contain: "uncontained", command: "exec /bin/sh", checkMs: 30, handshakesPerMinute: 1000 }); | |
| 248 | const { ws } = await open("alice"); | |
| 249 | await Bun.sleep(300); | |
| 250 | expect(liveSessions(app)).toBe(1); | |
| 251 | ms = 100; | |
| 252 | await new Promise((r) => ws.addEventListener("close", r)); | |
| 253 | await until(() => lastSessions(app.db, repo.id)[0]?.endReason !== undefined); | |
| 254 | expect(lastSessions(app.db, repo.id)[0]?.endReason).toBe("idle"); | |
| 255 | setShellOptions(app, { idleMs: 60_000, contain: "uncontained", command: "exec /bin/sh", handshakesPerMinute: 1000 }); | |
| 256 | }, 10_000); | |
| 257 | ||
| 218 | 258 | test("a forge that cannot hold a cell refuses the shell", async () => { |
| 219 | 259 | setShellOptions(app, { idleMs: 60_000, contain: "refuse", command: "exec /bin/sh", handshakesPerMinute: 1000 }); |
| 220 | 260 | const ws = new WebSocket(`${base}/alice/lab/machines/standard/shell`, { |
test/pages-budget.test.ts −1
| @@ -35,7 +35,6 @@ const HEAVIER: Record<string, { statements?: number; git?: number }> = { | ||
| 35 | 35 | "/:owner/:name/issues/:number": { statements: 70 }, |
| 36 | 36 | "/:owner/:name/next": { statements: 64 }, |
| 37 | 37 | "/:owner/:name/pulls/:number": { statements: 106 }, |
| 38 | "/admin/limits": { statements: 80 }, | |
| 39 | 38 | "/board": { statements: 64 }, |
outdated
Budgets are lowered, never raised. Reduce the limits page reads rather than raising the allowance. |
||
| 40 | 39 | "/settings/capacity": { statements: 65 }, |
| 41 | 40 | }; |
Non-blocking: this line still says a fixed 30 minutes with no keystroke. Point it at the Idle entry below.