---
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
findings: 15 HIGH, 22 MEDIUM, 18 LOW.

## Methodology

6 haiku-powered read-only agents inspected files in parallel, instructed to
report only verified bugs (no hallucination). Each agent constrained to its
cluster, no edits, no test runs.

## Status legend

- 🔴 HIGH: data loss / crash / security / definite wrong
- 🟡 MEDIUM: probable bug under specific conditions
- 🟢 LOW: robustness / style

---

## Cluster: engine + verification

### HIGH

- `verification-gate.ts:451` [async/concurrency] **Status: FALSE-POSITIVE**
  `checkAsyncStyleDrift()` returns `passed: true` when style drift is detected. The agent flagged this as wrong, but per the user's own verification: `// Warning only` comments are intentional — the field is correct as-is.
  Suggested fix: N/A (intentional design).

- `post-execution-checks.ts:452` [async/concurrency] **Status: FALSE-POSITIVE**
  Same pattern: `checkAsyncStyleDrift()` flags style drift but returns `passed: true`. The `// Warning only` comment confirms this is intentional.
  Suggested fix: N/A (intentional design).

- `production-mutation-approval.ts:268` [inverted condition] **Status: FALSE-POSITIVE**
  Agent flagged that `data.risk` is accessed without existence check. Per user verification, `data.risk !== "expected"` correctly flags missing fields — the inequality on undefined already signals the missing-field case.
  Suggested fix: N/A (intentional design).

### MEDIUM

- `verification-gate.ts:134-150` [resource efficiency] **Status: CONFIRMED**
  `formatFailureContext()` accumulates per-check stderr up to 2000 chars each, then truncates the whole output at 10 000 chars. With 5 failing checks the tail is silently cut mid-line, losing diagnostics from later checks.
  Suggested fix: truncate per-check to `floor(10000 / failCount)` chars so each check gets a fair share, or emit a "…N chars omitted" sentinel.

- `pre-execution-checks.ts:73-106` [logic] **Status: CONFIRMED**
  `extractPackageReferences()` silently drops package names that collide with the stopwords set. `npm install -D test` is a concrete example: the string `"test"` is a stopword so it is not captured even when the intent is the `test` package.
  Suggested fix: scope the stopword check to bare flag-tokens (tokens immediately after a flag like `-D`), not to all positional tokens.

- `post-execution-checks.ts:356-366` [logic] **Status: CONFIRMED**
  `checkCrossTaskSignatures()` compares against only `priorDefs[0]`. If the same function was defined with different signatures across multiple prior tasks, only the first definition is checked, missing signature drift chains.
  Suggested fix: iterate all prior definitions and report any that diverge from the current.

- `dispatch-guard.ts:104-150` [logic] **Status: CONFIRMED**
  When any slice in a milestone declares explicit dependencies, the positional-fallback is skipped globally, not just for that slice. Slices with no declared dependencies can therefore be dispatched out of order.
  Suggested fix: apply the `null` short-circuit only to the target slice, not to the entire milestone pass.

### LOW

- `verification-gate.ts:240-271` [robustness] **Status: CONFIRMED**
  `isLikelyCommand()` heuristics are gameable (e.g., short prose sentences with package-name-like words). Safe because `sanitizeCommand()` adds a second check, but the heuristics produce noisy misclassifications.
  Suggested fix: add a minimum word-count guard and explicit prose-article detection.

- `gate-registry.ts:185-186` [style] **Status: CONFIRMED**
  `ORDERED_GATES` uses `Object.values()` without a comment noting the ES2020 insertion-order guarantee. Pre-ES2020 engines (or linters that warn about object-key ordering) may flag this.
  Suggested fix: add an explicit ordering comment or use an explicit array literal.

- `custom-verification.ts:167-172` [robustness] **Status: CONFIRMED**
  Shell command guard rejects `$(`, backtick, `;rm`, `;curl`, etc., but not `&` or `&&`. `ls && rm -rf /` passes the dangerous-pattern check.
  Suggested fix: add `&&` and `||` operator patterns to the rejection list.

