fix(sf): repair doctor orphan cleanup

This commit is contained in:
Mikael Hugo 2026-05-02 14:34:16 +02:00
parent f990ce1048
commit 9f773815d1
6 changed files with 296 additions and 10 deletions

View file

@ -1,9 +1,8 @@
import { existsSync, statSync } from "node:fs";
import { existsSync, readdirSync, rmSync, statSync } from "node:fs";
import { join } from "node:path";
import type { DoctorIssue } from "./doctor-types.js";
import { resolveMilestoneFile } from "./paths.js";
import { _getAdapter, isDbAvailable } from "./sf-db.js";
import type { DoctorIssue, DoctorIssueCode } from "./doctor-types.js";
import { milestonesDir, resolveMilestoneFile } from "./paths.js";
import { _getAdapter, getAllMilestones, isDbAvailable } from "./sf-db.js";
import { deriveState } from "./state.js";
import { readEvents } from "./workflow-events.js";
import { renderAllProjections } from "./workflow-projections.js";
@ -19,6 +18,7 @@ export async function checkEngineHealth(
basePath: string,
issues: DoctorIssue[],
fixesApplied: string[],
shouldFix?: (code: DoctorIssueCode) => boolean,
): Promise<void> {
const dbPath = join(basePath, ".sf", "sf.db");
@ -183,6 +183,70 @@ export async function checkEngineHealth(
// Non-fatal — DB constraint checks failed entirely
}
// ── Orphaned milestone directories ─────────────────────────────────────
// Detect .sf/milestones/* directories that have no corresponding DB row.
// These are leftover from manual cleanup, failed deletions, or DB resets.
// When DB is available, DB is authoritative. When DB is unavailable,
// fall back to filesystem-derived registry (roadmap-based discovery).
try {
const msDir = milestonesDir(basePath);
if (existsSync(msDir)) {
const validMilestoneIds = new Set<string>();
if (isDbAvailable()) {
// DB-authoritative: only DB rows count as valid
for (const m of getAllMilestones()) {
validMilestoneIds.add(m.id);
}
} else {
// No DB: fall back to filesystem registry
const state = await deriveState(basePath);
for (const m of state.registry) {
validMilestoneIds.add(m.id);
}
}
for (const entry of readdirSync(msDir)) {
const fullPath = join(msDir, entry);
try {
if (!statSync(fullPath).isDirectory()) continue;
} catch {
continue;
}
// Extract milestone ID from directory name (handles M001, M001-r5jzab, etc.)
const milestoneId = entry.split("-")[0];
if (!milestoneId) continue;
if (
!validMilestoneIds.has(milestoneId) &&
!validMilestoneIds.has(entry)
) {
issues.push({
severity: "warning",
code: "orphaned_milestone_directory",
scope: "project",
unitId: entry,
message: `Milestone directory ${fullPath} exists on disk but has no corresponding database entry or roadmap. It may be leftover from manual cleanup or a DB reset.`,
fixable: true,
});
if (shouldFix?.("orphaned_milestone_directory")) {
try {
rmSync(fullPath, { recursive: true, force: true });
fixesApplied.push(
`removed orphaned milestone directory ${fullPath}`,
);
} catch {
fixesApplied.push(
`failed to remove orphaned milestone directory ${fullPath}`,
);
}
}
}
}
}
} catch {
// Non-fatal — orphaned milestone directory check failed
}
// ── Projection drift detection ──────────────────────────────────────────
// If the DB is available, check whether markdown projections are stale
// relative to the event log and re-render them.

View file

@ -81,6 +81,8 @@ export type DoctorIssueCode =
| "db_duplicate_id"
| "db_unavailable"
| "projection_drift"
// Milestone directory orphan check
| "orphaned_milestone_directory"
// ADR-021: scaffold versioning
| "scaffold_drift"
// Audit projection health

View file

@ -610,7 +610,7 @@ export async function runSFDoctor(
const envMs = Date.now() - t0env;
// Engine health checks — DB constraints and projection drift
await checkEngineHealth(basePath, issues, fixesApplied);
await checkEngineHealth(basePath, issues, fixesApplied, shouldFix);
const milestonesPath = milestonesDir(basePath);
if (!existsSync(milestonesPath)) {

View file

@ -0,0 +1,167 @@
/**
* native-edit-bridge.ts Wire the Rust-backed edit engine from @singularity-forge/native
* into the SF extension editing flows.
*
* Purpose: applyWorkspaceEdit and applyEdits (from @singularity-forge/native/edit)
* provide atomic multi-file LSP-style edits with Rust-level staging and fsync.
* This bridge makes them available to the SF edit tool so multi-file refactors
* (rename, code-action) bypass the JavaScript read-splice-write loop.
*
* Consumer: tools/edit-tool-executor.ts (registered as an SF tool that delegates
* to applyWorkspaceEdit for LSP rename/code-action results).
*
* Wire status (gap-audit.ts:124):
* - applyEdits imported here (native-edit-bridge.ts)
* - applyWorkspaceEdit imported here (native-edit-bridge.ts)
* - replaceSymbol imported here (native-edit-bridge.ts)
* - insertAroundSymbol imported here (native-edit-bridge.ts)
*/
import { createRequire } from "node:module";
const require = createRequire(import.meta.url);
type NativeEditModule = {
applyEdits: (
filePath: string,
edits: unknown[],
options?: unknown,
) => unknown;
applyWorkspaceEdit: (documentEdits: unknown[], options?: unknown) => unknown;
replaceSymbol: (
filePath: string,
symbolPath: string,
newBody: string,
options?: unknown,
) => unknown;
insertAroundSymbol: (
filePath: string,
symbolPath: string,
position: unknown,
code: string,
options?: unknown,
) => unknown;
};
let _native: NativeEditModule | null = null;
let _loadAttempted = false;
function getNative(): NativeEditModule | null {
if (_loadAttempted) return _native;
_loadAttempted = true;
try {
// Dynamic require to avoid hard dependency; fails gracefully if native is not built.
const mod = require("@singularity-forge/native") as NativeEditModule;
if (
mod.applyEdits &&
mod.applyWorkspaceEdit &&
mod.replaceSymbol &&
mod.insertAroundSymbol
) {
_native = mod;
}
} catch {
// Native module not available — callers fall back to JS edit
}
return _native;
}
/**
* Return whether the native edit engine can be loaded in this runtime.
*
* Purpose: let edit tools choose the Rust-backed path only when the optional
* native package is present, preserving JS fallback behavior in dev shells.
*
* Consumer: SF edit executors before dispatching multi-file workspace edits.
*/
export function isNativeEditAvailable(): boolean {
return getNative() !== null;
}
/**
* Apply LSP-style TextEdit entries to one file atomically (Rust-backed).
*
* Purpose: expose atomic native single-file edit application without making
* the SF extension hard-depend on a built native package.
*
* Consumer: SF edit executors for file-scoped LSP text edits.
*
* Falls back to null when the native module is unavailable.
*/
export function nativeApplyEdits(
filePath: string,
edits: unknown[],
options?: unknown,
): unknown {
const native = getNative();
if (!native) return null;
return native.applyEdits(filePath, edits, options);
}
/**
* Apply LSP-style workspace edits across multiple files (Rust-backed).
*
* Purpose: keep multi-file rename/code-action writes atomic when the native
* engine is available.
*
* Consumer: SF edit executors for LSP workspace edit results.
*
* Falls back to null when the native module is unavailable.
*/
export function nativeApplyWorkspaceEdit(
documentEdits: unknown[],
options?: unknown,
): unknown {
const native = getNative();
if (!native) return null;
return native.applyWorkspaceEdit(documentEdits, options);
}
/**
* Replace a symbol declaration by its semantic path (Rust-backed).
*
* Purpose: make semantic symbol replacement available to SF tools without
* duplicating parser/edit logic in TypeScript.
*
* Consumer: SF edit executors for native symbol-aware rewrites.
*
* Falls back to null when the native module is unavailable.
*/
export function nativeReplaceSymbol(
filePath: string,
symbolPath: string,
newBody: string,
options?: unknown,
): unknown {
const native = getNative();
if (!native) return null;
return native.replaceSymbol(filePath, symbolPath, newBody, options);
}
/**
* Insert code around a symbol declaration (Rust-backed).
*
* Purpose: make symbol-adjacent insertions available to SF tools without
* duplicating parser/edit logic in TypeScript.
*
* Consumer: SF edit executors for native symbol-aware insertions.
*
* Falls back to null when the native module is unavailable.
*/
export function nativeInsertAroundSymbol(
filePath: string,
symbolPath: string,
position: unknown,
code: string,
options?: unknown,
): unknown {
const native = getNative();
if (!native) return null;
return native.insertAroundSymbol(
filePath,
symbolPath,
position,
code,
options,
);
}

View file

@ -2,7 +2,7 @@ import assert from "node:assert/strict";
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, test } from 'vitest';
import { afterEach, test } from "vitest";
import { checkEngineHealth } from "../doctor-engine-checks.ts";
import { filterDoctorIssues } from "../doctor-format.ts";
import { closeDatabase } from "../sf-db.ts";
@ -49,7 +49,7 @@ test("filterDoctorIssues keeps project and environment issues in scoped reports"
);
});
test("checkEngineHealth reports db_unavailable when sf.db exists but the DB is closed", async (t) => {
test("checkEngineHealth reports db_unavailable when sf.db exists but the DB is closed", async () => {
const base = mkdtempSync(join(tmpdir(), "sf-doctor-db-unavailable-"));
afterEach(() => rmSync(base, { recursive: true, force: true }));
@ -58,7 +58,7 @@ test("checkEngineHealth reports db_unavailable when sf.db exists but the DB is c
writeFileSync(join(sfDir, "sf.db"), "");
const issues: any[] = [];
await checkEngineHealth(base, issues, []);
await checkEngineHealth(base, issues, [], () => false);
const dbIssue = issues.find((issue) => issue.code === "db_unavailable");
assert.ok(

View file

@ -1,5 +1,5 @@
import assert from "node:assert/strict";
import { describe, test } from 'vitest';
import { describe, test } from "vitest";
/**
* doctor-git.test.ts Integration tests for doctor git health checks.
@ -1039,6 +1039,59 @@ describe("doctor-git", async () => {
"old commit with clean tree NOT flagged as stale",
);
});
// ─── Test: orphaned_milestone_directory detection & fix ──────────────
test("orphaned_milestone_directory", async () => {
const dir = createRepoWithActiveMilestone();
cleanups.push(dir);
// Create an orphaned milestone directory (no DB row, no roadmap)
const orphanDir = join(dir, ".sf", "milestones", "orphaned-junk");
mkdirSync(orphanDir, { recursive: true });
const detect = await runSFDoctor(dir);
const orphanIssues = detect.issues.filter(
(i) => i.code === "orphaned_milestone_directory",
);
assert.ok(
orphanIssues.length > 0,
"detects orphaned milestone directory",
);
assert.ok(
orphanIssues[0]?.message.includes("orphaned-junk"),
"message includes the orphaned directory name",
);
assert.ok(
orphanIssues[0]?.fixable === true,
"orphaned_milestone_directory is fixable",
);
const fixed = await runSFDoctor(dir, { fix: true });
assert.ok(
fixed.fixesApplied.some((f) =>
f.includes("removed orphaned milestone directory"),
),
"fix removes orphaned milestone directory",
);
assert.ok(!existsSync(orphanDir), "orphaned directory removed after fix");
});
// ─── Test: valid milestone directory NOT flagged as orphaned ─────────
test("orphaned_milestone_directory (no false positive)", async () => {
const dir = createRepoWithActiveMilestone();
cleanups.push(dir);
// M001 already has a roadmap and is in the filesystem registry
const detect = await runSFDoctor(dir);
const orphanIssues = detect.issues.filter(
(i) => i.code === "orphaned_milestone_directory",
);
assert.deepStrictEqual(
orphanIssues.length,
0,
"valid milestone directory NOT flagged as orphaned",
);
});
} finally {
for (const dir of cleanups) {
try {