mirror of
https://github.com/openclaw/openclaw.git
synced 2026-09-28 05:54:09 +08:00
fix(doctor): skip GitHub issue draft for a clean session SQLite recovery (#155246)
Closes #155171 ## What Problem This Solves Doctor creates migration-failure reports and a GitHub support draft after session SQLite recovery has finished cleanly, with no remaining issues or restore conflicts. ## User Impact Clean recovery retains its manifest path and run identifier without creating new failure-report files or offering a false support issue. Real remaining issues and restore conflicts still produce reports. Existing historical report files are preserved. ## Why This Change Was Made Gate failure-report creation at its existing producer, after restoration, validation, and restore-conflict aggregation. A zero-issue report returns before failure artifacts or the support payload are authored. Recovery itself, stored formats, consent, and GitHub submission behavior are unchanged; this is not a special case for zero file moves. The ordinary updater and Doctor health implementations are untouched. The change is limited to recovery-report production after the existing restore and validation work. ## Evidence The real built CLI was run once before and once after the change against separate, otherwise equivalent synthetic recovery fixtures: ```sh node openclaw.mjs doctor --session-sqlite recover --github-issue --json \ --session-sqlite-store <isolated-store> --session-sqlite-agent main ``` Both runs deliberately omitted `--yes`. Their state, home, and temporary directories were isolated, and credentials were omitted from the child environment. | Observation | Before | After | | --- | --- | --- | | Remaining issues / restore conflicts | 0 / 0 | 0 / 0 | | Restore activity | No-op | No-op | | Recovery manifest and run identifier | Retained | Retained | | New `.failure.json` and `.failure.md` | Both created | Neither created | | Support-issue payload | Present; GitHub status `skipped` | Absent | | CLI exit | 0 | 0 | - The two clean-recovery regression cases fail against the original producer and pass after the fix. They cover fresh recovery and byte-for-byte preservation of existing historical reports. - **41/41 focused tests passed** across report, recovery, and manifest suites, including a real filesystem restore conflict that remains reported without replacing the current source, existing remaining-issue reporting, sanitization, and restore controls. Combined command wall: 47.40 seconds; the changed test file's 15 test bodies took 4.133 seconds. Hosted CI timing is not yet available. - The actual Doctor test-type project, targeted type-aware lint, formatting, line-cap, max-lines, assertion-safety, and scoped diff checks passed. - The candidate runtime was built after contributor-preserving integration; its complete source tree matches the tested tree. ### Published updater compatibility One isolated, unprivileged **bare Docker** cell installed the integrity-pinned published `openclaw@2026.9.5` package. That installed release's public SDK created the old configuration state; its updater then installed the exact candidate tarball with `update --tag file:<candidate> --yes --json --no-restart`. The result was `status: "ok"`: the installed build changed from release `ec9c1a13db89` to candidate `cebc02f6de19`, and the authored configuration values survived. The maintained upgrade-success assertion passed. Normal migration, health, configuration, plugin, Gateway-startup, and post-install Doctor checks completed; nonfatal environment advisories were retained, not hidden. This was one compatibility cell, not a full upgrade matrix. The host's global `/tmp`, sockets, credential directories, and operator state were not mounted into the container; only the task-owned evidence mount was writable. The container and its task-owned image were removed afterward. No live GitHub issue was submitted. The standalone recovery runs used the verified JSON/no-yes consent return and exited before the full Doctor health flow. Normal update-time Doctor execution occurred only inside the isolated container, never against host operator state. Interactive issue consent was not exercised. Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
Jialong(Bruce) Li
co-authored by
Ayaan Zaidi
Ayaan Zaidi
parent
cc216b8775
commit
86fc1df067
@@ -134,11 +134,15 @@ export async function recoverDoctorSessionSqliteTargets(params: {
|
||||
})),
|
||||
);
|
||||
const report = summarizeRecoverReport(targetReports.length > 0 ? targetReports : [reportTarget]);
|
||||
if (report.totals.issues === 0) {
|
||||
report.migrationRun = {
|
||||
manifestPath: failedRun.manifestPath,
|
||||
runId: failedRun.manifest.runId,
|
||||
};
|
||||
return report;
|
||||
}
|
||||
const failureReports = writeSessionSqliteMigrationFailureReports(failedRun.manifestPath, {
|
||||
reason:
|
||||
report.totals.issues > 0
|
||||
? "doctor recover completed with remaining issues"
|
||||
: "doctor recover completed validation of a failed session SQLite migration run",
|
||||
reason: "doctor recover completed with remaining issues",
|
||||
recoveryTargets: report.targets,
|
||||
trustedTargets,
|
||||
});
|
||||
|
||||
@@ -131,6 +131,86 @@ describe("runDoctorSessionSqlite", () => {
|
||||
expect(recover.supportIssue).not.toHaveProperty("url");
|
||||
});
|
||||
|
||||
it("keeps a support report for a restore conflict without replacing the current source", async () => {
|
||||
const store = createLegacyStore();
|
||||
const imported = await importLegacyStore(store);
|
||||
const manifestPath = requireMigrationManifestPath(imported.migrationRun?.manifestPath);
|
||||
const manifest = readMigrationManifest(manifestPath);
|
||||
manifest.failedAt = "2030-01-01T00:00:00.000Z";
|
||||
fs.writeFileSync(manifestPath, JSON.stringify(manifest), { mode: 0o600 });
|
||||
const replacement = '{"type":"event","id":"newer-source"}\n';
|
||||
fs.writeFileSync(store.transcriptPath, replacement, { mode: 0o600 });
|
||||
|
||||
const recover = await runDoctorSessionSqlite({ cfg: {}, env: store.env, mode: "recover" });
|
||||
|
||||
expect(recover.targets[0]?.restore?.conflicts).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ sourcePath: canonicalTestPaths([store.transcriptPath])[0] }),
|
||||
]),
|
||||
);
|
||||
expect(recover.targets[0]?.issues).toEqual(
|
||||
expect.arrayContaining([expect.objectContaining({ code: "restore_conflict" })]),
|
||||
);
|
||||
expect(recover.supportIssue?.body).toContain("restore_conflict");
|
||||
expect(fs.readFileSync(store.transcriptPath, "utf8")).toBe(replacement);
|
||||
});
|
||||
|
||||
it.each([false, true])(
|
||||
"does not prepare failure reports for clean recovery (existing reports=%s)",
|
||||
async (existingReports) => {
|
||||
const store = createLegacyStore();
|
||||
for (const file of [
|
||||
store.storePath,
|
||||
store.transcriptPath,
|
||||
store.trajectoryPath,
|
||||
store.unreferencedJsonlPath,
|
||||
]) {
|
||||
fs.rmSync(file);
|
||||
}
|
||||
const runsDir = path.join(store.stateDir, "session-sqlite-migration-runs");
|
||||
fs.mkdirSync(runsDir, { recursive: true, mode: 0o700 });
|
||||
const manifestPath = path.join(runsDir, "clean-recovery.json");
|
||||
const manifest: SessionSqliteMigrationManifest = {
|
||||
failedAt: "2030-01-01T00:00:00.000Z",
|
||||
manifestVersion: 3,
|
||||
openClawVersion: "test",
|
||||
runId: "clean-recovery",
|
||||
startedAt: "2030-01-01T00:00:00.000Z",
|
||||
targets: [
|
||||
{
|
||||
...trustedMigrationTarget(store),
|
||||
completedMoves: [],
|
||||
issues: [],
|
||||
plannedMoves: [],
|
||||
validationBeforeArchive: "not_run",
|
||||
},
|
||||
],
|
||||
};
|
||||
fs.writeFileSync(manifestPath, `${JSON.stringify(manifest, null, 2)}\n`, { mode: 0o600 });
|
||||
const previousReports = existingReports
|
||||
? writeSessionSqliteMigrationFailureReports(manifestPath, { reason: "Earlier recovery" })
|
||||
: undefined;
|
||||
const previousBytes = previousReports
|
||||
? [previousReports.jsonPath, previousReports.markdownPath].map((file) =>
|
||||
fs.readFileSync(file),
|
||||
)
|
||||
: undefined;
|
||||
|
||||
const recover = await runDoctorSessionSqlite({ cfg: {}, env: store.env, mode: "recover" });
|
||||
|
||||
expect(recover.totals.issues).toBe(0);
|
||||
expect(recover.migrationRun).toEqual({ manifestPath, runId: "clean-recovery" });
|
||||
expect(recover.supportIssue).toBeUndefined();
|
||||
if (previousReports && previousBytes) {
|
||||
expect(fs.readFileSync(previousReports.jsonPath)).toEqual(previousBytes[0]);
|
||||
expect(fs.readFileSync(previousReports.markdownPath)).toEqual(previousBytes[1]);
|
||||
} else {
|
||||
expect(fs.existsSync(manifestPath.replace(/\.json$/u, ".failure.json"))).toBe(false);
|
||||
expect(fs.existsSync(manifestPath.replace(/\.json$/u, ".failure.md"))).toBe(false);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["replaced", "missing"] as const)(
|
||||
"refuses a support claim when the saved report is %s during consent",
|
||||
(change) => {
|
||||
|
||||
Reference in New Issue
Block a user