- `pre-execution-checks.ts:265-287` [robustness] **Status: CONFIRMED**
  `normalizeFilePath()` strips `./`, normalises separators, and deduplicates segments. The normalised output is not filesystem-safe if used directly in a subsequent path operation without re-validation.
  Suggested fix: add a JSDoc note that the return value is for comparison only, not disk I/O.

---

## Cluster: scaffold + doctor

### HIGH

- `doctor-environment.ts:404` [logic error] **Status: CONFIRMED**
  Port-conflict detection parses `lsof` output with a regex whose capture group `[2]` reliably returns empty or garbage for process names. PID extraction (line 409) is correct, but the process-name field shown to users is unreliable.
  Suggested fix: Use `lsof -F cn` (field output mode) or parse the `COMMAND` column by position rather than regex group index.

- `doctor-runtime-checks.ts:394` [redundant dynamic import] **Status: FIXED**
  `checkRuntimeHealth` re-imports `detectScaffoldDrift` dynamically inside the try-block even though a static import already exists at line 27. The fix-swarm agent removed the dynamic import and confirmed the static import suffices.
  Suggested fix: Already applied by fix swarm (task `a1ba9d693050d27a1`).

### MEDIUM

- `doctor-runtime-checks.ts:737-774` [code duplication] **Status: CONFIRMED**
  `checkScaffoldFreshness` (exported) duplicates drift-check logic already in `checkRuntimeHealth`. Per fix-swarm review, the two functions serve different call sites, so they are not identical — but the shared logic still diverges risk.
  Suggested fix: Extract common bucket-count message logic into a private helper.

- `doctor-environment.ts:375` [regex over-matching] **Status: CONFIRMED**
  Port-detection regex can match colons in IPv6 addresses or other non-port contexts. The 1024–65535 range filter reduces false positives but the pattern itself is imprecise.
  Suggested fix: Anchor the alternation more tightly (require preceding whitespace or `=` for the `PORT=` variant).

- `scaffold-drift.ts:221` [classification inconsistency] **Status: CONFIRMED**
  A file with `pending` state, matching hash, and current version is labelled `customized` rather than something like `no_action_needed`. This misleads users into thinking they made edits when they did not. Per fix-swarm, the bucket assignment is by design for the current spec, so no code change was made — but the label is confusing.
  Suggested fix: rename the bucket or add a sub-label distinguishing user edits from version-matched pending files.

### LOW

- `scaffold-versioning.ts:249` [style] **Status: CONFIRMED**
  Four consecutive `(e as ScaffoldManifestEntry)` casts in one filter predicate. Repetitive and hides the intent.
  Suggested fix: Extract a `isScaffoldManifestEntry(e): e is ScaffoldManifestEntry` type guard.

- `doctor.ts:1119` [misleading variable] **Status: CONFIRMED**
  `sfState` timing is computed as `now - t0env - envMs` rather than being independently measured, making the value approximate and confusingly named.
  Suggested fix: Add `const t0state = Date.now()` before state derivation; compute `sfState = Date.now() - t0state` explicitly.

- `doctor-environment.ts:803` [style] **Status: CONFIRMED**
  Hard-coded emoji glyphs (✅, ⚠️, 🛑) in `formatEnvironmentReport` may not render on all terminals.
  Suggested fix: Guard behind a `process.env.NO_COLOR` or `isCI()` check, or make the icons configurable.

---

## Cluster: worktree + git

### HIGH

- `worktree-resolver.ts:616` [merge-state-cleanup] **Status: CONFIRMED**
  Cleanup of `SQUASH_MSG`/`MERGE_HEAD` after a failed merge uses `join(originalBase, ".git")`. In a git worktree the `.git` entry is a pointer file, not a directory; the real merge state lives in `.git/worktrees/<name>/`. The cleanup silently fails, leaving stale merge state that blocks subsequent merges.
  Suggested fix: Use the existing `resolveGitDir(originalBase || this.s.basePath)` helper (available in `worktree-manager.ts:106`) to resolve the actual git directory before attempting cleanup.

