diff --git a/docs/records/2026-05-02-bug-hunt-findings.md b/docs/records/2026-05-02-bug-hunt-findings.md index be5b0bd80..961eda933 100644 --- a/docs/records/2026-05-02-bug-hunt-findings.md +++ b/docs/records/2026-05-02-bug-hunt-findings.md @@ -1,3 +1,11 @@ +--- +actionable: true +kind: bug-hunt +date: 2026-05-02 +promoted: true +promoted_to: M008 +--- + # Bug Hunt Findings — 2026-05-02 Read-only audit across 6 clusters of /src/resources/extensions/sf/. 55 total diff --git a/src/resources/extensions/sf/journal.ts b/src/resources/extensions/sf/journal.ts index 354fd46af..31846c674 100644 --- a/src/resources/extensions/sf/journal.ts +++ b/src/resources/extensions/sf/journal.ts @@ -111,7 +111,7 @@ export function emitJournalEvent(basePath: string, entry: JournalEntry): void { // See auto/turn-epoch.ts for the full rationale. if (isStaleWrite("journal")) return; try { - const journalDir = join(sfRoot(basePath), "journal"); + const journalDir = join(sfRuntimeRoot(basePath), "journal"); mkdirSync(journalDir, { recursive: true }); const dateStr = entry.ts.slice(0, 10); const filePath = join(journalDir, `${dateStr}.jsonl`); @@ -171,7 +171,7 @@ export function queryJournal( filters?: JournalQueryFilters, ): JournalEntry[] { try { - const journalDir = join(sfRoot(basePath), "journal"); + const journalDir = join(sfRuntimeRoot(basePath), "journal"); const files = readdirSync(journalDir) .filter((f) => f.endsWith(".jsonl")) .sort(); diff --git a/src/resources/extensions/sf/memory-store.ts b/src/resources/extensions/sf/memory-store.ts index 865e37477..a57e5fe2e 100644 --- a/src/resources/extensions/sf/memory-store.ts +++ b/src/resources/extensions/sf/memory-store.ts @@ -204,6 +204,12 @@ export function createMemory(fields: { const seq = row["seq"] as number; const realId = `MEM${String(seq).padStart(3, "0")}`; rewriteMemoryId(placeholder, realId); + // Verify the rename actually landed — return null on miss so callers + // are not handed a stale _TMP_... placeholder as if it were a real ID. + const verify = adapter + .prepare("SELECT 1 FROM memories WHERE id = :id") + .get({ ":id": realId }); + if (!verify) return null; return realId; } catch { return null; diff --git a/src/resources/extensions/sf/pre-execution-checks.ts b/src/resources/extensions/sf/pre-execution-checks.ts index 0cf715a30..c5a5a0292 100644 --- a/src/resources/extensions/sf/pre-execution-checks.ts +++ b/src/resources/extensions/sf/pre-execution-checks.ts @@ -266,6 +266,11 @@ export async function checkPackageExistence( * - Resolves redundant segments (e.g., foo/../bar → bar) * * This ensures that "./src/a.ts", "src/a.ts", and "src//a.ts" all compare equal. + * + * @returns A normalized string suitable for equality comparisons only. + * Do NOT use the return value directly in filesystem operations (open, stat, + * readFile, etc.) without re-validating or re-resolving against an absolute + * base path — the output is not guaranteed to be a valid, safe filesystem path. */ export function normalizeFilePath(filePath: string): string { if (!filePath) return filePath; diff --git a/src/resources/extensions/sf/preferences-migrations.ts b/src/resources/extensions/sf/preferences-migrations.ts index 2d0d155eb..e0cb60776 100644 --- a/src/resources/extensions/sf/preferences-migrations.ts +++ b/src/resources/extensions/sf/preferences-migrations.ts @@ -48,6 +48,9 @@ export interface Migration { */ export const MIGRATIONS: ReadonlyArray = []; +/** + * Result of applying forward migrations to preferences. + */ export interface MigrationOutcome { preferences: SFPreferences; /** Migrations applied in order. Empty if no upgrade was needed. */ diff --git a/src/resources/extensions/sf/tests/auto-prompts.test.ts b/src/resources/extensions/sf/tests/auto-prompts.test.ts new file mode 100644 index 000000000..c1e9b35c7 --- /dev/null +++ b/src/resources/extensions/sf/tests/auto-prompts.test.ts @@ -0,0 +1,177 @@ +/** + * Unit tests for auto-prompts pure helpers: + * - extractMarkdownSection: H2-section extraction from markdown + * - buildSliceSummaryExcerpt: structured excerpt from a slice SUMMARY file + * + * Both functions are pure-ish (no DB, no network); tests write temp files + * only when buildSliceSummaryExcerpt needs an absPath on disk. + */ + +import assert from "node:assert/strict"; +import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test from "node:test"; +import { + buildSliceSummaryExcerpt, + extractMarkdownSection, +} from "../auto-prompts.ts"; + +// ─── extractMarkdownSection ────────────────────────────────────────────────── + +test("extractMarkdownSection: returns body of a matching H2 section", () => { + const content = `# Title\n\n## Diagnostics\nsome diag text\n\n## Other\nother text\n`; + const result = extractMarkdownSection(content, "Diagnostics"); + assert.equal(result, "some diag text"); +}); + +test("extractMarkdownSection: returns null when section is absent", () => { + const content = `## Present\ncontent here\n`; + assert.equal(extractMarkdownSection(content, "Missing"), null); +}); + +test("extractMarkdownSection: stops at the next H2 heading", () => { + const content = `## Alpha\nalpha body\n\n## Beta\nbeta body\n`; + const result = extractMarkdownSection(content, "Alpha"); + assert.equal(result, "alpha body"); + assert.ok(!result?.includes("beta"), "must not bleed into Beta section"); +}); + +test("extractMarkdownSection: returns trimmed content (no leading/trailing whitespace)", () => { + const content = `## Verification\n\n trimmed text \n\n`; + const result = extractMarkdownSection(content, "Verification"); + assert.equal(result, "trimmed text"); +}); + +test("extractMarkdownSection: last section (no following H2) returns to EOF", () => { + const content = `## Final\nfinal content here\n`; + const result = extractMarkdownSection(content, "Final"); + assert.equal(result, "final content here"); +}); + +test("extractMarkdownSection: heading match is case-sensitive", () => { + const content = `## diagnostics\nlower case heading\n`; + // Must NOT match uppercase "Diagnostics" + assert.equal(extractMarkdownSection(content, "Diagnostics"), null); + assert.equal(extractMarkdownSection(content, "diagnostics"), "lower case heading"); +}); + +test("extractMarkdownSection: heading with special regex chars is handled safely", () => { + // Heading contains . and * which would break an unescaped regex + const content = `## node_modules (*.js)\ncontent\n`; + const result = extractMarkdownSection(content, "node_modules (*.js)"); + assert.equal(result, "content"); +}); + +// ─── buildSliceSummaryExcerpt ───────────────────────────────────────────────── + +function makeTmpDir(prefix: string): string { + const dir = join(tmpdir(), `${prefix}-${Date.now()}-${Math.random().toString(36).slice(2)}`); + mkdirSync(dir, { recursive: true }); + return dir; +} + +const MINIMAL_SUMMARY = `--- +id: M001/S01/T03 +parent: M001/S01 +milestone: M001 +provides: [] +requires: [] +affects: [] +key_files: [] +key_decisions: [] +patterns_established: [] +drill_down_paths: [] +observability_surfaces: [] +duration: 3m +verification_result: pass +completed_at: "2026-01-01T00:00:00Z" +blocker_discovered: false +--- + +# Add Authentication Module + +**Implemented JWT-based authentication with refresh tokens.** + +## What Happened +Wired up the auth middleware. + +## Deviations +None. + +## Follow-ups +Write integration tests. +`; + +test("buildSliceSummaryExcerpt: happy path — returns header and structured fields", async () => { + const tmp = makeTmpDir("ap-excerpt-happy"); + try { + const filePath = join(tmp, "T03-SUMMARY.md"); + writeFileSync(filePath, MINIMAL_SUMMARY, "utf-8"); + + const result = await buildSliceSummaryExcerpt(filePath, ".sf/milestones/M001/slices/S01/T03-SUMMARY.md", "S01/T03"); + + assert.ok(result.includes("S01/T03 Summary (excerpt)"), "header must include sid"); + assert.ok(result.includes("Add Authentication Module"), "title must appear"); + assert.ok(result.includes("**Verification:** `pass`"), "verification result must appear"); + assert.ok(result.includes("**Blockers:** none"), "blockers line must appear"); + assert.ok(result.includes("**Duration:** 3m"), "duration must appear"); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } +}); + +test("buildSliceSummaryExcerpt: missing file returns not-found notice", async () => { + const result = await buildSliceSummaryExcerpt( + "/nonexistent/path/T03-SUMMARY.md", + ".sf/milestones/M001/slices/S01/T03-SUMMARY.md", + "S01/T03", + ); + + assert.ok(result.includes("not found"), "must mention not found for missing file"); + assert.ok(result.includes("S01/T03"), "header must still include sid"); +}); + +test("buildSliceSummaryExcerpt: null absPath returns not-found notice", async () => { + const result = await buildSliceSummaryExcerpt( + null, + ".sf/milestones/M001/slices/S01/T03-SUMMARY.md", + "S01/T03", + ); + + assert.ok(result.includes("not found"), "null absPath must yield not-found notice"); +}); + +test("buildSliceSummaryExcerpt: file without frontmatter id falls back to full inline", async () => { + const tmp = makeTmpDir("ap-excerpt-nofm"); + try { + const filePath = join(tmp, "T01-SUMMARY.md"); + // No frontmatter at all — parseSummary returns empty id + writeFileSync(filePath, "# Raw Summary\n\nSome unstructured text.", "utf-8"); + + const result = await buildSliceSummaryExcerpt(filePath, ".sf/m/s/T01-SUMMARY.md", "S01/T01"); + + // Fallback: full content inlined, no excerpt header + assert.ok(result.includes("Some unstructured text"), "must inline full content on fallback"); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } +}); + +test("buildSliceSummaryExcerpt: blocker_discovered=true surfaces warning text", async () => { + const tmp = makeTmpDir("ap-excerpt-blocker"); + try { + const summaryWithBlocker = MINIMAL_SUMMARY.replace( + "blocker_discovered: false", + "blocker_discovered: true", + ); + const filePath = join(tmp, "T04-SUMMARY.md"); + writeFileSync(filePath, summaryWithBlocker, "utf-8"); + + const result = await buildSliceSummaryExcerpt(filePath, ".sf/m/s/T04-SUMMARY.md", "S01/T04"); + + assert.ok(result.includes("⚠️ blocker recorded"), "blocker warning must appear"); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } +}); diff --git a/src/resources/extensions/sf/token-counter.ts b/src/resources/extensions/sf/token-counter.ts index fa6a672c2..1e76aab45 100644 --- a/src/resources/extensions/sf/token-counter.ts +++ b/src/resources/extensions/sf/token-counter.ts @@ -174,6 +174,9 @@ export function isGoogleGeminiCountablePayload( ); } +/** + * Count tokens in a Google Gemini CLI request using their server API. + */ export async function countGoogleGeminiCliTokens( payload: unknown, apiKeyRaw: string | undefined, diff --git a/src/resources/extensions/sf/workflow-logger.ts b/src/resources/extensions/sf/workflow-logger.ts index c75583a83..94fd37aa9 100644 --- a/src/resources/extensions/sf/workflow-logger.ts +++ b/src/resources/extensions/sf/workflow-logger.ts @@ -325,6 +325,8 @@ function _push( ); } catch (auditEmitErr) { // Best-effort: unified audit projection must never block workflow logger. + // Increment failure counter so doctor/status can surface persistent divergence. + _auditEmitFailureCount++; _writeStderr( `[sf:workflow-logger] unified-audit emit failed: ${(auditEmitErr as Error).message}\n`, ); diff --git a/src/resources/extensions/sf/workflow-templates.ts b/src/resources/extensions/sf/workflow-templates.ts index d9850d018..641bbf268 100644 --- a/src/resources/extensions/sf/workflow-templates.ts +++ b/src/resources/extensions/sf/workflow-templates.ts @@ -85,6 +85,10 @@ export interface MilestoneTemplateSliceScaffold { observabilityImpact: string; } +/** + * A command definition for /sf start completion and help text. + * Contains command name and description from template registry. + */ export interface WorkflowTemplateCommandDefinition { cmd: string; desc: string; diff --git a/src/resources/extensions/sf/worktree-manager.ts b/src/resources/extensions/sf/worktree-manager.ts index 9ae5d8e8d..1a723d669 100644 --- a/src/resources/extensions/sf/worktree-manager.ts +++ b/src/resources/extensions/sf/worktree-manager.ts @@ -34,6 +34,7 @@ import { SF_STALE_STATE, SFError, } from "./errors.js"; +import { SF_RUNTIME_PATTERNS } from "./gitignore.js"; import { nativeBranchDelete, nativeBranchExists,