Skip to content

Commit 1c31a6f

Browse files
steipetesteipete
committed
fix(doctor): repair ALTER-appended operator approval schema instead of wedging startup
1 parent 6745f9a commit 1c31a6f

4 files changed

Lines changed: 172 additions & 16 deletions

‎src/state/openclaw-state-db-operator-approval-migration.test.ts‎

Lines changed: 87 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,59 @@ function legacyTwoKindCreateSql(): string {
2424
.replace(/'exec',\s*'plugin',\s*'system-agent'/, "'exec', 'plugin'");
2525
}
2626

27-
function seedRow(db: DatabaseSync, kind: string): void {
28-
db.exec(`
27+
function withoutResolutionRefColumn(sql: string): string {
28+
const resolutionRefStart = sql.indexOf("\n resolution_ref ");
29+
const followingColumnStart = sql.indexOf("\n kind ", resolutionRefStart);
30+
if (resolutionRefStart < 0 || followingColumnStart < 0) {
31+
throw new Error("resolution_ref column not found");
32+
}
33+
return sql.slice(0, resolutionRefStart) + sql.slice(followingColumnStart);
34+
}
35+
36+
function createAlterAppendedLegacyTable(db: DatabaseSync, strict: boolean): void {
37+
const createSql = withoutResolutionRefColumn(
38+
strict ? legacyTwoKindCreateSql().replace(/\);$/u, ") STRICT;") : legacyTwoKindCreateSql(),
39+
);
40+
db.exec(createSql);
41+
db.exec("ALTER TABLE operator_approvals ADD COLUMN resolution_ref TEXT;");
42+
}
43+
44+
const RESOLUTION_REF = "ref0000000000000000000000000000000000000000";
45+
46+
function seedRow(
47+
db: DatabaseSync,
48+
kind: string,
49+
resolutionRef: string | null = RESOLUTION_REF,
50+
approvalId = "a1",
51+
): void {
52+
db.prepare(`
2953
INSERT INTO operator_approvals (
3054
approval_id, resolution_ref, kind, status, presentation_json,
3155
reviewer_device_ids_json, audience_session_keys_json, runtime_epoch,
3256
created_at_ms, expires_at_ms, updated_at_ms
3357
) VALUES (
34-
'a1', 'ref0000000000000000000000000000000000000000', '${kind}',
35-
'pending', '{}', '[]', '[]', 1, 1, 1, 1
58+
?, ?, ?, 'pending', '{}', '[]', '[]', 1, 1, 1, 1
3659
);
37-
`);
60+
`).run(approvalId, resolutionRef, kind);
61+
}
62+
63+
function expectAlterAppendedLegacyRepair(strict: boolean): void {
64+
const db = new DatabaseSync(":memory:");
65+
createAlterAppendedLegacyTable(db, strict);
66+
seedRow(db, "exec");
67+
68+
expect(repairOperatorApprovalSchema(db)).toEqual([
69+
"Migrated shared state operator approvals → OpenClaw system changes",
70+
]);
71+
72+
expect(() => assertCanonicalOperatorApprovalKinds(db, ":memory:")).not.toThrow();
73+
expect(
74+
db.prepare("SELECT approval_id, kind, resolution_ref FROM operator_approvals").all(),
75+
).toEqual([{ approval_id: "a1", kind: "exec", resolution_ref: RESOLUTION_REF }]);
76+
expect(
77+
db.prepare("SELECT strict FROM pragma_table_list WHERE name = 'operator_approvals'").get(),
78+
).toEqual({ strict: 1 });
79+
db.close();
3880
}
3981

4082
describe("repairOperatorApprovalKinds", () => {
@@ -51,22 +93,59 @@ describe("repairOperatorApprovalKinds", () => {
5193
]);
5294

5395
expect(() => assertCanonicalOperatorApprovalKinds(db, ":memory:")).not.toThrow();
54-
const rows = db.prepare("SELECT approval_id, kind FROM operator_approvals").all();
55-
expect(rows).toEqual([{ approval_id: "a1", kind: "exec" }]);
96+
const rows = db
97+
.prepare("SELECT approval_id, kind, resolution_ref FROM operator_approvals")
98+
.all();
99+
expect(rows).toEqual([{ approval_id: "a1", kind: "exec", resolution_ref: RESOLUTION_REF }]);
56100
expect(
57101
db.prepare("SELECT strict FROM pragma_table_list WHERE name = 'operator_approvals'").get(),
58102
).toEqual({ strict: 1 });
59103
db.close();
60104
});
61105