- `worktree-resolver.ts:228` [basePath-mutation-race] **Status: CONFIRMED**
  `emitWorktreeCreated` is called after `this.s.basePath` is mutated to the new worktree path. When `originalBasePath` is unset, the emit receives the new worktree path as `projectRoot`, breaking telemetry that requires a stable project root.
  Suggested fix: Save the original project root into a local `const` before mutating `basePath` at line 206, then always pass the saved value to emit calls.

### MEDIUM

- `worktree-manager.ts:572-574` [cwd-resolution-race] **Status: CONFIRMED**
  Directory-existence check and `chdir` are not atomic. A concurrent process could enter the worktree between the two calls, causing `git worktree remove` to fail. Damage is limited (caller retries), but the TOCTOU window is undocumented.
  Suggested fix: Add a comment noting the race window; wrap in try-catch with a targeted retry.

- `worktree-command.ts:757-762` [state-corruption-on-crash] **Status: CONFIRMED**
  `originalCwd` is cleared at line 761 before merge completion. A crash or hang between the chdir and merge completion leaves the session unable to detect it was inside a worktree on restart.
  Suggested fix: Move the `originalCwd = null` clear into the success path (after line 792); add a recovery step in `registerWorktreeCommand` to detect orphaned worktrees on reload.

- `worktree-health.ts:76` [symlink-follow] **Status: CONFIRMED**
  `existsSync(wt.path)` does not distinguish a deleted symlink target from an absent directory. A manually deleted symlink target causes the health check to report `not exists` even though git still records the worktree.
  Suggested fix: Explicitly check `lstatSync` to detect symlinks; return a `{ exists: false }` health marker when the target is missing.

- `worktree-resolver.ts:616` [path-separator] **Status: CONFIRMED**
  `join()` on Windows paths with backslashes mixed with forward-slash comparisons elsewhere. Low risk due to Node.js normalisation, but adds cognitive load and latent risk on non-Linux targets.
  Suggested fix: Use `path.resolve` consistently and normalise separators to forward-slash for all comparisons.

### LOW

- `worktree-command.ts:72-76` [path-extraction-fragile] **Status: CONFIRMED**
  Worktree name extracted via sequential `split("/")[0] ?? split("\\")[0]`. Trailing backslashes can yield an empty array.
  Suggested fix: Normalise path separators with `path.normalize` before splitting, or use `path.basename`.

- `gitignore.ts:43` [manual-sync drift] **Status: CONFIRMED**
  `SF_RUNTIME_PATTERNS` is not imported in `worktree-manager.ts`; the `SKIP_PATHS` array is manually maintained and can drift. The comment says "must stay synchronized" but there is no automated enforcement.
  Suggested fix: Export `SKIP_PATHS` from `gitignore.ts` and import it in `worktree-manager.ts`, or add a runtime assertion.

- `worktree-resolver.ts:252,373,597` [stale path reference] **Status: CONFIRMED**
  All three emit calls use `this.s.originalBasePath || this.s.basePath`, which may reference a stale value after `restoreToProjectRoot()` resets `basePath`.
  Suggested fix: Capture `const projectRoot = this.s.originalBasePath ?? this.s.basePath` at function entry and use it consistently.

---

## Cluster: memory + state + cache

### HIGH

- `json-persistence.ts:58-69` [atomicity] **Status: CONFIRMED**
  `saveJsonFile()` does not `fsync` after `renameSync()`. A system crash immediately after the rename can lose the atomic-write guarantee despite the tmp-file pattern.
  Suggested fix: Call `fs.fsyncSync(fd)` on the target file (and optionally the parent directory) before returning.

- `atomic-write.ts:56` [deadlock risk] **Status: CONFIRMED**
  `sleepSync()` uses `Atomics.wait()` on a `SharedArrayBuffer`, which blocks the event loop. In a single-threaded Node context this can freeze the process or deadlock if called during async I/O.
  Suggested fix: Use `setTimeout`-based sleep or document explicitly that this function must only be called from Worker threads.

