Repository navigation
feat(runtime): report background task process and endpoint health #5261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
01a0b3f
ac058fc
f77e95a
24235c1
2520b0c
bc97134
e92b62a
0e45fac
9d26eef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -137,6 +137,8 @@ export interface ShellRunRecord { | |
| cwd: string; | ||
| command: string; | ||
| status: ShellRunStatus; | ||
| /** Native root process id, when admitted by the process driver. */ | ||
| pid?: number; | ||
| exitCode?: number; | ||
| failureMessage?: string; | ||
| startedAt: number; | ||
|
|
@@ -159,7 +161,14 @@ export interface ShellRunRecord { | |
| export type ShellRunPatch = Partial< | ||
| Pick< | ||
| ShellRunRecord, | ||
| 'status' | 'exitCode' | 'failureMessage' | 'updatedAt' | 'completedAt' | 'observedAt' | 'output' | ||
| | 'status' | ||
| | 'pid' | ||
| | 'exitCode' | ||
| | 'failureMessage' | ||
| | 'updatedAt' | ||
| | 'completedAt' | ||
| | 'observedAt' | ||
| | 'output' | ||
| > | ||
| >; | ||
|
|
||
|
|
@@ -307,6 +316,7 @@ const SHELL_RUN_SESSION_ID_PATTERN = /^[A-Za-z0-9_-]{1,128}$/; | |
|
|
||
| const SHELL_RUN_PATCH_KEYS: ReadonlySet<string> = new Set([ | ||
| 'status', | ||
| 'pid', | ||
| 'exitCode', | ||
| 'failureMessage', | ||
| 'updatedAt', | ||
|
|
@@ -325,6 +335,7 @@ const SHELL_RUN_RECORD_KEYS: ReadonlySet<string> = new Set([ | |
| 'cwd', | ||
| 'command', | ||
| 'status', | ||
| 'pid', | ||
| 'startedAt', | ||
| 'updatedAt', | ||
| 'completedAt', | ||
|
|
@@ -380,6 +391,7 @@ export function normalizeShellRunRecord( | |
| record.sessionId === sessionId && | ||
| record.shellRunId === shellRunId && | ||
| isShellRunStatus(record.status) && | ||
| (record.pid === undefined || isPositiveInteger(record.pid)) && | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟢 Cross-version compat to declare.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Preserve the PID when canonicalizing the record. This validator accepts |
||
| isFiniteNumber(record.startedAt) && | ||
| isFiniteNumber(record.updatedAt) && | ||
| isPositiveInteger(record.revision) && | ||
|
|
@@ -515,6 +527,7 @@ function isShellRunSandboxEscalation(value: unknown, execution: unknown): boolea | |
|
|
||
| function canonicalShellRunRecord(record: ShellRunRecord): ShellRunRecord { | ||
| return { | ||
| ...(record.pid !== undefined ? { pid: record.pid } : {}), | ||
| shellRunId: record.shellRunId, | ||
| sessionId: record.sessionId, | ||
| ...(record.sourceRunId !== undefined ? { sourceRunId: record.sourceRunId } : {}), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ | |
| */ | ||
|
|
||
| import { buildWebFetchTool } from '@maka/runtime/web-fetch-tool'; | ||
| import { createLocalWebFetchExecutor } from '@maka/runtime/local-web-fetch'; | ||
| import { assertAllowedTarget, createLocalWebFetchExecutor } from '@maka/runtime/local-web-fetch'; | ||
| import { | ||
| createProxiedFetchTransport, | ||
| type ProxiedFetchProxy, | ||
|
|
@@ -29,6 +29,7 @@ import type { RuntimePolicyOperationCoordinator } from '@maka/storage/runtime-po | |
| import { toRuntimePolicyProxy } from './runtime-policy-proxy.js'; | ||
|
|
||
| interface HostWebFetchServiceInput { | ||
| readonly probeTimeoutMs?: number; | ||
| readonly policy: Pick<RuntimePolicyOperationCoordinator, 'resolveHostOutboundExecution'>; | ||
| readonly createFetchTransport?: (proxy: ProxiedFetchProxy | null) => ProxiedFetchTransport; | ||
| } | ||
|
|
@@ -39,6 +40,11 @@ export interface HostWebFetchService { | |
| readonly sessionId: string; | ||
| readonly abortSignal?: AbortSignal; | ||
| }): Promise<string>; | ||
| probe(input: { | ||
| url: string; | ||
| sessionId: string; | ||
| abortSignal: AbortSignal; | ||
| }): Promise<{ status: number; statusText?: string; elapsedMs: number }>; | ||
| } | ||
|
|
||
| export function createHostWebFetchService(input: HostWebFetchServiceInput): HostWebFetchService { | ||
|
|
@@ -65,6 +71,46 @@ export function createHostWebFetchService(input: HostWebFetchServiceInput): Host | |
| await transport.close(); | ||
| } | ||
| }, | ||
| probe: async ({ url, abortSignal }) => { | ||
| const parsed = new URL(url); | ||
| assertAllowedTarget(parsed); | ||
| abortSignal.throwIfAborted(); | ||
| const resolved = await input.policy.resolveHostOutboundExecution(); | ||
| if (resolved.kind === 'privacy_mode') | ||
| throw new Error('Endpoint health checks are disabled while privacy mode is active.'); | ||
| if (resolved.kind === 'credential_not_configured') | ||
| throw new Error('Configure the network proxy credential before checking an endpoint.'); | ||
| const transport = createFetchTransport( | ||
| toRuntimePolicyProxy(resolved.networkProxy, resolved.secretMaterial.networkProxy?.secret), | ||
| ); | ||
| const started = Date.now(); | ||
| const timeout = new AbortController(); | ||
| const timer = setTimeout( | ||
| () => timeout.abort(new Error('Endpoint health probe timed out.')), | ||
| input.probeTimeoutMs ?? 30_000, | ||
| ); | ||
| const signal = AbortSignal.any([abortSignal, timeout.signal]); | ||
| try { | ||
| let response = await transport.fetch(parsed, { | ||
| method: 'HEAD', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] A HEAD-only response is not sufficient to classify endpoint health. A local service whose registered route returned 200 to GET and 405 to HEAD was reported by the real |
||
| redirect: 'manual', | ||
| signal, | ||
| }); | ||
| await response.body?.cancel(); | ||
| if (response.status === 405 || response.status === 501) { | ||
| response = await transport.fetch(parsed, { method: 'GET', redirect: 'manual', signal }); | ||
| await response.body?.cancel(); | ||
| } | ||
| return { | ||
| status: response.status, | ||
| ...(response.statusText ? { statusText: response.statusText } : {}), | ||
| elapsedMs: Date.now() - started, | ||
| }; | ||
| } finally { | ||
| clearTimeout(timer); | ||
| await transport.close(); | ||
| } | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| import assert from 'node:assert/strict'; | ||
| import { test } from 'node:test'; | ||
| import { buildBackgroundTaskHealthTool } from '../background-task-health-tool.js'; | ||
|
|
||
| const context = { | ||
| sessionId: 'session-1', | ||
| turnId: 'turn-1', | ||
| toolCallId: 'tool-1', | ||
| cwd: '/tmp', | ||
| abortSignal: new AbortController().signal, | ||
| } as any; | ||
| const shell = (status: string, pid?: number) => ({ | ||
| kind: 'shell_run', | ||
| ref: 'maka://runtime/background-tasks/run-1', | ||
| status, | ||
| mode: 'pipes', | ||
| cwd: '/tmp', | ||
| cmd: 'python -m http.server', | ||
| startedAt: 1, | ||
| updatedAt: 2, | ||
| revision: 2, | ||
| ...(pid ? { pid } : {}), | ||
| }); | ||
|
|
||
| test('reports process tracking separately when endpoint is not checked', async () => { | ||
| const tool = buildBackgroundTaskHealthTool( | ||
| { readRuntimeResource: async () => shell('running', 1234) } as any, | ||
| { | ||
| probe: async () => { | ||
| throw new Error('must not probe'); | ||
| }, | ||
| }, | ||
| ); | ||
| assert.deepEqual( | ||
| JSON.parse(String(await tool.impl({ ref: 'maka://runtime/background-tasks/run-1' }, context))), | ||
| { | ||
| process: { status: 'running', tracked: true, startedAt: 1, updatedAt: 2, pid: 1234 }, | ||
| endpoint: { state: 'not_checked' }, | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| test('reports endpoint health only from the probe result', async () => { | ||
| let called = 0; | ||
| const tool = buildBackgroundTaskHealthTool( | ||
| { readRuntimeResource: async () => shell('running', 1234) } as any, | ||
| { | ||
| probe: async () => { | ||
| called += 1; | ||
| return { status: 204, statusText: 'No Content', elapsedMs: 4 }; | ||
| }, | ||
| }, | ||
| ); | ||
| assert.deepEqual( | ||
| JSON.parse( | ||
| String( | ||
| await tool.impl( | ||
| { ref: 'maka://runtime/background-tasks/run-1', url: 'http://127.0.0.1:8765/' }, | ||
| context, | ||
| ), | ||
| ), | ||
| ), | ||
| { | ||
| process: { status: 'running', tracked: true, startedAt: 1, updatedAt: 2, pid: 1234 }, | ||
| endpoint: { | ||
| state: 'checked', | ||
| httpStatus: 204, | ||
| elapsedMs: 4, | ||
| target: 'http://127.0.0.1:8765/', | ||
| health: 'healthy', | ||
| }, | ||
| }, | ||
| ); | ||
| assert.equal(called, 1); | ||
| }); | ||
|
|
||
| test('does not convert a failed probe into a ready claim', async () => { | ||
| const tool = buildBackgroundTaskHealthTool( | ||
| { readRuntimeResource: async () => shell('running', 1234) } as any, | ||
| { | ||
| probe: async () => { | ||
| throw new Error('connection refused'); | ||
| }, | ||
| }, | ||
| ); | ||
| assert.deepEqual( | ||
| JSON.parse( | ||
| String( | ||
| await tool.impl( | ||
| { ref: 'maka://runtime/background-tasks/run-1', url: 'http://127.0.0.1:8765/' }, | ||
| context, | ||
| ), | ||
| ), | ||
| ), | ||
| { | ||
| process: { status: 'running', tracked: true, startedAt: 1, updatedAt: 2, pid: 1234 }, | ||
| endpoint: { state: 'unknown', target: 'http://127.0.0.1:8765/', error: 'connection refused' }, | ||
| }, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Formatting churn: please revert. This region has no logic change — the original single-line
Pick<...>list was reformatted to one key per line. It passes biome (I ran the repo-pinned Biome 2.5.11format/lintover all 11 changed files: zero violations), but it is meaningless diff noise: it obscures the actual change, burdens review, and pollutes futuregit blame. Please restore the original layout and just insert'pid'into the existing list.