106+
it("migrates the ALTER-appended non-STRICT legacy shape", () => {
107+
expectAlterAppendedLegacyRepair(false);
108+
});
109+
110+
it("migrates the ALTER-appended STRICT legacy shape", () => {
111+
expectAlterAppendedLegacyRepair(true);
112+
});
113+
114+
it("drops pre-ref rows that violate the canonical resolution_ref shape", () => {
115+
const db = new DatabaseSync(":memory:");
116+
createAlterAppendedLegacyTable(db, false);
117+
seedRow(db, "exec");
118+
seedRow(db, "exec", null, "a2-null-ref");
119+
seedRow(db, "plugin", "short", "a3-bad-ref");
120+
// Non-STRICT legacy tables can hold a BLOB in the TEXT column; it must be
121+
// filtered out or the STRICT destination insert aborts the whole repair.
122+
db.prepare(`
123+
INSERT INTO operator_approvals (
124+
approval_id, resolution_ref, kind, status, presentation_json,
125+
reviewer_device_ids_json, audience_session_keys_json, runtime_epoch,
126+
created_at_ms, expires_at_ms, updated_at_ms
127+
) VALUES ('a4-blob-ref', ?, 'exec', 'pending', '{}', '[]', '[]', 1, 1, 1, 1);
128+
`).run(Buffer.from("ref0000000000000000000000000000000000000000"));
129+
130+
expect(repairOperatorApprovalSchema(db)).toEqual([
131+
"Migrated shared state operator approvals → OpenClaw system changes",
132+
]);
133+
134+
expect(() => assertCanonicalOperatorApprovalKinds(db, ":memory:")).not.toThrow();
135+
expect(db.prepare("SELECT approval_id, resolution_ref FROM operator_approvals").all()).toEqual([
136+
{ approval_id: "a1", resolution_ref: RESOLUTION_REF },
137+
]);
138+
db.close();
139+
});
140+
62141
it("is a no-op when the schema is already canonical", () => {
63142
const db = new DatabaseSync(":memory:");
64143
db.exec(canonicalOperatorApprovalCreateSql());
65144
expect(repairOperatorApprovalSchema(db)).toEqual([]);
66145
db.close();
67146
});
68147