- `json-persistence.ts:58,75` [race condition] **Status: CONFIRMED**
  Both `saveJsonFile()` and `writeJsonFileAtomic()` use randomised tmp names but do not clean up orphaned `.tmp.*` files from prior crashed writes before starting a new write. On Windows a locked stale tmp file can cause `renameSync` to fail.
  Suggested fix: Glob and delete `*.tmp.*` files for the target path at the start of each write operation.

### MEDIUM

- `memory-extractor.ts:93-94` [resource leak] **Status: CONFIRMED**
  The API key is resolved once in the outer closure and cached. If the key expires or auth revokes mid-extraction, all subsequent calls silently use stale credentials.
  Suggested fix: Resolve `apiKey` inside the async function body on each invocation, not in the closure.

- `cache.ts:24-29` [incomplete invalidation] **Status: CONFIRMED**
  `invalidateAllCaches()` calls four functions sequentially without error isolation. If one throws, the remainder are skipped, leaving a partially-stale cache state.
  Suggested fix: Wrap each call in an independent try-catch; log failures and continue clearing the remaining caches.

- `memory-store.ts:203` [stale data fallback] **Status: CONFIRMED**
  When `rewriteMemoryId()` fails to find the placeholder row, it returns the placeholder ID instead of `null`. Downstream code then treats `_TMP_...` as a real memory ID, causing silent state divergence.
  Suggested fix: Return `null` explicitly on SELECT failure; let the caller decide whether to retry or raise.

- `atomic-write.ts:122-137` [error swallowing] **Status: CONFIRMED**
  The rename-retry loop overwrites `lastError` on each iteration. The final error may differ from earlier ones, making retry diagnostics misleading.
  Suggested fix: Accumulate errors in an array and include all of them in the thrown error message.

### LOW

- `context-injector.ts:82-89` [truncation signal] **Status: CONFIRMED**
  Truncated artifacts get a plain `\n...[truncated]` suffix. If the artifact is JSON, the suffix breaks its structure for downstream parsers.
  Suggested fix: Document the truncation strategy; or wrap JSON artifacts in a container object before truncation.

- `definition-io.ts:14-18` [no error context] **Status: CONFIRMED**
  `readFrozenDefinition()` propagates raw YAML parse errors without identifying the offending file path.
  Suggested fix: Wrap in try-catch and prepend `defPath` to the error message.

- `memory-sleeper.ts:38-42` [memory leak] **Status: CONFIRMED**
  `seenKeys` Set accumulates entries across units indefinitely. Identical tool-failure keys from different units are silently deduplicated even when the context has changed.
  Suggested fix: Reset `seenKeys` at unit boundaries or per invocation.

---

## Cluster: bootstrap + workflow

### HIGH

- `workflow-reconcile.ts:544` [async/concurrency] **Status: CONFIRMED**
  Reconciliation reads `mainLogPath` at step 2 and re-checks before writing, but the advisory lock (line 469) is not held across the entire read-check-write window. Concurrent reconcilers can interleave log entries and cause divergence.
  Suggested fix: Acquire the lock before the initial read at step 1, not just before the write.

- `workflow-events.ts:81` [silent failure] **Status: CONFIRMED**
  `readEvents()` silently skips corrupted JSONL lines with only `logWarning()`. Systematic corruption (e.g., from a crash mid-write) makes a milestone's event log silently incomplete.
  Suggested fix: Track corruption count; fail (or prompt recovery from manifest backup) when the ratio exceeds a threshold.

- `workflow-reconcile.ts:164-175` [data loss] **Status: CONFIRMED**
  `replaySliceComplete()` validates task closure but not the milestone guard. A slice can be marked done even with incomplete tasks; on subsequent reconciliation error the incomplete work is orphaned.
  Suggested fix: Validate milestone-level constraints before accepting a `complete_slice` event.

### MEDIUM

- `workflow-logger.ts:305-328` [audit divergence] **Status: CONFIRMED**
  If `emitUokAuditEvent` fails, the error is caught and written to stderr, but the log entry is still persisted at line 346. The audit log and unified audit can permanently diverge with no reconciliation path.
  Suggested fix: Track audit-emit failures and surface them in the next doctor run or workflow status report.

