diff --git a/src/resources/extensions/sf/bootstrap/register-hooks.ts b/src/resources/extensions/sf/bootstrap/register-hooks.ts index b9e93fa47..726376461 100644 --- a/src/resources/extensions/sf/bootstrap/register-hooks.ts +++ b/src/resources/extensions/sf/bootstrap/register-hooks.ts @@ -639,9 +639,10 @@ export function registerHooks( pi.on("tool_execution_end", async (event) => { markToolEnd(event.toolCallId); - // #2883: Capture tool invocation errors (malformed/truncated JSON arguments) - // so postUnitPreVerification can break the retry loop instead of re-dispatching. - if (event.isError && event.toolName.startsWith("sf_")) { + // #2883/#4974: Capture deterministic invocation/policy errors so + // postUnitPreVerification can break the retry loop instead of re-dispatching. + // Covers sf_ tool JSON errors AND write-gate blocks on write/edit/bash tools. + if (event.isError) { const errorText = typeof event.result === "string" ? event.result diff --git a/src/resources/extensions/sf/deep-project-setup-policy.ts b/src/resources/extensions/sf/deep-project-setup-policy.ts index 3a556cd6a..addf0cee3 100644 --- a/src/resources/extensions/sf/deep-project-setup-policy.ts +++ b/src/resources/extensions/sf/deep-project-setup-policy.ts @@ -4,10 +4,8 @@ import { join } from "node:path"; import type { SFPreferences } from "./preferences-types.js"; import { clearParseCache } from "./files.js"; import { sfRoot, clearPathCache } from "./paths.js"; -// TODO: getProjectResearchStatus is not yet ported to SF — add project-research-policy.ts -// import { getProjectResearchStatus } from "./project-research-policy.js"; -// TODO: validateArtifact is not yet ported to SF — add schemas/validate.ts -// import { validateArtifact } from "./schemas/validate.js"; +import { getProjectResearchStatus } from "./project-research-policy.js"; +import { validateArtifact } from "./schemas/validate.js"; export type DeepProjectSetupStage = | "workflow-preferences" @@ -109,9 +107,7 @@ export function resolveDeepProjectSetupState( prefs: SFPreferences | undefined, basePath: string, ): DeepProjectSetupState { - // TODO: SF does not yet have planning_depth in SFPreferences — treat as always not-applicable - // until the field is added and project-research-policy.ts is ported. - if ((prefs as any)?.planning_depth !== "deep") { + if (prefs?.planning_depth !== "deep") { return { status: "not-applicable", stage: null, diff --git a/src/resources/extensions/sf/project-research-policy.ts b/src/resources/extensions/sf/project-research-policy.ts index 5e0e2edb7..97b9a1fe8 100644 --- a/src/resources/extensions/sf/project-research-policy.ts +++ b/src/resources/extensions/sf/project-research-policy.ts @@ -7,17 +7,8 @@ import { } from "./milestone-scope-classifier.js"; import { clearParseCache } from "./files.js"; import { sfRoot, clearPathCache } from "./paths.js"; -// TODO: port schemas/parsers.ts from gsd2 to SF — parseProject and parseRequirements are not yet available -// eslint-disable-next-line @typescript-eslint/no-explicit-any -type ParsedProject = any; -// eslint-disable-next-line @typescript-eslint/no-explicit-any -type ParsedRequirements = any; -function parseProject(_content: string): ParsedProject { - throw new Error("parseProject: schemas/parsers not yet ported to SF"); -} -function parseRequirements(_content: string): ParsedRequirements { - throw new Error("parseRequirements: schemas/parsers not yet ported to SF"); -} +import { parseProject, parseRequirements } from "./schemas/parsers.js"; +import type { ParsedProject, ParsedRequirements } from "./schemas/parsers.js"; export const PROJECT_RESEARCH_DIMENSIONS = ["STACK", "FEATURES", "ARCHITECTURE", "PITFALLS"] as const; export const PROJECT_RESEARCH_BLOCKER = "PROJECT-RESEARCH-BLOCKER.md"; diff --git a/src/resources/extensions/sf/tests/runtime-root-redirect.test.ts b/src/resources/extensions/sf/tests/runtime-root-redirect.test.ts new file mode 100644 index 000000000..cc4783e8a --- /dev/null +++ b/src/resources/extensions/sf/tests/runtime-root-redirect.test.ts @@ -0,0 +1,221 @@ +/** + * Tests for self-detection and sfRuntimeRoot path redirect. + * + * Verifies: + * - isRunningOnSelf returns true only when both signals are present + * (package.json name = "singularity-forge" AND loader.ts exists) + * - sfRuntimeRoot returns /.sf for non-self repos + * - sfRuntimeRoot returns sfHome (~/.sf or SF_HOME) for the self repo + * - Cache is isolated per basePath and reset correctly + */ + +import assert from "node:assert/strict"; +import { + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { homedir } from "node:os"; +import { join } from "node:path"; +import { afterEach, before, describe, test } from "node:test"; + +import { + _resetSelfDetectionCache, + isRunningOnSelf, + sfRuntimeRoot, +} from "../paths.ts"; + +// ── Helpers ────────────────────────────────────────────────────────────────── + +function createTmpDir(): string { + return mkdtempSync(join(homedir(), ".sf-test-")); +} + +/** Write a minimal package.json with the given name. */ +function writePackageJson(dir: string, name: string): void { + writeFileSync(join(dir, "package.json"), JSON.stringify({ name }), "utf-8"); +} + +/** Scaffold the loader.ts signal path. */ +function writeLoaderTs(dir: string): void { + const loaderDir = join(dir, "src", "resources", "extensions", "sf"); + mkdirSync(loaderDir, { recursive: true }); + writeFileSync(join(loaderDir, "loader.ts"), "// stub", "utf-8"); +} + +// ── Fixture Dirs ───────────────────────────────────────────────────────────── + +const tmpDirs: string[] = []; + +function tmpDir(): string { + const d = createTmpDir(); + tmpDirs.push(d); + return d; +} + +afterEach(() => { + _resetSelfDetectionCache(); +}); + +before(() => { + // Ensure cleanup even if tests are aborted + process.on("exit", () => { + for (const d of tmpDirs) { + try { + rmSync(d, { recursive: true, force: true }); + } catch { + /* ignore */ + } + } + }); +}); + +// ── isRunningOnSelf tests ───────────────────────────────────────────────────── + +describe("isRunningOnSelf", () => { + test("returns true when both signals are present", () => { + const dir = tmpDir(); + writePackageJson(dir, "singularity-forge"); + writeLoaderTs(dir); + assert.equal(isRunningOnSelf(dir), true); + }); + + test("returns false when only package.json signal is present (no loader.ts)", () => { + const dir = tmpDir(); + writePackageJson(dir, "singularity-forge"); + // No loader.ts + assert.equal(isRunningOnSelf(dir), false); + }); + + test("returns false when only loader.ts signal is present (wrong package name)", () => { + const dir = tmpDir(); + writePackageJson(dir, "some-other-project"); + writeLoaderTs(dir); + assert.equal(isRunningOnSelf(dir), false); + }); + + test("returns false when neither signal is present", () => { + const dir = tmpDir(); + assert.equal(isRunningOnSelf(dir), false); + }); + + test("returns false when package.json has no name field", () => { + const dir = tmpDir(); + writeFileSync(join(dir, "package.json"), JSON.stringify({ version: "1.0" }), "utf-8"); + writeLoaderTs(dir); + assert.equal(isRunningOnSelf(dir), false); + }); + + test("returns false when package.json is malformed JSON", () => { + const dir = tmpDir(); + writeFileSync(join(dir, "package.json"), "NOT JSON {{{", "utf-8"); + writeLoaderTs(dir); + // Detection failure must default to false + assert.equal(isRunningOnSelf(dir), false); + }); + + test("caches result and returns same value on second call", () => { + const dir = tmpDir(); + writePackageJson(dir, "singularity-forge"); + writeLoaderTs(dir); + const first = isRunningOnSelf(dir); + // Remove the signals to verify the cache is used + rmSync(join(dir, "package.json")); + const second = isRunningOnSelf(dir); + assert.equal(first, true); + assert.equal(second, true, "should return cached result even after signals removed"); + }); + + test("_resetSelfDetectionCache causes re-evaluation", () => { + const dir = tmpDir(); + writePackageJson(dir, "singularity-forge"); + writeLoaderTs(dir); + assert.equal(isRunningOnSelf(dir), true); + _resetSelfDetectionCache(); + // Remove the package.json so re-evaluation returns false + rmSync(join(dir, "package.json")); + assert.equal(isRunningOnSelf(dir), false); + }); +}); + +// ── sfRuntimeRoot tests ─────────────────────────────────────────────────────── + +describe("sfRuntimeRoot", () => { + test("returns /.sf when NOT running on self", () => { + const dir = tmpDir(); + writePackageJson(dir, "dr-repo"); + // No loader.ts — not self + const result = sfRuntimeRoot(dir); + assert.equal(result, join(dir, ".sf")); + }); + + test("returns sfHome when running on self (default SF_HOME)", () => { + const original = process.env.SF_HOME; + // Ensure SF_HOME is unset so we use the default ~/.sf + delete process.env.SF_HOME; + try { + const dir = tmpDir(); + writePackageJson(dir, "singularity-forge"); + writeLoaderTs(dir); + const expected = join(homedir(), ".sf"); + const result = sfRuntimeRoot(dir); + assert.equal(result, expected); + } finally { + if (original !== undefined) process.env.SF_HOME = original; + } + }); + + test("returns SF_HOME env value when running on self and SF_HOME is set", () => { + const original = process.env.SF_HOME; + const customHome = tmpDir(); + process.env.SF_HOME = customHome; + // Reset cache so the sfHome constant is not stale — note: sfHome is + // module-level, so we just verify the function respects it via normal flow. + // Since sfHome is computed once at module load, this test documents the + // behavior but SF_HOME must be set BEFORE module import for the constant + // to pick it up. Here we verify the contract in isolation. + try { + _resetSelfDetectionCache(); + // For the runtime root test, use a non-self dir to verify non-self path. + const nonSelfDir = tmpDir(); + writePackageJson(nonSelfDir, "other-project"); + const result = sfRuntimeRoot(nonSelfDir); + assert.equal(result, join(nonSelfDir, ".sf"), "non-self should still be /.sf regardless of SF_HOME"); + } finally { + if (original !== undefined) { + process.env.SF_HOME = original; + } else { + delete process.env.SF_HOME; + } + } + }); +}); + +// ── Tracked-artifact verification (source-level) ───────────────────────────── + +describe("tracked artifacts still use sfRoot (not sfRuntimeRoot)", () => { + test("milestones dir is resolved via sfRoot in paths.ts", () => { + // This test verifies the contract by importing milestonesDir and checking + // it doesn't point to the runtime root for a self repo. Since milestonesDir + // calls sfRoot() internally (which does a filesystem probe), and our test + // tmpDir has no .sf directory, sfRoot will return /.sf anyway — + // but the point is that milestonesDir DOES NOT call sfRuntimeRoot. + // We validate this at the source level: grep would show no sfRuntimeRoot in paths.ts + // for milestonesDir. Instead, we do a behavioral check: + // For a non-self dir, sfRoot(dir) === sfRuntimeRoot(dir), so any behavior + // difference shows up for self dirs. + const dir = tmpDir(); + writePackageJson(dir, "singularity-forge"); + writeLoaderTs(dir); + + // sfRuntimeRoot should return sfHome (not dir/.sf) + const runtimeRoot = sfRuntimeRoot(dir); + const expected = process.env.SF_HOME || join(homedir(), ".sf"); + assert.equal(runtimeRoot, expected, "sfRuntimeRoot → global ~./sf for self"); + + // The milestones dir is NOT imported here — we just document the expectation: + // tracked artifacts (milestones, PROJECT.md, etc.) use sfRoot() which + // returns /.sf, not ~./sf. + }); +});