69-
it("refuses to replace a look-alike table with the same columns but different constraints", () => {
148+
it("refuses to replace an arbitrarily different table", () => {
70149
const db = new DatabaseSync(":memory:");
71150
// Same column names, but a different (non-canonical) kind constraint — the
72151
// fail-closed guard must not copy its rows under today's schema.

‎src/state/openclaw-state-db-operator-approval-migration.ts‎

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,21 @@ function canonicalOperatorApprovalCreateSql(): string {
8989
return OPENCLAW_STATE_SCHEMA_SQL.slice(start, end + tableTerminator.length);
9090
}
9191

92-
// The only legacy shape this repair may destructively replace is the exact
93-
// prior canonical table with the two-kind constraint (before 'system-agent').
92+
function alterAppendedResolutionRefCreateSql(sql: string): string {
93+
const resolutionRefStart = sql.indexOf("\n resolution_ref ");
94+
const followingColumnStart = sql.indexOf("\n kind ", resolutionRefStart);
95+
const tailColumn = "\n consumed_by TEXT,";
96+
const tailColumnStart = sql.indexOf(tailColumn, followingColumnStart);
97+
if (resolutionRefStart < 0 || followingColumnStart < 0 || tailColumnStart < 0) {
98+
throw new Error("canonical operator approval resolution reference schema is unavailable");
99+
}
100+
const withoutResolutionRef = sql.slice(0, resolutionRefStart) + sql.slice(followingColumnStart);
101+
return withoutResolutionRef.replace(tailColumn, `${tailColumn} resolution_ref TEXT,`);
102+
}
103+
104+
// The only legacy shapes this repair may destructively replace are the exact
105+
// prior canonical table with the two-kind constraint (before 'system-agent')
106+
// and its shipped ALTER-appended resolution_ref variant.
94107
// Matching column names alone is not fail-closed: a table with the same names
95108
// but different constraints/defaults/types would get its rows copied under a
96109
// schema that silently discards those semantics.
@@ -103,10 +116,13 @@ function hasExactLegacyOperatorApprovalSchema(db: DatabaseSync): boolean {
103116
// sqlite_master stores the CREATE statement without "IF NOT EXISTS".
104117
.replace("CREATE TABLE IF NOT EXISTS operator_approvals (", "CREATE TABLE operator_approvals (")
105118
.replace(/'exec',\s*'plugin',\s*'system-agent'/, "'exec', 'plugin'");
106-
const expectedStrictLegacy = normalizeDdl(exactStrictLegacy);
107-
const expectedFlexibleLegacy = normalizeDdl(exactStrictLegacy.replace(/\) STRICT;$/u, ");"));
108119
const normalizedLive = normalizeDdl(live);
109-
return normalizedLive === expectedStrictLegacy || normalizedLive === expectedFlexibleLegacy;
120+
const alterAppendedStrictLegacy = alterAppendedResolutionRefCreateSql(exactStrictLegacy);
121+
return [exactStrictLegacy, alterAppendedStrictLegacy].some((strictLegacy) =>
122+
[strictLegacy, strictLegacy.replace(/\) STRICT;$/u, ");")]
123+
.map(normalizeDdl)
124+
.includes(normalizedLive),
125+
);
110126
}
111127

112128
function canonicalCreateSql(): string {
@@ -116,6 +132,22 @@ function canonicalCreateSql(): string {
116132
);
117133
}
118134

135+
// Rebuilding the table drops its indexes. Replay only the operator-approval
136+
// index statements: executing the full schema here fails on many-versions-
137+
// behind databases whose other tables still lack columns the later additive
138+
// and STRICT repairs add (e.g. "no such column: agent_id").
139+
function operatorApprovalIndexSql(): string {
140+
const statements = OPENCLAW_STATE_SCHEMA_SQL.split(";")
141+
.map((statement) => statement.trim())
142+
.filter((statement) =>
143+
/^CREATE (?:UNIQUE )?INDEX IF NOT EXISTS idx_operator_approvals_/.test(statement),
144+
);
145+
if (statements.length === 0) {
146+
throw new Error("canonical operator approval index schema is unavailable");
147+
}
148+
return `${statements.join(";\n")};`;
149+
}
150+
119151
function repairOperatorApprovalKinds(db: DatabaseSync): boolean {
120152
if (
121153
hasCanonicalOperatorApprovalKinds(db) ||
@@ -130,13 +162,21 @@ function repairOperatorApprovalKinds(db: DatabaseSync): boolean {
130162
// recreate an empty canonical table and abandon them.
131163
runSqliteImmediateTransactionSync(db, () => {
132164
db.exec(canonicalCreateSql());
165+
// The ALTER-appended variant is nullable, so pre-ref rows can hold NULL or
166+
// junk that violates the canonical NOT NULL/shape CHECK. Approvals are
167+
// transient runtime state: drop nonconforming rows instead of aborting the
168+
// whole repair and re-wedging startup. The filter mirrors the canonical
169+
// CHECK exactly.
133170
db.exec(`
134171
INSERT INTO operator_approvals_migration_new (${columns})
135-
SELECT ${columns} FROM operator_approvals;
172+
SELECT ${columns} FROM operator_approvals
173+
WHERE typeof(resolution_ref) = 'text'
174+
AND length(resolution_ref) = 43
175+
AND resolution_ref NOT GLOB '*[^A-Za-z0-9_-]*';
136176
DROP TABLE operator_approvals;
137177
ALTER TABLE operator_approvals_migration_new RENAME TO operator_approvals;
138178
`);
139-
db.exec(OPENCLAW_STATE_SCHEMA_SQL);
179+
db.exec(operatorApprovalIndexSql());
140180
});
141181
return true;
142182
}

‎src/state/openclaw-state-db.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2101,6 +2101,37 @@ describe("openclaw state database", () => {
21012101
expect(migratedSql.sql).toContain("'system-agent'");
21022102
});
21032103

2104+
it("does not recursively recommend doctor when operator approval repair refuses a shape", () => {
2105+
const stateDir = createTempStateDir();
2106+
const options = { env: { OPENCLAW_STATE_DIR: stateDir } };
2107+
const database = openOpenClawStateDatabase(options);
2108+
const databasePath = database.path;
2109+
closeOpenClawStateDatabaseForTest();
2110+
2111+
const { DatabaseSync } = requireNodeSqlite();
2112+
const customizedDb = new DatabaseSync(databasePath);
2113+
const currentSql = (
2114+
customizedDb
2115+
.prepare(
2116+
"SELECT sql FROM sqlite_schema WHERE type = 'table' AND name = 'operator_approvals'",
2117+
)
2118+
.get() as { sql: string }
2119+
).sql;
2120+
customizedDb.exec("ALTER TABLE operator_approvals RENAME TO operator_approvals_current");
2121+
customizedDb.exec(
2122+
currentSql.replace("'exec', 'plugin', 'system-agent'", "'exec', 'plugin', 'custom-thing'"),
2123+
);
2124+
customizedDb.exec("DROP TABLE operator_approvals_current");
2125+
customizedDb.close();
2126+
2127+
const result = repairOpenClawStateDatabaseSchema(options);
2128+
expect(result.changes).toEqual([]);
2129+
expect(result.warnings).toEqual([
2130+
expect.stringContaining("automatic repair refused the unrecognized schema shape"),
2131+
]);
2132+
expect(result.warnings[0]).not.toContain("run openclaw doctor --fix");
2133+
});
2134+
21042135
it("adds managed-image typed columns before creating canonical indexes", () => {
21052136
const stateDir = createTempStateDir();
21062137
const options = { env: { OPENCLAW_STATE_DIR: stateDir } };

‎src/state/openclaw-state-db.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -850,9 +850,15 @@ export function repairOpenClawStateDatabaseSchema(options: OpenClawStateDatabase
850850
);
851851
return { changes, warnings: [] };
852852
} catch (err) {
853+
// Reaching this catch inside doctor means repair itself refused or failed,
854+
// so the runtime asserts' "run openclaw doctor --fix" advice is circular here.
855+
const reason = String(err).replace(
856+
/has a legacy ([a-z ]+) schema; run openclaw doctor --fix to migrate it\./u,
857+
"has a legacy $1 schema; automatic repair refused the unrecognized schema shape.",
858+
);
853859
return {
854860
changes: [],
855-
warnings: [`Failed migrating shared state database schema at ${pathname}: ${String(err)}`],
861+
warnings: [`Failed migrating shared state database schema at ${pathname}: ${reason}`],
856862
};
857863
} finally {
858864
if (db.isOpen) {

0 commit comments

Comments
 (0)