- `write-gate.ts:277-279` [edge case] **Status: CONFIRMED**
  `extractDepthVerificationMilestoneId()` regex matches only the specific pattern `depth_verification_M001`. Non-standard milestone ID formats silently return `null`, causing gate confirmation to fail unexpectedly.
  Suggested fix: Normalise milestone IDs to lowercase before matching, or broaden the regex to accept common variants.

- `workflow-manifest.ts:77-222` [type safety] **Status: CONFIRMED**
  `snapshotState()` calls `JSON.parse()` on `depends_on` columns without schema validation. Malformed but parseable JSON (e.g., escaped-quote corruption) returns wrong structure that passes the type-coercion guard.
  Suggested fix: Add a runtime schema validator (e.g., Zod or a manual array-of-strings check) after parse.

- `notify-interceptor.ts:40-43` [idempotency] **Status: CONFIRMED**
  When `appendNotification()` throws and is caught, the original notify still proceeds. Callers cannot distinguish a persisted notification from a failed one.
  Suggested fix: Log the persistence failure with a correlation ID; surface it in the notification status API.

- `system-context.ts:620-644` [async/concurrency] **Status: CONFIRMED**
  `buildCarryForwardLines()` does not handle the case where a task file is deleted between directory scan and file load. A rejection in `Promise.all` fails the entire context injection with no fallback.
  Suggested fix: Wrap per-file loads in individual try-catch blocks; skip missing files and log a warning.

### LOW

- `system-context.ts:774-776` [robustness] **Status: CONFIRMED**
  Forensics-marker expiry fires silently after 2 hours. Users returning to a paused forensics session find the marker cleared with no log entry explaining why.
  Suggested fix: Log an expiry event at `INFO` level when the timer fires.

- Former workflow transport helper (now `workflow-tools.js`) [edge case] **Status: CONFIRMED**
  Error message for the old local-transport path suggested `/sf mcp init`, which did not resolve the structured-questions limitation for local transports.
  Suggested fix: Keep workflow questions on native SF structured-question support; MCP remains client-only for external tools.

- `workflow-reconcile.ts:752-759` [idempotency] **Status: CONFIRMED**
  After resolving the last conflict, `resolveConflict()` calls `reconcileWorktreeLogs()` a second time (line 758). If this second reconcile fails, the conflict is left in a partially-applied state with no rollback.
  Suggested fix: Move the second reconcile into the explicit success path, or add rollback logic on failure.

---

## Cluster: notification + detection + headless

### HIGH

- `src/headless.ts:1677-1685` [exit code mapping] **Status: CONFIRMED**
  `EXIT_RELOAD` has no case in the `mapStatusToExitCode()` switch (headless-events.ts:33-48) and falls through to `EXIT_ERROR`, corrupting the reload-sentinel feature. Additionally, `emitBatchJsonResult()` sets `status: "timeout"` when `totalEvents === 0`, which silently misclassifies genuine timeouts that did emit events.
  Suggested fix: Add an explicit `case EXIT_RELOAD:` in `mapStatusToExitCode()`; separate the timeout-detection logic from the event-count heuristic.

- `src/resources/extensions/sf/notification-store.ts:109` [dedup off-by-one] **Status: CONFIRMED**
  The deduplication window uses strict `<` instead of `<=`. A second notification delivered exactly at `DEDUP_WINDOW_MS` milliseconds is not deduplicated, allowing boundary duplicates.
  Suggested fix: Change `< DEDUP_WINDOW_MS` to `<= DEDUP_WINDOW_MS`.

- `src/resources/extensions/sf/detection.ts:302-320` [root-only markers silent over-detection] **Status: CONFIRMED**
  `ROOT_ONLY_PROJECT_FILES` markers (e.g., `package.json`, `Cargo.toml`) are checked against the full recursive scan result rather than only root-level files. Nested workspace files are counted as top-level project markers, causing `verificationCommands` to emit bare `cargo check` / `npm test` from repo root, which fails when the only manifest is in a subdirectory.
  Suggested fix: Filter `scannedFiles` to only entries at depth 0 (relative path contains no `/`) before matching against `ROOT_ONLY_PROJECT_FILES`.

