mirror of
https://github.com/morluto/rea
synced 2026-10-26 06:16:17 -04:00
feat(sqlite): make worker deadlines adjustable with recovery guidance (#5818)
Expose a per-call SQLite worker deadline with exact CLI/MCP recovery controls, parent-owned execution metadata and historical unknown preservation. Keep fixed resource limits and cleanup admission intact. Share typed cleanup composition with one captured-output owner and preserve the timeout after cleanup resolves. Fixes #5812. Follow-up tracking: #5808. Existing CLI test-oracle failure: #5845. Verified 155 SQLite tests (3 skipped), static/docs/discovery checks, six real CLI/MCP pairs and 13 actual Codex sessions including four corrected deadline recoveries. Three pre-dispatch discovery failures remain explicit, not passes. Review findings include real retained-cleanup and fixture ownership regressions. Assisted-by: Kogasa (gensokyo-skills:kogasa-surprise-testing)
This commit is contained in:
@@ -8,7 +8,9 @@ import { projectAnalysisError } from "../domain/analysisErrorProjection.js";
|
||||
import {
|
||||
AnalysisError,
|
||||
type AnalysisCapturedOutput,
|
||||
type AnalysisErrorOptions,
|
||||
} from "../domain/analysisErrorBase.js";
|
||||
import type { JsonValue } from "../domain/jsonValue.js";
|
||||
import {
|
||||
AnalysisAccessDeniedError,
|
||||
AnalysisArtifactChangedError,
|
||||
@@ -23,8 +25,11 @@ import { ProviderAdapterError } from "../domain/providerAdapterError.js";
|
||||
import { ProviderCleanupError } from "../domain/providerCleanupError.js";
|
||||
import { OwnedCommandFailure } from "../process/OwnedCommand.js";
|
||||
import type { ProviderProcessSnapshot } from "../process/ProviderProcess.js";
|
||||
import { SQLITE_PROVIDER_IDENTITY } from "../domain/sqlite/sqliteDatabase.js";
|
||||
import { SQLITE_DATABASE_LIMITS } from "./SqliteDatabaseLimits.js";
|
||||
import {
|
||||
SQLITE_PROVIDER_IDENTITY,
|
||||
DEFAULT_SQLITE_WORKER_TIMEOUT_MS,
|
||||
MAX_SQLITE_WORKER_TIMEOUT_MS,
|
||||
} from "../domain/sqlite/sqliteDatabase.js";
|
||||
|
||||
/** Preserve diagnostics independently of worker exit and private-root cleanup. */
|
||||
export const capturedSqliteOutput = (
|
||||
@@ -35,6 +40,38 @@ export const capturedSqliteOutput = (
|
||||
truncated: snapshot.diagnosticTruncated === true,
|
||||
});
|
||||
|
||||
/** Keep typed cleanup history and one canonical output owner, including after resolution. */
|
||||
export const createSqliteCleanupError = (
|
||||
resources: readonly string[],
|
||||
diagnostics: Readonly<Record<string, JsonValue>>,
|
||||
options: Omit<AnalysisErrorOptions, "diagnostics">,
|
||||
): ProviderCleanupError => {
|
||||
const capturedOutput =
|
||||
options.capturedOutput ?? options.cleanupPrimary?.capturedOutput;
|
||||
let previous =
|
||||
options.cleanupPrimary === undefined
|
||||
? null
|
||||
: projectAnalysisError(options.cleanupPrimary);
|
||||
// History carries facts/recovery; the wrapper delivers output only once.
|
||||
if (capturedOutput !== undefined && previous?.details !== undefined) {
|
||||
const { captured_output: _capturedOutput, ...metadata } = previous.details;
|
||||
previous = { ...previous, details: metadata };
|
||||
}
|
||||
return new ProviderCleanupError(
|
||||
SQLITE_PROVIDER_IDENTITY.id,
|
||||
resources,
|
||||
{
|
||||
...diagnostics,
|
||||
previous_error: previous ?? diagnostics.previous_error ?? null,
|
||||
},
|
||||
{
|
||||
...options,
|
||||
operation: "inspect_sqlite_database",
|
||||
...(capturedOutput === undefined ? {} : { capturedOutput }),
|
||||
},
|
||||
);
|
||||
};
|
||||
|
||||
/** The file-set member a filesystem error names, if it is the database, its WAL or its journal. */
|
||||
const busyMember = (cause: Error, path: string): string => {
|
||||
const failed = "path" in cause ? cause.path : undefined;
|
||||
@@ -50,6 +87,7 @@ export const sqliteDatabaseFailure = (
|
||||
path: string,
|
||||
phase: string,
|
||||
signal?: AbortSignal,
|
||||
workerTimeoutMs = DEFAULT_SQLITE_WORKER_TIMEOUT_MS,
|
||||
): AnalysisError => {
|
||||
const operation = "inspect_sqlite_database";
|
||||
if (cause instanceof AnalysisError) return cause;
|
||||
@@ -61,28 +99,36 @@ export const sqliteDatabaseFailure = (
|
||||
: undefined;
|
||||
let previous: AnalysisError | undefined;
|
||||
if (physicalRead?.kind === "failed")
|
||||
previous = sqliteDatabaseFailure(physicalRead.cause, path, phase);
|
||||
previous = sqliteDatabaseFailure(
|
||||
physicalRead.cause,
|
||||
path,
|
||||
phase,
|
||||
undefined,
|
||||
workerTimeoutMs,
|
||||
);
|
||||
else if (cause.reason === "cancelled")
|
||||
previous = new AnalysisCancelledError(operation, {
|
||||
cause: artifactFileCancellationCause(cause),
|
||||
});
|
||||
else if (physicalRead?.kind === "completed") previous = undefined;
|
||||
else if (cause.cause !== undefined)
|
||||
previous = sqliteDatabaseFailure(cause.cause, path, phase);
|
||||
previous = sqliteDatabaseFailure(
|
||||
cause.cause,
|
||||
path,
|
||||
phase,
|
||||
undefined,
|
||||
workerTimeoutMs,
|
||||
);
|
||||
else if (signal?.aborted)
|
||||
previous = new AnalysisCancelledError(operation, {
|
||||
cause: signal.reason,
|
||||
});
|
||||
const error = new ProviderCleanupError(
|
||||
SQLITE_PROVIDER_IDENTITY.id,
|
||||
const error = createSqliteCleanupError(
|
||||
cause.cleanup.resources,
|
||||
{
|
||||
reason: cause.cleanup.reason,
|
||||
previous_error:
|
||||
previous === undefined ? null : projectAnalysisError(previous),
|
||||
},
|
||||
{
|
||||
operation,
|
||||
cause,
|
||||
...(previous === undefined ? {} : { cleanupPrimary: previous }),
|
||||
},
|
||||
@@ -91,7 +137,13 @@ export const sqliteDatabaseFailure = (
|
||||
return error;
|
||||
}
|
||||
if (cause.reason === "io" && cause.cause !== undefined)
|
||||
return sqliteDatabaseFailure(cause.cause, path, phase, signal);
|
||||
return sqliteDatabaseFailure(
|
||||
cause.cause,
|
||||
path,
|
||||
phase,
|
||||
signal,
|
||||
workerTimeoutMs,
|
||||
);
|
||||
if (cause.reason === "integrity")
|
||||
return new AnalysisArtifactChangedError(operation, path, cause.message, {
|
||||
cause,
|
||||
@@ -116,7 +168,7 @@ export const sqliteDatabaseFailure = (
|
||||
: cause,
|
||||
});
|
||||
if (cause instanceof OwnedCommandFailure) {
|
||||
const projected = ownedCommandFailure(cause);
|
||||
const projected = ownedCommandFailure(cause, workerTimeoutMs);
|
||||
if (projected !== undefined) return projected;
|
||||
}
|
||||
if (phase === "artifact-read" && cause instanceof Error && "code" in cause) {
|
||||
@@ -173,6 +225,7 @@ export const sqliteDatabaseFailure = (
|
||||
|
||||
const ownedCommandFailure = (
|
||||
cause: OwnedCommandFailure,
|
||||
workerTimeoutMs: number,
|
||||
): AnalysisError | undefined => {
|
||||
const operation = "inspect_sqlite_database";
|
||||
const capturedOutput =
|
||||
@@ -181,21 +234,37 @@ const ownedCommandFailure = (
|
||||
cause,
|
||||
...(capturedOutput === undefined ? {} : { capturedOutput }),
|
||||
};
|
||||
// Preserve the same typed primary whether or not owned cleanup also fails.
|
||||
const timeout =
|
||||
cause.reason === "timeout"
|
||||
? new AnalysisTimeoutError(operation, workerTimeoutMs, {
|
||||
...options,
|
||||
userMessage: `The SQLite worker reached its ${String(workerTimeoutMs)} ms server/provider deadline (worker_timeout_ms). This is not a whole-inspection or MCP client deadline.`,
|
||||
remediationAction: `${
|
||||
workerTimeoutMs < MAX_SQLITE_WORKER_TIMEOUT_MS
|
||||
? `Increase worker_timeout_ms above ${String(workerTimeoutMs)} (integer milliseconds, maximum ${String(MAX_SQLITE_WORKER_TIMEOUT_MS)}), or use rea inspect-sqlite-database <path> --worker-timeout-ms <milliseconds> on the CLI.`
|
||||
: `worker_timeout_ms is already at the maximum supported ${String(MAX_SQLITE_WORKER_TIMEOUT_MS)} ms; no higher worker deadline is supported. Report the blocker or narrow the question with caller approval.`
|
||||
} This per-call setting needs no server restart. It covers worker startup, queries and reply writing, not snapshot acquisition or final reply reads/cleanup. Allow client headroom for the whole call; increasing only the client deadline does not change the worker budget. Keep the selected database/WAL and requested facts; narrow table/columns/schema_scope/row_limit only if the question permits it. Byte and heap safety limits remain fixed. Resolve incomplete cleanup before retrying; do not repeat the unchanged failed call.`,
|
||||
})
|
||||
: undefined;
|
||||
if (cause.cleanupFailure !== null)
|
||||
return new ProviderCleanupError(
|
||||
SQLITE_PROVIDER_IDENTITY.id,
|
||||
return createSqliteCleanupError(
|
||||
cause.resources,
|
||||
{
|
||||
reason: cause.cleanupFailure,
|
||||
previous_error: {
|
||||
failure_kind: cause.reason,
|
||||
message: cause.message,
|
||||
},
|
||||
...(capturedOutput === undefined
|
||||
? {}
|
||||
: { captured_output: { ...capturedOutput } }),
|
||||
...(timeout === undefined
|
||||
? {
|
||||
previous_error: {
|
||||
failure_kind: cause.reason,
|
||||
message: cause.message,
|
||||
},
|
||||
}
|
||||
: {}),
|
||||
},
|
||||
{
|
||||
...options,
|
||||
...(timeout === undefined ? {} : { cleanupPrimary: timeout }),
|
||||
},
|
||||
{ operation, cause },
|
||||
);
|
||||
if (cause.reason === "cancelled")
|
||||
return new AnalysisCancelledError(operation, {
|
||||
@@ -203,12 +272,7 @@ const ownedCommandFailure = (
|
||||
cause:
|
||||
cause.cancellation === undefined ? cause : cause.cancellation.reason,
|
||||
});
|
||||
if (cause.reason === "timeout")
|
||||
return new AnalysisTimeoutError(
|
||||
operation,
|
||||
SQLITE_DATABASE_LIMITS.timeoutMs,
|
||||
options,
|
||||
);
|
||||
if (timeout !== undefined) return timeout;
|
||||
if (cause.reason === "output-limit")
|
||||
return new AnalysisOutputError(operation, cause.message, options);
|
||||
if (
|
||||
|
||||
@@ -549,7 +549,7 @@ export const inspectSqliteDatabaseSnapshot = (
|
||||
"SQLite reports current schema and selected committed rows from its valid database/WAL state. Deleted records, uncommitted or invalid WAL tails, encrypted databases, forensic recovery, and full integrity verification are outside this profile.",
|
||||
"Stored CREATE statements preserve index/view/trigger definitions. Views, triggers, and virtual/shadow table rows are not executed; virtual-table column expansion is unsupported.",
|
||||
"Selected rows have unspecified order and are a bounded sample. Text display may replace invalid sequences; bytes_hex and encoding preserve SQLite's text byte representation, not original on-disk page offsets.",
|
||||
"Resource boundaries: at most two concurrent inspections per provider; combined input 256 MiB per inspection, complete worker JSON 16 MiB, native SQLite value/record and SQL statement lengths 16 MiB, owned-worker JavaScript heap 256 MiB and supervised deadline 30 seconds. These are distinct from total resident memory; an overall native SQLite heap cap is not claimed. Failures return no partial success.",
|
||||
"Resource boundaries: at most two concurrent inspections per provider; combined input 256 MiB per inspection, complete worker JSON 16 MiB, native SQLite value/record and SQL statement lengths 16 MiB, owned-worker JavaScript heap 256 MiB. The parent applies a separate supervised worker deadline, reported in worker_timeout_ms. These are distinct from total resident memory; an overall native SQLite heap cap is not claimed. Failures return no partial success.",
|
||||
];
|
||||
const budget = new OutputBudget();
|
||||
const { schema, selectedTable } = readSchema(db, input, {
|
||||
|
||||
@@ -3,7 +3,6 @@ export const SQLITE_DATABASE_LIMITS = {
|
||||
concurrentInspections: 2,
|
||||
inputBytes: 256 * 1024 * 1024,
|
||||
outputBytes: 16 * 1024 * 1024,
|
||||
timeoutMs: 30_000,
|
||||
diagnosticBytes: 1024 * 1024,
|
||||
} as const;
|
||||
/** Internal rejection retains the failed boundary without returning partial success. */
|
||||
|
||||
@@ -27,6 +27,7 @@ import {
|
||||
sqliteDatabaseSchema,
|
||||
type InspectSqliteDatabaseInput,
|
||||
SQLITE_PROVIDER_IDENTITY,
|
||||
DEFAULT_SQLITE_WORKER_TIMEOUT_MS,
|
||||
sqliteInspectionParameters,
|
||||
} from "../domain/sqlite/sqliteDatabase.js";
|
||||
import { isAbsolute } from "node:path";
|
||||
@@ -39,7 +40,10 @@ import type {
|
||||
ProviderProcessStopResult,
|
||||
ProviderProcessSupervisor,
|
||||
} from "../process/ProviderProcess.js";
|
||||
import { sqliteDatabaseFailure } from "./SqliteDatabaseFailures.js";
|
||||
import {
|
||||
createSqliteCleanupError,
|
||||
sqliteDatabaseFailure,
|
||||
} from "./SqliteDatabaseFailures.js";
|
||||
import { captureSqliteDatabaseSnapshot } from "./SqliteDatabaseSnapshot.js";
|
||||
import { SQLITE_DATABASE_LIMITS } from "./SqliteDatabaseLimits.js";
|
||||
|
||||
@@ -257,6 +261,8 @@ export class SqliteDatabaseProvider implements SqliteDatabasePort {
|
||||
let retainedOutput: AnalysisCapturedOutput | undefined;
|
||||
let phase = "configuration";
|
||||
let result: Result<AnalysisExecution, AnalysisError>;
|
||||
const workerTimeoutMs =
|
||||
input.worker_timeout_ms ?? DEFAULT_SQLITE_WORKER_TIMEOUT_MS;
|
||||
try {
|
||||
if (options?.signal?.aborted)
|
||||
throw new AnalysisCancelledError(operation, {
|
||||
@@ -283,6 +289,7 @@ export class SqliteDatabaseProvider implements SqliteDatabasePort {
|
||||
input,
|
||||
root: root.path,
|
||||
snapshotPath: snapshot.snapshotPath,
|
||||
timeoutMs: workerTimeoutMs,
|
||||
environment: this.environment,
|
||||
resourceScope: this.#artifactResources,
|
||||
...(this.launcher === undefined ? {} : { launcher: this.launcher }),
|
||||
@@ -299,6 +306,7 @@ export class SqliteDatabaseProvider implements SqliteDatabasePort {
|
||||
artifact: snapshot.artifact,
|
||||
wal: snapshot.wal,
|
||||
...inspection,
|
||||
worker_timeout_ms: workerTimeoutMs,
|
||||
});
|
||||
if (!validated.success)
|
||||
throw new AnalysisOutputError(
|
||||
@@ -322,6 +330,7 @@ export class SqliteDatabaseProvider implements SqliteDatabasePort {
|
||||
input.path,
|
||||
phase,
|
||||
options?.signal,
|
||||
workerTimeoutMs,
|
||||
);
|
||||
retainedOutput ??= failure.capturedOutput;
|
||||
result = err(failure);
|
||||
@@ -495,17 +504,14 @@ const sqliteCleanupFailure = (
|
||||
const partialObservation = result.ok
|
||||
? completedSqliteEvidence(input, result.value)
|
||||
: result.error.partialObservation;
|
||||
return new ProviderCleanupError(
|
||||
SQLITE_PROVIDER_IDENTITY.id,
|
||||
return createSqliteCleanupError(
|
||||
resources,
|
||||
{
|
||||
...details,
|
||||
reason,
|
||||
previous_error: result.ok ? null : projectAnalysisError(result.error),
|
||||
...(output === undefined ? {} : { captured_output: { ...output } }),
|
||||
},
|
||||
{
|
||||
operation,
|
||||
...(output === undefined ? {} : { capturedOutput: output }),
|
||||
...(result.ok ? {} : { cleanupPrimary: result.error }),
|
||||
...(cause === undefined ? {} : { cause }),
|
||||
...(partialObservation === undefined ? {} : { partialObservation }),
|
||||
|
||||
@@ -8,7 +8,6 @@ import { writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { z } from "zod";
|
||||
import { fixedAllowanceOptions } from "../domain/fixedAllowanceRemediation.js";
|
||||
import type { AnalysisCapturedOutput } from "../domain/analysisErrorBase.js";
|
||||
import {
|
||||
AnalysisCancelledError,
|
||||
@@ -36,6 +35,7 @@ const operation = "inspect_sqlite_database";
|
||||
const {
|
||||
artifact: _artifactSchema,
|
||||
wal: _walSchema,
|
||||
worker_timeout_ms: _workerTimeoutSchema,
|
||||
...inspectionShape
|
||||
} = sqliteDatabaseSchema.shape;
|
||||
const inspectionSchema = z.strictObject(inspectionShape);
|
||||
@@ -101,7 +101,9 @@ const rejectedReply = (
|
||||
},
|
||||
{
|
||||
capturedOutput,
|
||||
...fixedAllowanceOptions("The SQLite worker reply allowance", "report"),
|
||||
retryable: false,
|
||||
remediationAction:
|
||||
"The SQLite value/record and worker JSON reply allowances are fixed at 16 MiB; worker_timeout_ms and client deadlines do not raise them. If it preserves the requested facts, select exact columns, reduce row_limit, or use schema_scope: inventory instead of database. This worker refusal leaves no completed Evidence to retrieve. Do not repeat the unchanged request or truncate required database bytes.",
|
||||
},
|
||||
);
|
||||
const argument = rejectedArgument[reply.reason];
|
||||
@@ -123,6 +125,7 @@ export const executeSqliteDatabaseWorker = async (request: {
|
||||
readonly input: InspectSqliteDatabaseInput;
|
||||
readonly root: string;
|
||||
readonly snapshotPath: string;
|
||||
readonly timeoutMs: number;
|
||||
readonly environment: Readonly<NodeJS.ProcessEnv>;
|
||||
readonly resourceScope: ArtifactResourceScope;
|
||||
readonly launcher?: SqliteWorkerLauncher;
|
||||
@@ -160,7 +163,7 @@ export const executeSqliteDatabaseWorker = async (request: {
|
||||
hostEnvironment: environment,
|
||||
},
|
||||
{
|
||||
timeoutMs: SQLITE_DATABASE_LIMITS.timeoutMs,
|
||||
timeoutMs: request.timeoutMs,
|
||||
diagnosticBytes: SQLITE_DATABASE_LIMITS.diagnosticBytes,
|
||||
},
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user