mirror of
https://github.com/openclaw/openclaw.git
synced 2026-09-28 05:54:09 +08:00
fix(state): retire invalid shared workers before reuse
Read-only metadata actors can outlive native-only fixture cleanup. Reusing an inode-matched actor without checking its original admission could reopen a retired path and acquire the plugin lifecycle lease in the wrong database. Validate the original admission, join idle actor retirement, and refuse replacement while its callbacks remain active. Await agent and shared-state fixture teardown before deleting roots, and retain refusal details in failures. Reproduced main CI run 36322558996's 44-file shard on Linux/Node 24.19.0: two lease-loss failures in 195.31s; immediate verification saw empty lease rows. All three deterministic relocation cases fail before the owner fix. Proof: fixed local focused suites 56/56; standalone plugin execution 23/23 in 70.11s (77.64s wrapper wall, one worker). Linux Testbox ordered shard 503 passed, 2 skipped; worker suite 33/33 in 37.89s (39.53s wrapper wall, one worker). Real-worker coverage proves relocated writes and active-owner fencing that isolated helper mocks cannot establish. Testbox workflow: https://github.com/openclaw/openclaw/actions/runs/36325573876 Core tsgo, infra/state test graphs, changed-file lint/format, diff check, and independent Codex P2 autoreview passed. No lease budget, schema, permission, or installed update-driver contract changes.
This commit is contained in:
@@ -93,6 +93,13 @@ not recreate a missing file. Preparing a new database directory and quarantining
|
||||
orphaned sidecars require the existing schema-maintenance owner; later permission
|
||||
hardening never recreates a removed directory.
|
||||
|
||||
Cached shared-state actors also retain their original read admission. Before reuse,
|
||||
the owner checks that admission even when a new caller has a matching file identity.
|
||||
Relocation or inode reuse retires an invalid idle actor before opening the current
|
||||
path; active work must settle before replacement. This prevents Doctor and plugin
|
||||
migrations from recreating retired database paths or acquiring leases in the wrong
|
||||
database. Existing update drivers and stored schemas need no migration.
|
||||
|
||||
Each SQLite broker worker admits up to 128 running and queued requests. A busy
|
||||
worker's admission queue does not consume another worker's request capacity;
|
||||
independent workers continue serving their databases. Requests on the same worker
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
|
||||
import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
import { Worker } from "node:worker_threads";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
} from "../agents/subagents/registry/subagent-registry.store.sqlite.js";
|
||||
import type { SubagentRunRecord } from "../agents/subagents/registry/subagent-registry.types.js";
|
||||
import { writeConfigMachineState } from "../state/config-machine-state-write.js";
|
||||
import { createOpenClawDatabaseMaintenanceScope } from "../state/openclaw-state-db-async-lifecycle.js";
|
||||
import { closeOpenClawStateDatabaseAsync } from "../state/openclaw-state-db-cache.js";
|
||||
import { openOpenClawStateDatabase } from "../state/openclaw-state-db.js";
|
||||
import { claimOpenClawStateOwnership } from "../state/openclaw-state-ownership-operations.js";
|
||||
@@ -377,6 +378,98 @@ describe("canonical shared-state worker admission", () => {
|
||||
expect(existsSync(captured.admission.databasePath)).toBe(false);
|
||||
});
|
||||
|
||||
it.each(["same scope", "different scope", "active callback"] as const)(
|
||||
"writes to the relocated database without recreating a retired inspection path (%s)",
|
||||
async (ownership) => {
|
||||
const seeded = context();
|
||||
const originalPath = seeded.admission.databasePath;
|
||||
const value = { generation: "retained-before-relocation", plugins: [] };
|
||||
writeConfigMachineState("plugins.installedIndex", value, {
|
||||
path: originalPath,
|
||||
env: seeded.environment,
|
||||
});
|
||||
await closeOpenClawStateDatabaseAsync();
|
||||
const originalMaintenance = createOpenClawDatabaseMaintenanceScope();
|
||||
const relocatedMaintenance =
|
||||
ownership === "same scope" ? originalMaintenance : createOpenClawDatabaseMaintenanceScope();
|
||||
try {
|
||||
const original = originalMaintenance.run(() =>
|
||||
captureOpenClawStateWorkerContext({
|
||||
path: originalPath,
|
||||
env: seeded.environment,
|
||||
}),
|
||||
);
|
||||
const relocate = () => {
|
||||
const relocatedRoot = dirs.make("openclaw-worker-relocated-");
|
||||
// Relocation keeps the real inode while ending the old path's admission.
|
||||
renameSync(path.dirname(originalPath), path.join(relocatedRoot, "state"));
|
||||
const relocated = relocatedMaintenance.run(() =>
|
||||
captureOpenClawStateWorkerContext({
|
||||
env: { OPENCLAW_STATE_DIR: relocatedRoot },
|
||||
}),
|
||||
);
|
||||
expect(relocated.admission.identity.key).toBe(original.admission.identity.key);
|
||||
expect(existsSync(originalPath)).toBe(false);
|
||||
return relocated;
|
||||
};
|
||||
const flow = buildFlowRecord({
|
||||
ownerKey: "agent:main:relocated-state",
|
||||
syncMode: "managed",
|
||||
controllerId: "tests/relocated-state",
|
||||
goal: "Write only to the current database",
|
||||
});
|
||||
const write = (relocated: ReturnType<typeof captureOpenClawStateWorkerContext>) =>
|
||||
executeOpenClawStateWorker(relocated, {
|
||||
type: "flows.createManaged",
|
||||
input: { flow },
|
||||
});
|
||||
const relocatedDuringCallback = await runOpenClawStateWorkerOperation(
|
||||
original,
|
||||
async (scope) => {
|
||||
expect(
|
||||
await scope.execute({
|
||||
type: "plugins.metadata.read",
|
||||
input: { selector: "installed-index", artifactPreservingReadOnly: true },
|
||||
}),
|
||||
).toEqual({ value_json: JSON.stringify(value) });
|
||||
if (ownership === "active callback") {
|
||||
const relocated = relocate();
|
||||
// The old callback cannot await its own retirement through a new caller.
|
||||
await expect(write(relocated)).rejects.toMatchObject({
|
||||
code: "STATE_DATABASE_READ_ADMISSION_INVALIDATED",
|
||||
});
|
||||
expect(existsSync(originalPath)).toBe(false);
|
||||
return relocated;
|
||||
}
|
||||
return undefined;
|
||||
},
|
||||
{ existingOnly: true },
|
||||
);
|
||||
const relocated = relocatedDuringCallback ?? relocate();
|
||||
await write(relocated);
|
||||
|
||||
expect(existsSync(originalPath)).toBe(false);
|
||||
const database = openOpenClawStateDatabase({
|
||||
path: relocated.admission.databasePath,
|
||||
env: relocated.environment,
|
||||
});
|
||||
expect(
|
||||
database.db.prepare("SELECT goal FROM flow_runs WHERE flow_id = ?").get(flow.flowId),
|
||||
).toEqual({ goal: "Write only to the current database" });
|
||||
expect(
|
||||
database.db
|
||||
.prepare("SELECT value_json FROM config_machine_state WHERE state_key = ?")
|
||||
.get("plugins.installedIndex"),
|
||||
).toEqual({ value_json: JSON.stringify(value) });
|
||||
} finally {
|
||||
await originalMaintenance.close();
|
||||
if (relocatedMaintenance !== originalMaintenance) {
|
||||
await relocatedMaintenance.close();
|
||||
}
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("preserves future-schema rejection through a cold worker open", async () => {
|
||||
const captured = context();
|
||||
const database = openOpenClawStateDatabase({
|
||||
|
||||
@@ -6,8 +6,11 @@ import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { pluginDoctorContractRegistryLoaderState } from "../plugins/doctor-contract-registry-loader-state.js";
|
||||
import { clearPluginDoctorContractRegistryCache } from "../plugins/doctor-contract-registry.test-fixtures.js";
|
||||
import { EMPTY_LEGACY_SESSION_SURFACES } from "../plugins/legacy-session-surfaces.types.js";
|
||||
import { closeOpenClawAgentDatabasesForTest } from "../state/openclaw-agent-db.js";
|
||||
import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js";
|
||||
import { closeOpenClawAgentDatabasesAsync } from "../state/openclaw-agent-db-lifecycle.js";
|
||||
import {
|
||||
closeOpenClawStateDatabaseAsync,
|
||||
closeOpenClawStateDatabaseForTest,
|
||||
} from "../state/openclaw-state-db.js";
|
||||
import { resolveOpenClawStateSqlitePath } from "../state/openclaw-state-db.paths.js";
|
||||
import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js";
|
||||
import {
|
||||
@@ -61,7 +64,8 @@ async function makeFixture() {
|
||||
afterEach(async () => {
|
||||
pluginDoctorContractRegistryLoaderState.moduleLoaderFactory = undefined;
|
||||
resetAutoMigrateLegacyStateDirForTest();
|
||||
closeOpenClawAgentDatabasesForTest();
|
||||
await closeOpenClawAgentDatabasesAsync();
|
||||
await closeOpenClawStateDatabaseAsync();
|
||||
closeOpenClawStateDatabaseForTest();
|
||||
await tempDirs.cleanup();
|
||||
vi.restoreAllMocks();
|
||||
@@ -539,11 +543,10 @@ module.exports = { stateMigrations: [{
|
||||
legacySessionSurfaces: EMPTY_LEGACY_SESSION_SURFACES,
|
||||
});
|
||||
|
||||
expect(
|
||||
result.stepReceipts.find(
|
||||
(receipt) => receipt.id === (legacyRoot ? "state-dir" : "plugin-install-index"),
|
||||
),
|
||||
).toMatchObject({ outcome: "completed" });
|
||||
const stateReceipt = result.stepReceipts.find(
|
||||
(receipt) => receipt.id === (legacyRoot ? "state-dir" : "plugin-install-index"),
|
||||
);
|
||||
expect(stateReceipt, JSON.stringify(stateReceipt)).toMatchObject({ outcome: "completed" });
|
||||
expect(fs.realpathSync(legacyStateDir)).toBe(fs.realpathSync(stateDir));
|
||||
expect(result.warnings).toEqual([]);
|
||||
if (legacySchema) {
|
||||
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
import { runInDetachedAsyncContext } from "../shared/async-work-scope.js";
|
||||
import { resolveGlobalSingleton } from "../shared/global-singleton.js";
|
||||
import { isStateDatabaseReadAdmissionInvalidatedError } from "./openclaw-state-db-async-lifecycle.js";
|
||||
import {
|
||||
publishOpenClawStateDatabaseWorkerAdmission,
|
||||
registerOpenClawStateDatabaseAsyncResource,
|
||||
@@ -383,10 +384,27 @@ function createSharedStateWorkerOwner() {
|
||||
let entry: Entry | undefined;
|
||||
for (;;) {
|
||||
for (const candidate of stores) {
|
||||
if (!matches(candidate, admission.identity)) {
|
||||
continue;
|
||||
}
|
||||
try {
|
||||
// Other scopes can share this actor; an inode match cannot renew its original admission.
|
||||
candidate.context.admission.assertCurrent();
|
||||
} catch (error) {
|
||||
if (
|
||||
!isStateDatabaseReadAdmissionInvalidatedError(error) ||
|
||||
hasActiveActorOperations(candidate)
|
||||
) {
|
||||
throw error;
|
||||
}
|
||||
await (candidate.actor
|
||||
? retireActor(candidate.actor, candidate.context.admission.identity)
|
||||
: retire(candidate));
|
||||
return this.open(context, options);
|
||||
}
|
||||
if (
|
||||
matches(candidate, admission.identity) &&
|
||||
(candidate.context.existingSchemaPath !== context.existingSchemaPath ||
|
||||
candidate.source.moduleUrl.href !== source.moduleUrl.href)
|
||||
candidate.context.existingSchemaPath !== context.existingSchemaPath ||
|
||||
candidate.source.moduleUrl.href !== source.moduleUrl.href
|
||||
) {
|
||||
await retire(candidate);
|
||||
assertAdmission();
|
||||
|
||||
Reference in New Issue
Block a user