### MEDIUM

- `src/headless.ts:1013-1023` [idle timeout arm condition] **Status: CONFIRMED**
  `shouldArmHeadlessIdleTimeout()` re-arms immediately after a fast-returning interactive tool completes, potentially firing the timeout before the LLM reasons post-response.
  Suggested fix: Introduce a short grace period (e.g., 500 ms) before re-arming after an interactive tool completes.

- `src/headless.ts:1190` [milestone-ready detection in streaming] **Status: CONFIRMED**
  `isMilestoneReadyText()` is evaluated against incremental text deltas. If the pattern spans two deltas it will not match either fragment, potentially missing a milestone-ready signal.
  Suggested fix: Accumulate a rolling buffer of recent deltas (e.g., last 200 chars) and test the regex against the buffer.

- `src/resources/extensions/sf/notification-store.ts:347-348` [stale lock detection race] **Status: CONFIRMED**
  When the lock creator crashes after opening the fd but before writing the timestamp, `lockTime` parses as `NaN`. `Date.now() - NaN > 5000` evaluates to `false`, so the stale lock is never removed.
  Suggested fix: Treat `NaN` as stale (i.e., remove the lock if `isNaN(lockTime)`).

### LOW

- `src/headless.ts:348-356` [autonomous command chaining] **Status: CONFIRMED**
  The `autonomous` command took the wrong parser path, so autonomous chaining did not activate from the command name alone.
  Suggested fix: When the `autonomous` subcommand is parsed, implicitly set the autonomous chaining option.

- `src/headless.ts:1189` [autonomous chaining visibility] **Status: CONFIRMED**
  With verbose output enabled, log lines containing "milestone X ready" could spuriously match `isMilestoneReadyText()` and trigger premature autonomous chaining.
  Suggested fix: Restrict the detection to `message_delta` events of type `text` and exclude tool-output or log-prefixed content.

---

## Recommended next actions

Group by category, ordered by urgency.

### 1. Durability / Data-loss (fix first)

- `json-persistence.ts:58-69` — Missing `fsync` after atomic rename (memory+state+cache). A single line of code; fix immediately.
- `workflow-reconcile.ts:544` — Advisory lock not held across read-check-write in reconciler (bootstrap+workflow). Extend lock scope to cover the initial read.
- `workflow-events.ts:81` — Silent JSONL corruption in event logs (bootstrap+workflow). Add a corruption-rate threshold.

### 2. Crash / State-corruption

- `atomic-write.ts:56` — `Atomics.wait()` blocks event loop; deadlock risk (memory+state+cache).
- `worktree-resolver.ts:616` — Merge-state cleanup fails in worktrees; blocks subsequent merges (worktree+git).
- `worktree-command.ts:757-762` — `originalCwd` cleared before merge completes; session loses worktree context on crash (worktree+git).

### 3. Exit-code / Signal correctness

- `src/headless.ts:1677-1685` — `EXIT_RELOAD` falls through to `EXIT_ERROR` in switch (notification+detection+headless). High-visibility sentinel corruption; fix alongside the `totalEvents === 0` timeout heuristic.

### 4. Detection correctness

- `src/resources/extensions/sf/detection.ts:302-320` — Root-only markers checked against recursive scan, generating wrong `verificationCommands` (notification+detection+headless).
- `notification-store.ts:347-348` — `NaN` lock timestamp never treated as stale (notification+detection+headless).
- `notification-store.ts:109` — Off-by-one in dedup window (notification+detection+headless).

### 5. Logic / Guard correctness

- `dispatch-guard.ts:104-150` — Positional fallback skipped globally when any slice has dependencies (engine+verification).
- `workflow-reconcile.ts:164-175` — `replaySliceComplete()` skips milestone guard (bootstrap+workflow).
- `doctor-environment.ts:404` — lsof process-name extraction unreliable (scaffold+doctor).
