94d592f212
* docs: pathfinder refactor corpus + Node 20 preflight
Adds the PATHFINDER-2026-04-22 principle-driven refactor plan (11 docs,
cross-checked PASS) plus the exploratory PATHFINDER-2026-04-21 corpus
that motivated it. Bumps engines.node to >=20.0.0 per the ingestion-path
plan preflight (recursive fs.watch). Adds the pathfinder skill.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: land PATHFINDER Plan 01 — data integrity
Schema, UNIQUE constraints, self-healing claim, Chroma upsert fallback.
- Phase 1: fresh schema.sql regenerated at post-refactor shape.
- Phase 2: migrations 23+24 — rebuild pending_messages without
started_processing_at_epoch; UNIQUE(session_id, tool_use_id);
UNIQUE(memory_session_id, content_hash) on observations; dedup
duplicate rows before adding indexes.
- Phase 3: claimNextMessage rewritten to self-healing query using
worker_pid NOT IN live_worker_pids; STALE_PROCESSING_THRESHOLD_MS
and the 60-s stale-reset block deleted.
- Phase 4: DEDUP_WINDOW_MS and findDuplicateObservation deleted;
observations.insert now uses ON CONFLICT DO NOTHING.
- Phase 5: failed-message purge block deleted from worker-service
2-min interval; clearFailedOlderThan method deleted.
- Phase 6: repairMalformedSchema and its Python subprocess repair
path deleted from Database.ts; SQLite errors now propagate.
- Phase 7: Chroma delete-then-add fallback gated behind
CHROMA_SYNC_FALLBACK_ON_CONFLICT env flag as bridge until
Chroma MCP ships native upsert.
- Phase 8: migration 19 no-op block absorbed into fresh schema.sql.
Verification greps all return 0 matches. bun test tests/sqlite/
passes 63/63. bun run build succeeds.
Plan: PATHFINDER-2026-04-22/01-data-integrity.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: land PATHFINDER Plan 02 — process lifecycle
OS process groups replace hand-rolled reapers. Worker runs until
killed; orphans are prevented by detached spawn + kill(-pgid).
- Phase 1: src/services/worker/ProcessRegistry.ts DELETED. The
canonical registry at src/supervisor/process-registry.ts is the
sole survivor; SDK spawn site consolidated into it via new
createSdkSpawnFactory/spawnSdkProcess/getSdkProcessForSession/
ensureSdkProcessExit/waitForSlot helpers.
- Phase 2: SDK children spawn with detached:true + stdio:
['ignore','pipe','pipe']; pgid recorded on ManagedProcessInfo.
- Phase 3: shutdown.ts signalProcess teardown uses
process.kill(-pgid, signal) on Unix when pgid is recorded;
Windows path unchanged (tree-kill/taskkill).
- Phase 4: all reaper intervals deleted — startOrphanReaper call,
staleSessionReaperInterval setInterval (including the co-located
WAL checkpoint — SQLite's built-in wal_autocheckpoint handles
WAL growth without an app-level timer), killIdleDaemonChildren,
killSystemOrphans, reapOrphanedProcesses, reapStaleSessions, and
detectStaleGenerator. MAX_GENERATOR_IDLE_MS and MAX_SESSION_IDLE_MS
constants deleted.
- Phase 5: abandonedTimer — already 0 matches; primary-path cleanup
via generatorPromise.finally() already lives in worker-service
startSessionProcessor and SessionRoutes ensureGeneratorRunning.
- Phase 6: evictIdlestSession and its evict callback deleted from
SessionManager. Pool admission gates backpressure upstream.
- Phase 7: SDK-failure fallback — SessionManager has zero matches
for fallbackAgent/Gemini/OpenRouter. Failures surface to hooks
via exit code 2 through SessionRoutes error mapping.
- Phase 8: ensureWorkerRunning in worker-utils.ts rewritten to
lazy-spawn — consults isWorkerPortAlive (which gates
captureProcessStartToken for PID-reuse safety via commit
99060bac), then spawns detached with unref(), then
waitForWorkerPort({ attempts: 3, backoffMs: 250 }) hand-rolled
exponential backoff 250→500→1000ms. No respawn npm dep.
- Phase 9: idle self-shutdown — zero matches for
idleCheck/idleTimeout/IDLE_MAX_MS/idleShutdown. Worker exits
only on external SIGTERM via supervisor signal handlers.
Three test files that exercised deleted code removed:
tests/worker/process-registry.test.ts,
tests/worker/session-lifecycle-guard.test.ts,
tests/services/worker/reap-stale-sessions.test.ts.
Pass count: 1451 → 1407 (-44), all attributable to deleted test
files. Zero new failures. 31 pre-existing failures remain
(schema-repair suite, logger-usage-standards, environmental
openclaw / plugin-distribution) — none introduced by Plan 02.
All 10 verification greps return 0. bun run build succeeds.
Plan: PATHFINDER-2026-04-22/02-process-lifecycle.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: land PATHFINDER Plan 04 (narrowed) — search fail-fast
Phases 3, 5, 6 only. Plan-doc inaccuracies for phases 1/2/4/7/8/9
deferred for plan reconciliation:
- Phase 1/2: ObservationRow type doesn't exist; the four
"formatters" operate on three incompatible types.
- Phase 4: RECENCY_WINDOW_MS already imported from
SEARCH_CONSTANTS at every call site.
- Phase 7: getExistingChromaIds is NOT @deprecated and has an
active caller in ChromaSync.backfillMissingSyncs.
- Phase 8: estimateTokens already consolidated.
- Phase 9: knowledge-corpus rewrite blocked on PG-3
prompt-caching cost smoke test.
Phase 3 — Delete SearchManager.findByConcept/findByFile/findByType.
SearchRoutes handlers (handleSearchByConcept/File/Type) now call
searchManager.getOrchestrator().findByXxx() directly via new
getter accessors on SearchManager. ~250 LoC deleted.
Phase 5 — Fail-fast Chroma. Created
src/services/worker/search/errors.ts with ChromaUnavailableError
extends AppError(503, 'CHROMA_UNAVAILABLE'). Deleted
SearchOrchestrator.executeWithFallback's Chroma-failed
SQLite-fallback branch; runtime Chroma errors now throw 503.
"Path 3" (chromaSync was null at construction — explicit-
uninitialized config) preserved as legitimate empty-result state
per plan text. ChromaSearchStrategy.search no longer wraps in
try/catch — errors propagate.
Phase 6 — Delete HybridSearchStrategy three try/catch silent
fallback blocks (findByConcept, findByType, findByFile) at lines
~82-95, ~120-132, ~161-172. Removed `fellBack` field from
StrategySearchResult type and every return site
(SQLiteSearchStrategy, BaseSearchStrategy.emptyResult,
SearchOrchestrator).
Tests updated (Principle 7 — delete in same PR):
- search-orchestrator.test.ts: "fall back to SQLite" rewritten
as "throw ChromaUnavailableError (HTTP 503)".
- chroma/hybrid/sqlite-search-strategy tests: rewritten to
rejects.toThrow; removed fellBack assertions.
Verification: SearchManager.findBy → 0; fellBack → 0 in src/.
bun test tests/worker/search/ → 122 pass, 0 fail.
bun test (suite-wide) → 1407 pass, baseline maintained, 0 new
failures. bun run build succeeds.
Plan: PATHFINDER-2026-04-22/04-read-path.md (Phases 3, 5, 6)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: land PATHFINDER Plan 03 — ingestion path
Fail-fast parser, direct in-process ingest, recursive fs.watch,
DB-backed tool pairing. Worker-internal HTTP loopback eliminated.
- Phase 0: Created src/services/worker/http/shared.ts exporting
ingestObservation/ingestPrompt/ingestSummary as direct
in-process functions plus ingestEventBus (Node EventEmitter,
reusing existing pattern — no third event bus introduced).
setIngestContext wires the SessionManager dependency from
worker-service constructor.
- Phase 1: src/sdk/parser.ts collapsed to one parseAgentXml
returning { valid:true; kind: 'observation'|'summary'; data }
| { valid:false; reason: string }. Inspects root element;
<skip_summary reason="…"/> is a first-class summary case
with skipped:true. NEVER returns undefined. NEVER coerces.
- Phase 2: ResponseProcessor calls parseAgentXml exactly once,
branches on the discriminated union. On invalid → markFailed
+ logger.warn(reason). On observation → ingestObservation.
On summary → ingestSummary then emit summaryStoredEvent
{ sessionId, messageId } (consumed by Plan 05's blocking
/api/session/end).
- Phase 3: Deleted consecutiveSummaryFailures field
(ResponseProcessor + SessionManager + worker-types) and
MAX_CONSECUTIVE_SUMMARY_FAILURES constant. Circuit-breaker
guards and "tripped" log lines removed.
- Phase 4: coerceObservationToSummary deleted from sdk/parser.ts.
- Phase 5: src/services/transcripts/watcher.ts rescan setInterval
replaced with fs.watch(transcriptsRoot, { recursive: true,
persistent: true }) — Node 20+ recursive mode.
- Phase 6: src/services/transcripts/processor.ts pendingTools
Map deleted. tool_use rows insert with INSERT OR IGNORE on
UNIQUE(session_id, tool_use_id) (added by Plan 01). New
pairToolUsesByJoin query in PendingMessageStore for read-time
pairing (UNIQUE INDEX provides idempotency; explicit consumer
not yet wired).
- Phase 7: HTTP loopback at processor.ts:252 replaced with
direct ingestObservation call. maybeParseJson silent-passthrough
rewritten to fail-fast (throws on malformed JSON).
- Phase 8: src/utils/tag-stripping.ts countTags + stripTagsInternal
collapsed into one alternation regex, single-pass over input.
- Phase 9: src/utils/transcript-parser.ts (dead TranscriptParser
class) deleted. The active extractLastMessage at
src/shared/transcript-parser.ts:41-144 is the sole survivor.
Tests updated (Principle 7 — same-PR delete):
- tests/sdk/parser.test.ts + parse-summary.test.ts: rewritten
to assert discriminated-union shape; coercion-specific
scenarios collapse into { valid:false } assertions.
- tests/worker/agents/response-processor.test.ts: circuit-breaker
describe block skipped; non-XML/empty-response tests assert
fail-fast markFailed behavior.
Verification: every grep returns 0. transcript-parser.ts deleted.
bun run build succeeds. bun test → 1399 pass / 28 fail / 7 skip
(net -8 pass = the 4 retired circuit-breaker tests + 4 collapsed
parser cases). Zero new failures vs baseline.
Deferred (out of Plan 03 scope, will land in Plan 06): SessionRoutes
HTTP route handlers still call sessionManager.queueObservation
inline rather than the new shared helpers — the helpers are ready,
the route swap is mechanical and belongs with the Zod refactor.
Plan: PATHFINDER-2026-04-22/03-ingestion-path.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: land PATHFINDER Plan 05 — hook surface
Worker-call plumbing collapsed to one helper. Polling replaced by
server-side blocking endpoint. Fail-loud counter surfaces persistent
worker outages via exit code 2.
- Phase 1: plugin/hooks/hooks.json — three 20-iteration `for i in
1..20; do curl -sf .../health && break; sleep 0.1; done` shell
retry wrappers deleted. Hook commands invoke their bun entry
point directly.
- Phase 2: src/shared/worker-utils.ts — added
executeWithWorkerFallback<T>(url, method, body) returning
T | { continue: true; reason?: string }. All 8 hook handlers
(observation, session-init, context, file-context, file-edit,
summarize, session-complete, user-message) rewritten to use
it instead of duplicating the ensureWorkerRunning →
workerHttpRequest → fallback sequence.
- Phase 3: blocking POST /api/session/end in SessionRoutes.ts
using validateBody + sessionEndSchema (z.object({sessionId})).
One-shot ingestEventBus.on('summaryStoredEvent') listener,
30 s timer, req.aborted handler — all share one cleanup so
the listener cannot leak. summarize.ts polling loop, plus
MAX_WAIT_FOR_SUMMARY_MS / POLL_INTERVAL_MS constants, deleted.
- Phase 4: src/shared/hook-settings.ts — loadFromFileOnce()
memoizes SettingsDefaultsManager.loadFromFile per process.
Per-handler settings reads collapsed.
- Phase 5: src/shared/should-track-project.ts — single exclusion
check entry; isProjectExcluded no longer referenced from
src/cli/handlers/.
- Phase 6: cwd validation pushed into adapter normalizeInput
(all 6 adapters: claude-code, cursor, raw, gemini-cli,
windsurf). New AdapterRejectedInput error in
src/cli/adapters/errors.ts. Handler-level isValidCwd checks
deleted from file-edit.ts and observation.ts. hook-command.ts
catches AdapterRejectedInput → graceful fallback.
- Phase 7: session-init.ts conditional initAgent guard deleted;
initAgent is idempotent. tests/hooks/context-reinjection-guard
test (validated the deleted conditional) deleted in same PR
per Principle 7.
- Phase 8: fail-loud counter at ~/.claude-mem/state/hook-failures
.json. Atomic write via .tmp + rename. CLAUDE_MEM_HOOK_FAIL_LOUD
_THRESHOLD setting (default 3). On consecutive worker-unreachable
≥ N: process.exit(2). On success: reset to 0. NOT a retry.
- Phase 9: ensureWorkerAliveOnce() module-scope memoization
wrapping ensureWorkerRunning. executeWithWorkerFallback calls
the memoized version.
Minimal validateBody middleware stub at
src/services/worker/http/middleware/validateBody.ts. Plan 06 will
expand with typed inference + error envelope conventions.
Verification: 4/4 grep targets pass. bun run build succeeds.
bun test → 1393 pass / 28 fail / 7 skip; -6 pass attributable
solely to deleted context-reinjection-guard test file. Zero new
failures vs baseline.
Plan: PATHFINDER-2026-04-22/05-hook-surface.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: land PATHFINDER Plan 06 — API surface
One Zod-based validator wrapping every POST/PUT. Rate limiter,
diagnostic endpoints, and shutdown wrappers deleted. Failure-
marking consolidated to one helper.
- Phase 1 (preflight): zod@^3 already installed.
- Phase 2: validateBody middleware confirmed at canonical shape
in src/services/worker/http/middleware/validateBody.ts —
safeParse → 400 { error: 'ValidationError', issues: [...] }
on failure, replaces req.body with parsed value on success.
- Phase 3: Per-route Zod schemas declared at the top of each
route file. 24 POST endpoints across SessionRoutes,
CorpusRoutes, DataRoutes, MemoryRoutes, SearchRoutes,
LogsRoutes, SettingsRoutes now wrap with validateBody().
/api/session/end (Plan 05) confirmed using same middleware.
- Phase 4: validateRequired() deleted from BaseRouteHandler
along with every call site. Inline coercion helpers
(coerceStringArray, coercePositiveInteger) and inline
if (!req.body...) guards deleted across all route files.
- Phase 5: Rate limiter middleware and its registration deleted
from src/services/worker/http/middleware.ts. Worker binds
127.0.0.1:37777 — no untrusted caller.
- Phase 6: viewer.html cached at module init in ViewerRoutes.ts
via fs.readFileSync; served as Buffer with text/html content
type. SKILL.md + per-operation .md files cached in
Server.ts as Map<string, string>; loadInstructionContent
helper deleted. NO fs.watch, NO TTL — process restart is the
cache-invalidation event.
- Phase 7: Four diagnostic endpoints deleted from DataRoutes.ts
— /api/pending-queue (GET), /api/pending-queue/process (POST),
/api/pending-queue/failed (DELETE), /api/pending-queue/all
(DELETE). Helper methods that ONLY served them
(getQueueMessages, getStuckCount, getRecentlyProcessed,
clearFailed, clearAll) deleted from PendingMessageStore.
KEPT: /api/processing-status (observability), /health
(used by ensureWorkerRunning).
- Phase 8: stopSupervisor wrapper deleted from supervisor/index.ts.
GracefulShutdown now calls getSupervisor().stop() directly.
Two functions retained with clear roles:
- performGracefulShutdown — worker-side 6-step shutdown
- runShutdownCascade — supervisor-side child teardown
(process.kill(-pgid), Windows tree-kill, PID-file cleanup)
Each has unique non-trivial logic and a single canonical caller.
- Phase 9: transitionMessagesTo(status, filter) is the sole
failure-marking path on PendingMessageStore. Old methods
markSessionMessagesFailed and markAllSessionMessagesAbandoned
deleted along with all callers (worker-service,
SessionCompletionHandler, tests/zombie-prevention).
Tests updated (Principle 7 same-PR delete): coercion test files
refactored to chain validateBody → handler. Zombie-prevention
tests rewritten to call transitionMessagesTo.
Verification: all 4 grep targets → 0. bun run build succeeds.
bun test → 1393 pass / 28 fail / 7 skip — exact match to
baseline. Zero new failures.
Plan: PATHFINDER-2026-04-22/06-api-surface.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: land PATHFINDER Plan 07 — dead code sweep
ts-prune-driven sweep across the tree after Plans 01-06 landed.
Deleted unused exports, orphan helpers, and one fully orphaned
file. Earlier-plan deletions verified.
Deleted:
- src/utils/bun-path.ts (entire file — getBunPath, getBunPathOrThrow,
isBunAvailable: zero importers)
- bun-resolver.getBunVersionString: zero callers
- PendingMessageStore.retryMessage / resetProcessingToPending /
abortMessage: superseded by transitionMessagesTo (Plan 06 Phase 9)
- EnvManager.MANAGED_CREDENTIAL_KEYS, EnvManager.setCredential:
zero callers
- CodexCliInstaller.checkCodexCliStatus: zero callers; no status
command exists in npx-cli
- Two "REMOVED: cleanupOrphanedSessions" stale-fence comments
Kept (with documented justification):
- Public API surface in dist/sdk/* (parseAgentXml, prompt
builders, ParsedObservation, ParsedSummary, ParseResult,
SUMMARY_MODE_MARKER) — exported via package.json sdk path.
- generateContext / loadContextConfig / token utilities — used
via dynamic await import('../../../context-generator.js') in
worker SearchRoutes.
- MCP_IDE_INSTALLERS, install/uninstall functions for codex/goose
— used via dynamic await import in npx-cli/install.ts +
uninstall.ts (ts-prune cannot trace dynamic imports).
- getExistingChromaIds — active caller in
ChromaSync.backfillMissingSyncs (Plan 04 narrowed scope).
- processPendingQueues / getSessionsWithPendingMessages — active
orphan-recovery caller in worker-service.ts plus
zombie-prevention test coverage.
- StoreAndMarkCompleteResult legacy alias — return-type annotation
in same file.
- All Database.ts barrel re-exports — used downstream.
Earlier-plan verification:
- Plan 03 Phase 9: VERIFIED — src/utils/transcript-parser.ts
is gone; TranscriptParser has 0 references in src/.
- Plan 01 Phase 8: VERIFIED — migration 19 no-op absorbed.
- SessionStore.ts:52-70 consolidation NOT executed (deferred):
the methods are not thin wrappers but ~900 LoC of bodies, and
two methods are documented as intentional mirrors so the
context-generator.cjs bundle stays schema-consistent without
pulling MigrationRunner. Deserves its own plan, not a sweep.
Verification: TranscriptParser → 0; transcript-parser.ts → gone;
no commented-out code markers remain. bun run build succeeds.
bun test → 1393 pass / 28 fail / 7 skip — EXACT match to
baseline. Zero regressions.
Plan: PATHFINDER-2026-04-22/07-dead-code.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: remove residual ProcessRegistry comment reference
Plan 07 dead-code sweep missed one comment-level reference to the
deleted in-memory ProcessRegistry class in SessionManager.ts:347.
Rewritten to describe the supervisor.json scope without naming the
deleted class, completing the verification grep target.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: address Greptile review (P1 + 2× P2)
P1 — Plan 05 Phase 3 blocking endpoint was non-functional:
executeWithWorkerFallback used HEALTH_CHECK_TIMEOUT_MS (3 s) for
the POST /api/session/end call, but the server holds the
connection for SERVER_SIDE_SUMMARY_TIMEOUT_MS (30 s). Client
always raced to a "timed out" rejection that isWorkerUnavailable
classified as worker-unreachable, so the hook silently degraded
instead of waiting for summaryStoredEvent.
- Added optional timeoutMs to executeWithWorkerFallback,
forwarded to workerHttpRequest.
- summarize.ts call site now passes 35_000 (5 s above server
hold window).
P2 — ingestSummary({ kind: 'parsed' }) branch was dead code:
ResponseProcessor emitted summaryStoredEvent directly via the
event bus, bypassing the centralized helper that the comment
claimed was the single source.
- ResponseProcessor now calls ingestSummary({ kind: 'parsed',
sessionDbId, messageId, contentSessionId, parsed }) so the
event-emission path is single-sourced.
- ingestSummary's requireContext() resolution moved inside the
'queue' branch (the only branch that needs sessionManager /
dbManager). 'parsed' is a pure event-bus emission and
doesn't need worker-internal context — fixes mocked
ResponseProcessor unit tests that don't call
setIngestContext.
P2 — isWorkerFallback could false-positive on legitimate API
responses whose schema includes { continue: true, ... }:
- Added a Symbol.for('claude-mem/worker-fallback') brand to
WorkerFallback. isWorkerFallback now checks the brand, not
a duck-typed property name.
Verification: bun run build succeeds. bun test → 1393 pass /
28 fail / 7 skip — exact baseline match. Zero new failures.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: address Greptile iteration 2 (P1 + P2)
P1 — summaryStoredEvent fired regardless of whether the row was
persisted. ResponseProcessor's call to ingestSummary({ kind:
'parsed' }) ran for every parsed.kind === 'summary' even when
result.summaryId came back null (e.g. FK violation, null
memory_session_id at commit). The blocking /api/session/end
endpoint then returned { ok: true } and the Stop hook logged
'Summary stored' for a non-existent row.
- Gate ingestSummary call on (parsed.data.skipped ||
session.lastSummaryStored). Skipped summaries are an explicit
no-op bypass and still confirm; real summaries only confirm
when storage actually wrote a row.
- Non-skipped + summaryId === null path logs a warn and lets
the server-side timeout (504) surface to the hook instead of
a false ok:true.
P2 — PendingMessageStore.enqueue() returns 0 when INSERT OR
IGNORE suppresses a duplicate (the UNIQUE(session_id, tool_use_id)
constraint added by Plan 01 Phase 1). The two callers
(SessionManager.queueObservation and queueSummarize) previously
logged 'ENQUEUED messageId=0' which read like a row was inserted.
- Branch on messageId === 0 and emit a 'DUP_SUPPRESSED' debug
log instead of the misleading ENQUEUED line. No behavior
change — the duplicate is still correctly suppressed by the
DB (Principle 3); only the log surface is corrected.
- confirmProcessed is never called with the enqueue() return
value (it operates on session.processingMessageIds[] from
claimNextMessage), so no caller is broken; the visibility
fix prevents future misuse.
Verification: bun run build succeeds. bun test → 1393 pass /
28 fail / 7 skip — exact baseline match. Zero new failures.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: address Greptile iteration 3 (P1 + 2× P2)
- P1 worker-service.ts: wire ensureGeneratorRunning into the ingest
context after SessionRoutes is constructed. setIngestContext runs
before routes exist, so transcript-watcher observations queued via
ingestObservation() had no way to auto-start the SDK generator.
Added attachIngestGeneratorStarter() to patch the callback in.
- P2 shared.ts: IngestEventBus now sets maxListeners to 0. Concurrent
/api/session/end calls register one listener each and clean up on
completion, so the default-10 warning fires spuriously under normal
load.
- P2 SessionRoutes.ts: handleObservationsByClaudeId now delegates to
ingestObservation() instead of duplicating skip-tool / meta /
privacy / queue logic. Single helper, matching the Plan 03 goal.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: address Greptile iteration 4 (P1 tool-pair + P2 parse/path/doc)
- processor.handleToolResult: restore in-memory tool-use→tool-result
pairing via session.pendingTools for schemas (e.g. Codex) whose
tool_result events carry only tool_use_id + output. Without this,
neither handler fired — all tool observations silently dropped.
- processor.maybeParseJson: return raw string on parse failure instead
of throwing. Previously a single malformed JSON-shaped field caused
handleLine's outer catch to discard the entire transcript line.
- watcher.deepestNonGlobAncestor: split on / and \\, emit empty string
for purely-glob inputs so the caller skips the watch instead of
anchoring fs.watch at the filesystem root. Windows-compatible.
- PendingMessageStore.enqueue: tighten docstring — callers today only
log on the returned id; the SessionManager branches on id === 0.
* fix: forward tool_use_id through ingestObservation (Greptile iter 5)
P1 — Plan 01's UNIQUE(content_session_id, tool_use_id) dedup never
fired because the new shared ingest path dropped the toolUseId before
queueObservation. SQLite treats NULL values as distinct for UNIQUE,
so every replayed transcript line landed a duplicate row.
- shared.ingestObservation: forward payload.toolUseId to
queueObservation so INSERT OR IGNORE can actually collapse.
- SessionRoutes.handleObservationsByClaudeId: destructure both
tool_use_id (HTTP convention) and toolUseId (JS convention) from
req.body and pass into ingestObservation.
- observationsByClaudeIdSchema: declare both keys explicitly so the
validator doesn't rely on .passthrough() alone.
* fix: drop dead pairToolUsesByJoin, close session-end listener race
- PendingMessageStore: delete pairToolUsesByJoin. The method was never
called and its self-join semantics are structurally incompatible
with UNIQUE(content_session_id, tool_use_id): INSERT OR IGNORE
collapses any second row with the same pair, so a self-join can
only ever match a row to itself. In-memory pendingTools in
processor.ts remains the pairing path for split-event schemas.
- IngestEventBus: retain a short-lived (60s) recentStored map keyed
by sessionId. Populated on summaryStoredEvent emit, evicted on
consume or TTL.
- handleSessionEnd: drain the recent-events buffer before attaching
the listener. Closes the register-after-emit race where the summary
can persist between the hook's summarize POST and its session/end
POST — previously that window returned 504 after the 30s timeout.
* chore: merge origin/main into vivacious-teeth
Resolves conflicts with 15 commits on main (v12.3.9, security
observation types, Telegram notifier, PID-reuse worker start-guard).
Conflict resolution strategy:
- plugin/hooks/hooks.json, plugin/scripts/*.cjs, plugin/ui/viewer-bundle.js:
kept ours — PATHFINDER Plan 05 deletes the for-i-in-1-to-20 curl retry
loops and the built artifacts regenerate on build.
- src/cli/handlers/summarize.ts: kept ours — Plan 05 blocking
POST /api/session/end supersedes main's fire-and-forget path.
- src/services/worker-service.ts: kept ours — Plan 05 ingest bus +
summaryStoredEvent supersedes main's SessionCompletionHandler DI
refactor + orphan-reaper fallback.
- src/services/worker/http/routes/SessionRoutes.ts: kept ours — same
reason; generator .finally() Stop-hook self-clean is a guard for a
path our blocking endpoint removes.
- src/services/worker/http/routes/CorpusRoutes.ts: merged — added
security_alert / security_note to ALLOWED_CORPUS_TYPES (feature from
#2084) while preserving our Zod validateBody schema.
Typecheck: 294 errors (vs 298 pre-merge). No new errors introduced; all
remaining are pre-existing (Component-enum gaps, DOM lib for viewer,
bun:sqlite types).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: address Greptile P2 findings
1) SessionRoutes.handleSessionEnd was the only route handler not wrapped
in wrapHandler — synchronous exceptions would hang the client rather
than surfacing as 500s. Wrap it like every other handler.
2) processor.handleToolResult only consumed the session.pendingTools
entry when the tool_result arrived without a toolName. In the
split-schema path where tool_result carries both toolName and toolId,
the entry was never deleted and the map grew for the life of the
session. Consume the entry whenever toolId is present.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: typing cleanup and viewer tsconfig split for PR feedback
- Add explicit return types for SessionStore query methods
- Exclude src/ui/viewer from root tsconfig, give it its own DOM-typed config
- Add bun to root tsconfig types, plus misc typing tweaks flagged by Greptile
- Rebuilt plugin/scripts/* artifacts
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: address Greptile P2 findings (iter 2)
- PendingMessageStore.transitionMessagesTo: require sessionDbId (drop
the unscoped-drain branch that would nuke every pending/processing
row across all sessions if a future caller omitted the filter).
- IngestEventBus.takeRecentSummaryStored: make idempotent — keep the
cached event until TTL eviction so a retried Stop hook's second
/api/session/end returns immediately instead of hanging 30 s.
- TranscriptWatcher fs.watch callback: skip full glob scan for paths
already tailed (JSONL appends fire on every line; only unknown
paths warrant a rescan).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: call finalizeSession in terminal session paths (Greptile iter 3)
terminateSession and runFallbackForTerminatedSession previously called
SessionCompletionHandler.finalizeSession before removeSessionImmediate;
the refactor dropped those calls, leaving sdk_sessions.status='active'
for every session killed by wall-clock limit, unrecoverable error, or
exhausted fallback chain. The deleted reapStaleSessions interval was
the only prior backstop.
Re-wires finalizeSession (idempotent: marks completed, drains pending,
broadcasts) into both paths; no reaper reintroduced.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: GC failed pending_messages rows at startup (Greptile iter 4)
Plan 07 deleted clearFailed/clearFailedOlderThan as "dead code", but
with the periodic sweep also removed, nothing reaps status='failed'
rows now — they accumulate indefinitely. Since claimNextMessage's
self-healing subquery scans this table, unbounded growth degrades
claim latency over time.
Re-introduces clearFailedOlderThan and calls it once at worker startup
(not a reaper — one-shot, idempotent). 7-day retention keeps enough
history for operator inspection while bounding the table.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: finalize sessions on normal exit; cleanup hoist; share handler (iter 5)
1. startSessionProcessor success branch now calls completionHandler.
finalizeSession before removeSessionImmediate. Hooks-disabled installs
(and any Stop hook that fails before POST /api/sessions/complete) no
longer leave sdk_sessions rows as status='active' forever. Idempotent
— a subsequent /api/sessions/complete is a no-op.
2. Hoist SessionRoutes.handleSessionEnd cleanup declaration above the
closures that reference it (TDZ safety; safe at runtime today but
fragile if timeout ever shrinks).
3. SessionRoutes now receives WorkerService's shared SessionCompletionHandler
instead of constructing its own — prevents silent divergence if the
handler ever becomes stateful.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: stop runaway crash-recovery loop on dead sessions
Two distinct bugs were combining to keep a dead session restarting forever:
Bug 1 (uncaught "The operation was aborted."):
child_process.spawn emits 'error' asynchronously for ENOENT/EACCES/abort
signal aborts. spawnSdkProcess() never attached an 'error' listener, so
any async spawn failure became uncaughtException and escaped to the
daemon-level handler. Attach an 'error' listener immediately after spawn,
before the !child.pid early-return, so async spawn errors are logged
(with errno code) and swallowed locally.
Bug 2 (sliding-window limiter never trips on slow restart cadence):
RestartGuard tripped only when restartTimestamps.length exceeded
MAX_WINDOWED_RESTARTS (10) within RESTART_WINDOW_MS (60s). With the 8s
exponential-backoff cap, only ~7-8 restarts fit in the window, so a dead
session that fail-restart-fail-restart on 8s cycles would loop forever
(consecutiveRestarts climbing past 30+ in observed logs). Add a
consecutiveFailures counter that increments on every restart and resets
only on recordSuccess(). Trip when consecutive failures exceed
MAX_CONSECUTIVE_FAILURES (5) — meaning 5 restarts with zero successful
processing in between proves the session is dead. Both guards now run in
parallel: tight loops still trip the windowed cap; slow loops trip the
consecutive-failure cap.
Also: when the SessionRoutes path trips the guard, drain pending messages
to 'abandoned' so the session does not reappear in
getSessionsWithPendingMessages and trigger another auto-start cycle. The
worker-service.ts path already does this via terminateSession.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* perf: streamline worker startup and consolidate database connections
1. Database Pooling: Modified DatabaseManager, SessionStore, and SessionSearch to share a single bun:sqlite connection, eliminating redundant file descriptors.
2. Non-blocking Startup: Refactored WorktreeAdoption and Chroma backfill to run in the background (fire-and-forget), preventing them from stalling core initialization.
3. Diagnostic Routes: Added /api/chroma/status and bypassed the initialization guard for health/readiness endpoints to allow diagnostics during startup.
4. Robust Search: Implemented reliable SQLite FTS5 fallback in SearchManager for when Chroma (uvx) fails or is unavailable.
5. Code Cleanup: Removed redundant loopback MCP checks and mangled initialization logic from WorkerService.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: hard-exclude observer-sessions from hooks; bundle migration 29 (#2124)
* fix: hard-exclude observer-sessions from hooks; backfill bundle migrations
Stop hook + SessionEnd hook were storing the SDK observer's own
init/continuation/summary prompts in user_prompts, leaking into the
viewer (meta-observation regression). 25 such rows accumulated.
- shouldTrackProject: hard-reject OBSERVER_SESSIONS_DIR (and its subtree)
before consulting user-configured exclusion globs.
- summarize.ts (Stop) and session-complete.ts (SessionEnd): early-return
when shouldTrackProject(cwd) is false, so the observer's own hooks
cannot bootstrap the worker or queue a summary against the meta-session.
- SessionRoutes: cap user-prompt body at 256 KiB at the session-init
boundary so a runaway observer prompt cannot blow up storage.
- SessionStore: add migration 29 (UNIQUE(memory_session_id, content_hash)
on observations) inline so bundled artifacts (worker-service.cjs,
context-generator.cjs) stay schema-consistent — without it, the
ON CONFLICT clause in observation inserts throws.
- spawnSdkProcess: stdio[stdin] from 'ignore' to 'pipe' so the
supervisor can actually feed the observer's stdin.
Also rebuilds plugin/scripts/{worker-service,context-generator}.cjs.
* fix: walk back to UTF-8 boundary on prompt truncation (Greptile P2)
Plain Buffer.subarray at MAX_USER_PROMPT_BYTES can land mid-codepoint,
which the utf8 decoder silently rewrites to U+FFFD. Walk back over any
continuation bytes (0b10xxxxxx) before decoding so the truncated prompt
ends on a valid sequence boundary instead of a replacement character.
* fix: cross-platform observer-dir containment; clarify SDK stdin pipe
claude-review feedback on PR #2124.
- shouldTrackProject: literal `cwd.startsWith(OBSERVER_SESSIONS_DIR + '/')`
hard-coded a POSIX separator and missed Windows backslash paths plus any
trailing-slash variance. Switched to a path.relative-based isWithin()
helper so Windows hook input under observer-sessions\\... is also excluded.
- spawnSdkProcess: added a comment explaining why stdin must be 'pipe' —
SpawnedSdkProcess.stdin is typed NonNullable and the Claude Agent SDK
consumes that pipe; 'ignore' would null it and the null-check below
would tear the child down on every spawn.
* fix: make Stop hook fire-and-forget; remove dead /api/session/end
The Stop hook was awaiting a 35-second long-poll on /api/session/end,
which the worker held open until the summary-stored event fired (or its
30s server-side timeout elapsed). Followed by another await on
/api/sessions/complete. Three sequential awaits, the middle one a 30s
hold — not fire-and-forget despite repeated requests.
The Stop hook now does ONE thing: POST /api/sessions/summarize to
queue the summary work and return. The worker drives the rest async.
Session-map cleanup is performed by the SessionEnd handler
(session-complete.ts), not duplicated here.
- summarize.ts: drop the /api/session/end long-poll and the trailing
/api/sessions/complete await; ~40 lines removed; unused
SessionEndResponse interface gone; header comment rewritten.
- SessionRoutes: delete handleSessionEnd, sessionEndSchema, the
SERVER_SIDE_SUMMARY_TIMEOUT_MS constant, and the /api/session/end
route registration. Drop the now-unused ingestEventBus and
SummaryStoredEvent imports.
- ResponseProcessor + shared.ts + worker-utils.ts: update stale
comments that referenced the dead endpoint. The IngestEventBus is
left in place dormant (no listeners) for follow-up cleanup so this
PR stays focused on the blocker.
Bundle artifact (worker-service.cjs) rebuilt via build-and-sync.
Verification:
- grep '/api/session/end' plugin/scripts/worker-service.cjs → 0
- grep 'timeoutMs:35' plugin/scripts/worker-service.cjs → 0
- Worker restarted clean, /api/health ok at pid 92368
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* deps: bump all dependencies to latest including majors
Upgrades: React 18→19, Express 4→5, Zod 3→4, TypeScript 5→6,
@types/node 20→25, @anthropic-ai/claude-agent-sdk 0.1→0.2,
@clack/prompts 0.9→1.2, plus minors. Adds Daily Maintenance section
to CLAUDE.md mandating latest-version policy across manifests.
Express 5 surfaced a race in Server.listen() where the 'error' handler
was attached after listen() was invoked; refactored to use
http.createServer with both 'error' and 'listening' handlers attached
before listen(), restoring port-conflict rejection semantics.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: surface real chroma errors and add deep status probe
Replace the misleading "Vector search failed - semantic search unavailable.
Install uv... restart the worker." string in SearchManager with the actual
exception text from chroma_query_documents. The lying message blamed `uv`
for any failure — even when the real cause was a chroma-mcp transport
timeout, an empty collection, or a dead subprocess.
Also add /api/chroma/status?deep=1 backed by a new
ChromaMcpManager.probeSemanticSearch() that round-trips a real query
(chroma_list_collections + chroma_query_documents) instead of just
checking the stdio handshake. The cheap default path is unchanged.
Includes the diagnostic plan (PLAN-fix-mcp-search.md) and updated test
fixtures for the new structured failure message.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: rebuild worker-service bundle to match merged src
Bundle was stale after the squash merge of #2124 — it still contained
the old "Install uv... semantic search unavailable" string and lacked
probeSemanticSearch. Rebuilt via bun run build-and-sync.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs: address coderabbit feedback on PLAN-fix-mcp-search.md
- replace machine-specific /Users/alexnewman absolute paths with portable
<repo-root> placeholder (MD-style portability)
- add blank lines around the TypeScript fenced block (MD031)
- tag the bare fenced block with `text` (MD040)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
692 lines
44 KiB
Markdown
692 lines
44 KiB
Markdown
# Pathfinder Phase 6: Implementation Plan
|
||
|
||
**Date**: 2026-04-22
|
||
**Source**: `PATHFINDER-2026-04-21/05-clean-flowcharts.md`
|
||
**Scope**: 15 execution phases to land the brutal-audit cleanup. Each phase is self-contained so it can be run in a fresh chat session.
|
||
|
||
> **Design authority**: `05-clean-flowcharts.md` is the canonical design doc. This plan references it by section number (e.g., "05: 3.2" = section 3.2 of the clean-flowcharts file). When the plan and audit disagree, the plan's *verified-findings* take precedence — those corrections are called out explicitly in Phase 0.
|
||
|
||
---
|
||
|
||
## Phase 0 — Documentation Discovery (ALREADY COMPLETED)
|
||
|
||
The design docs needed for this plan have been read and verified against the live codebase. **Do not re-do this phase**; cite its outputs from later phases.
|
||
|
||
### Sources consulted
|
||
|
||
1. `PATHFINDER-2026-04-21/05-clean-flowcharts.md` — brutal audit + 12 clean flowcharts (Part 3), timer census (Part 4), deletion ledger (Part 5), execution order (Part 6), non-cull list (Part 7)
|
||
2. `PATHFINDER-2026-04-21/02-duplication-report.md` — 12 cross-feature duplication findings (background)
|
||
3. `PATHFINDER-2026-04-21/03-unified-proposal.md` — earlier consolidation targets (U1–U8)
|
||
4. Live codebase at `/Users/alexnewman/.superset/worktrees/claude-mem/vivacious-teeth/src/**/*.ts`
|
||
|
||
### Verified-findings corrections (supersede the audit where they disagree)
|
||
|
||
These were produced by four parallel discovery subagents. Use these numbers in every downstream phase.
|
||
|
||
| # | Audit claimed | Reality | Impact on plan |
|
||
|---|---|---|---|
|
||
| V1 | Summary path only strips `<system-reminder>` (`summarize.ts:66`, `SessionRoutes.ts:669`) | Summary paths strip **ZERO** tags. `handleSummarize` (`SessionRoutes.ts:491`) and `handleSummarizeByClaudeId` (`SessionRoutes.ts:669`) pass `last_assistant_message` straight to `queueSummarize` with no strip. | Privacy gap is **worse** than audit — fix must be added to `ingestSummary`, not a one-line patch. |
|
||
| V2 | Legacy `handleObservations` with no-strip at `SessionRoutes.ts:378` | `handleObservations` is at `SessionRoutes.ts:464`. It does **not** strip tags. `handleObservationsByClaudeId` at `SessionRoutes.ts:560` **does** strip (lines 629–633). | Delete/consolidate *both* into `ingestObservation` helper. |
|
||
| V3 | `stripMemoryTagsFromJson` + `stripMemoryTagsFromPrompt` wrappers exist | Confirmed. `src/utils/tag-stripping.ts:79` and `:89` both delegate to `stripTagsInternal` at line 48. Six sequential `.replace()` calls at lines 61–66. | U6 target is exact. |
|
||
| V4 | Only 3 files call any `stripMemoryTags*` variant | Confirmed. `SessionRoutes.ts:629`, `:633`, `:862`. **No call sites** in summary, legacy observation, or summarize hook. | After U6, verify call-site count equals number of new ingest helpers × text fields. |
|
||
| V5 | `startUnifiedReaper` at `process-registry.ts:492` | **Does not exist**. Supervisor registry (`src/supervisor/process-registry.ts`, 408 lines) has `ProcessRegistry` class + `reapSession()` (line 292) but no background timer. Both reapers live in the **worker layer**. | Phase 6 builds `ReaperTick` fresh in worker-service.ts; supervisor registry stays as-is. |
|
||
| V6 | Two reapers in worker | Confirmed. `startOrphanReaper` (`src/services/worker/ProcessRegistry.ts:508`, invoked from `worker-service.ts:537`, 30 s). `staleSessionReaperInterval` (inline `setInterval` at `worker-service.ts:547`, 2 min, calls `SessionManager.reapStaleSessions`). Orphan reaper does **not** call `reapStaleSessions`. | Phase 6 replaces both. |
|
||
| V7 | `coerceObservationToSummary` exists + non-XML early-fail + circuit breaker | Confirmed. Private fn at `src/sdk/parser.ts:222`. Non-XML fail at `ResponseProcessor.ts:87–108`. Circuit breaker at `ResponseProcessor.ts:176–200` using `session.consecutiveSummaryFailures`. | Phase 3 deletion set is exact. |
|
||
| V8 | 500 ms poll up to 110 s in summarize hook | Confirmed. `src/cli/handlers/summarize.ts:117–150`. Constants: `POLL_INTERVAL_MS = 500` (:24), `MAX_WAIT_FOR_SUMMARY_MS = 110_000` (:25). | Phase 11 replaces with blocking endpoint. |
|
||
| V9 | SessionRoutes has 8 endpoints | Actually **10**: six under `/sessions/:sessionDbId/*` (`:377–:382`) and five under `/api/sessions/*` (`:385–:389`). `/api/sessions/status` is the one summary-hook polls. | Phase 11 collapses to 4; deletes are larger than audit implied. |
|
||
| V10 | `ensureWorkerRunning` at every hook entry | Confirmed. Called in all 8 CLI handlers (`context.ts:19`, `user-message.ts:35`, `summarize.ts:44`, `observation.ts`, `file-context.ts`, `file-edit.ts`, `session-init.ts`, `session-complete.ts`). | Phase 1 hook-cache module lands before endpoint consolidation. |
|
||
| V11 | SearchManager thin facade | Confirmed for `@deprecated` methods (`queryChroma` at `:59`, `searchChromaForTimeline` at `:70`) — but `search()` at `:161–445` does *real* work (result combining, date filtering, grouping, markdown tables). File is 2069 lines. | Phase 4 keeps display-wrap, deletes deprecated + passthroughs only. |
|
||
| V12 | 27 migrations | 22 private methods in `MigrationRunner.runAllMigrations` (lines 22–41 of `src/services/sqlite/migrations/runner.ts`); legacy system adds ~5 more. `schema_versions` table created at `runner.ts:55`. | Phase 9 target is "22+legacy → schema.sql + N upgrade migrations". |
|
||
| V13 | Python `sqlite3` subprocess ~120 lines | Python script embedded; invoked via `execSync('python3 ...')` at `tests/services/sqlite/schema-repair.test.ts:62` (test file is 253 lines; production script similar). | Phase 9 deletion confirmed; move to user-facing `claude-mem repair` subcommand. |
|
||
| V14 | 30-s content-hash dedup window + `findDuplicateObservation` ~30 lines | Confirmed at `src/services/sqlite/observations/store.ts:13` (`DEDUP_WINDOW_MS = 30_000`). `findDuplicateObservation` is 11 lines at `:36–46`. Dedup key is SHA of `(memory_session_id, title, narrative)` — not `tool_use_id`. | Phase 9 adds `UNIQUE(session_id, tool_use_id)` constraint and removes window; this is a **new** constraint, not an existing one. |
|
||
| V15 | No `chroma_synced` column | Confirmed. Phase 10 must add it in a migration. | Blocks Phase 10's backfill simplification. |
|
||
| V16 | Granular per-field Chroma docs (3–5 per obs) | Confirmed. 7 observation fields + 6 summary fields (`ChromaSync.ts:125–256`). `formatObservationsAsDocs` and `formatSummariesAsDocs` produce separate docs. | Phase 10 concatenates into one doc per observation/summary. |
|
||
| V17 | `getExistingChromaIds` metadata scan + delete-then-add on conflict | Confirmed. `getExistingChromaIds` at `ChromaSync.ts:479–545` pages via `chroma_get_documents` with `include: ['metadatas']`. Delete-then-add at `:292–306`. | Phase 10 replaces with `upsert` using stable IDs. |
|
||
| V18 | 5-s rescan + `pendingTools` map + HTTP loopback | Confirmed. `src/services/transcripts/watcher.ts:124` (`rescanIntervalMs ?? 5000`). `pendingTools` in `SessionState` interface. `observation.ts:17` loops through `workerHttpRequest('/api/sessions/observations', …)`. Watcher calls handler directly; handler HTTPs back to worker. | Phase 7 replaces with `fs.watch(parentDir, {recursive})` and direct `ingestObservation(payload)` call. |
|
||
| V19 | 60-s stale reset in every `claimNextMessage` | Confirmed. `src/services/sqlite/PendingMessageStore.ts:99–145`. Constant `STALE_PROCESSING_THRESHOLD_MS = 60_000` at `:6`. | Phase 6 moves the reset to worker startup. |
|
||
| V20 | Rate limiter 300/min | Confirmed at `src/services/worker/http/middleware.ts:45–79`. Constants at `:49–50`. Keyed by IP, normalizes `::ffff:127.0.0.1`. | Phase 14 deletes. |
|
||
|
||
### Allowed APIs (what the refactor may rely on)
|
||
|
||
Copy from these exact sources; do **not** invent.
|
||
|
||
- **bun:sqlite** — `Database`, `db.prepare(sql)`, `db.run`, `db.transaction(fn)`. Unique constraint: `CREATE TABLE x (... UNIQUE(a,b))`. Conflict clause: `INSERT ... ON CONFLICT DO NOTHING` or `ON CONFLICT (a,b) DO UPDATE SET ...`. (Used everywhere under `src/services/sqlite/`.)
|
||
- **Express 4** — `app.get/post`, `router.use(middleware)`, `req.body`, `res.json`, `res.sendFile`, SSE via `res.write('event: …\ndata: …\n\n')`. (See `BaseRouteHandler.ts`, `SSEBroadcaster.ts`.)
|
||
- **Zod** — `z.object({...})`, `schema.safeParse(body)`, `result.success ? result.data : result.error.flatten()`. (Not yet a dep; Phase 12 adds `zod` via npm; already shipped transitively via `@anthropic-ai/sdk` — confirm before landing.)
|
||
- **Node `fs.watch`** — `fs.watch(dir, { recursive: true }, (event, filename) => …)`. On macOS + Linux recursive is supported; Windows is too. New files in the watched directory fire `rename` events. (Replaces the 5-s rescan timer.)
|
||
- **Claude Agent SDK `@anthropic-ai/claude-agent-sdk`** — existing usage in `src/services/worker/SDKAgent.ts`. Agent contract requires `<summary>` OR `<skip_summary/>`; see `src/sdk/prompts.ts` for the exact instruction text.
|
||
|
||
### Anti-patterns to prohibit (cite in every phase)
|
||
|
||
A. **Inventing APIs** — never add a method to a class because it "should exist". Grep the class first.
|
||
B. **Polling where events exist** — `setInterval` + HTTP poll replaced by blocking endpoint or SSE.
|
||
C. **Silent fallbacks** — Chroma failure returns 503, not dropped-query-text search. Parser failure marks `pending_messages` FAILED, not coerced summary.
|
||
D. **Facades that pass through** — if a method body is `return this.other.method(args)`, delete it; call `this.other` directly.
|
||
E. **Two code paths for the same data** — if transcript watcher and CLI handler both ingest observations, they call the same helper. No duplicate tag-strip logic.
|
||
|
||
---
|
||
|
||
## Phase 1 — One `stripMemoryTags` + close summary privacy gap
|
||
|
||
**Outcome**: A single public `stripMemoryTags(text: string): string`. Every text-ingress call-site switches to it. Summary paths strip tags (closes P1 security bug).
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.2 (privacy-tag-filtering clean flowchart)
|
||
- Verified-findings V1, V2, V3, V4
|
||
- `src/utils/tag-stripping.ts:48–91` — existing wrappers
|
||
|
||
### Tasks
|
||
|
||
1. **Rewrite `src/utils/tag-stripping.ts`** to export:
|
||
```ts
|
||
const MEMORY_TAGS = ['private','claude-mem-context','system_instruction','system-instruction','persisted-output','system-reminder'] as const;
|
||
const STRIP_REGEX = new RegExp(`<(${MEMORY_TAGS.join('|')})\\b[^>]*>[\\s\\S]*?<\\/\\1>`, 'g');
|
||
export function stripMemoryTags(text: string): string { /* one pass; ReDoS guard if match count > 100 */ }
|
||
```
|
||
Delete `stripMemoryTagsFromPrompt`, `stripMemoryTagsFromJson`, `stripTagsInternal`, `SYSTEM_REMINDER_REGEX`. Keep the length/timing guards from the existing file if they're there today.
|
||
2. **Fix every call site** to use `stripMemoryTags`:
|
||
- `SessionRoutes.ts:629,633` (was `stripMemoryTagsFromJson`): call on `JSON.stringify(tool_input)` and `JSON.stringify(tool_response)` — same shape, new name.
|
||
- `SessionRoutes.ts:862` (was `stripMemoryTagsFromPrompt`): unchanged signature.
|
||
- **Add** in `SessionRoutes.ts:464` (legacy `handleObservations`): strip `tool_input` and `tool_response` before `queueObservation`.
|
||
- **Add** in `SessionRoutes.ts:491` (`handleSummarize`): strip `last_assistant_message` before `queueSummarize`.
|
||
- **Add** in `SessionRoutes.ts:669` (`handleSummarizeByClaudeId`): same.
|
||
3. **Update the test** `tests/utils/tag-stripping.test.ts` (if present) to cover the merged function; delete tests for the removed wrappers.
|
||
|
||
### Verification
|
||
|
||
- [ ] `grep -r "stripMemoryTagsFromJson\|stripMemoryTagsFromPrompt\|stripTagsInternal" src/` → zero hits.
|
||
- [ ] `grep -c "stripMemoryTags(" src/` ≥ 5 (new call sites: 3 existing + 3 new summary/legacy paths).
|
||
- [ ] Regression test: insert `<private>secret</private>` into a summary via `/sessions/:id/summarize`; assert `session_summaries.last_assistant_message` contains no `<private>` or `secret`.
|
||
- [ ] `npm run build-and-sync` succeeds.
|
||
|
||
### Anti-pattern guards
|
||
|
||
- A: Don't add a `stripMemoryTagsV2` wrapper — rename in place.
|
||
- D: Don't leave the old function names as re-exports "for safety" — delete.
|
||
|
||
### Blast radius
|
||
|
||
Edits: 2 files (`tag-stripping.ts`, `SessionRoutes.ts`). No schema changes.
|
||
|
||
---
|
||
|
||
## Phase 2 — Unified ingest helpers
|
||
|
||
**Outcome**: Three helpers that every ingest point calls. No HTTP loopback inside the worker process.
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.1 (lifecycle-hooks clean), Part 2 Decision D1
|
||
- Verified-findings V2, V18
|
||
- Phase 1 **MUST** be done first.
|
||
|
||
### Tasks
|
||
|
||
1. **Create `src/services/worker/ingest/index.ts`** exporting:
|
||
```ts
|
||
export function ingestObservation(payload: IngestObservationPayload): Promise<IngestResult>;
|
||
export function ingestPrompt(payload: IngestPromptPayload): Promise<IngestResult>;
|
||
export function ingestSummary(payload: IngestSummaryPayload): Promise<IngestResult>;
|
||
```
|
||
Each helper: (a) calls `stripMemoryTags` on user-facing text fields, (b) runs privacy / project-exclusion validation (move logic from `SessionRoutes.handleObservationsByClaudeId:614–621` and `PrivacyCheckValidator.ts`), (c) INSERTs into `pending_messages`. Returns `{ skipped: boolean, id?: number, reason?: string }`.
|
||
2. **Rewire** `SessionRoutes.ts:464` (`handleObservations`), `:560` (`handleObservationsByClaudeId`), `:491` + `:669` (summarize), `:862` (`handleSessionInitByClaudeId` → `ingestPrompt`) to call the helpers. Route handler's job shrinks to body parsing + response serialization.
|
||
3. **Rewire** `src/cli/handlers/observation.ts` to call `ingestObservation` directly when the worker is the current process — but since hooks run in CLI, they still HTTP to the worker. The key change: the worker side of the route talks to the helper, no more inline logic.
|
||
4. **Rewire** `src/services/transcripts/watcher.ts` to call `ingestObservation(payload)` directly (no `workerHttpRequest` from inside the worker). Delete the inner HTTP call from the transcript path.
|
||
|
||
### Verification
|
||
|
||
- [ ] `grep -n "stripMemoryTags" src/services/worker/` → only inside `ingest/index.ts`.
|
||
- [ ] `grep -n "queueObservation\|queueSummarize" src/services/worker/http/routes/SessionRoutes.ts` → zero (handlers use ingest helpers).
|
||
- [ ] Unit tests for each helper: tag stripping, privacy validation, project exclusion, INSERT behaviour, idempotent returns for dup.
|
||
- [ ] Integration: run full hook cycle via `npm run build-and-sync` + trigger `SessionStart` + `PostToolUse`; observe `pending_messages` row.
|
||
|
||
### Anti-pattern guards
|
||
|
||
- E: Don't leave behind `handleObservations` and `handleObservationsByClaudeId` with slightly different logic. One helper, both handlers call it.
|
||
- A: No `IngestService` class unless two existing classes already share state. A module with three functions is enough.
|
||
|
||
### Blast radius
|
||
|
||
Files touched: `SessionRoutes.ts`, new `ingest/*`, `watcher.ts`, `PrivacyCheckValidator.ts` (may collapse into helper). No schema changes.
|
||
|
||
---
|
||
|
||
## Phase 3 — Unify parser; delete coerce + circuit breaker
|
||
|
||
**Outcome**: One `parseAgentXml(text, {requireSummary})`. `coerceObservationToSummary`, consecutive-failure counter, and non-XML early-fail branch are gone. RestartGuard handles repeated failures.
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.7, Part 2 Decision D5
|
||
- Verified-findings V7
|
||
- `src/sdk/parser.ts`, `src/services/worker/agents/ResponseProcessor.ts:87–200`, `src/services/worker/RestartGuard.ts`
|
||
- `src/sdk/prompts.ts` — agent instructions must already state "return `<summary>` or `<skip_summary/>`". If not, update the prompt in this phase.
|
||
|
||
### Tasks
|
||
|
||
1. **Replace `parser.ts`** with:
|
||
```ts
|
||
export interface ParsedAgentOutput {
|
||
observations: ParsedObservation[];
|
||
summary: ParsedSummary | null;
|
||
skipSummary: boolean;
|
||
}
|
||
export interface ParseResult {
|
||
valid: boolean;
|
||
data?: ParsedAgentOutput;
|
||
reason?: 'no_xml' | 'missing_summary' | 'malformed';
|
||
}
|
||
export function parseAgentXml(text: string, opts: { requireSummary: boolean }): ParseResult;
|
||
```
|
||
Delete `parseObservations` and `parseSummary` exports; keep them as private helpers only if the call sites merge into one. Delete `coerceObservationToSummary` outright.
|
||
2. **Update `ResponseProcessor.ts`**:
|
||
- Replace the parse path with a single `parseAgentXml(text, {requireSummary: session.expectsSummary})`.
|
||
- On `valid:false`: call `session.recordFailure(result.reason)` → mark `pending_messages` FAILED → let RestartGuard decide. Delete lines `:87–108` (non-XML early-fail), lines `:176–200` (`consecutiveSummaryFailures` counter + circuit).
|
||
- Remove the `consecutiveSummaryFailures` field from `ActiveSession`.
|
||
3. **Update `sdk/prompts.ts`** if needed so the agent contract is explicit: on work → one or more `<observation>` then exactly one `<summary>`; on no work → `<skip_summary/>`.
|
||
|
||
### Verification
|
||
|
||
- [ ] `grep -n "coerceObservationToSummary\|consecutiveSummaryFailures" src/` → zero hits.
|
||
- [ ] `grep -n "parseObservations\|parseSummary" src/ | grep -v parser.ts` → zero (callers use `parseAgentXml`).
|
||
- [ ] Test: inject garbage-text agent output; assert `pending_messages.status = 'failed'` and no summary row written.
|
||
- [ ] Test: inject valid `<observation>` without `<summary>` when `requireSummary=true`; assert `valid:false, reason:'missing_summary'`.
|
||
- [ ] RestartGuard still trips after N consecutive failures (unchanged count).
|
||
|
||
### Anti-pattern guards
|
||
|
||
- C: Don't coerce "close enough" to `<summary>`. Fail fast.
|
||
- A: No new `ParserValidator` class. Pure function returns a result object.
|
||
|
||
### Blast radius
|
||
|
||
Files: `parser.ts`, `ResponseProcessor.ts`, possibly `prompts.ts`, `ActiveSession` (remove counter field). No schema changes.
|
||
|
||
---
|
||
|
||
## Phase 4 — Delete `SearchManager` pass-throughs
|
||
|
||
**Outcome**: HTTP route → `SearchOrchestrator` directly. `SearchManager` shrinks to the display-wrap only.
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.6
|
||
- Verified-finding V11
|
||
- `src/services/worker/SearchManager.ts` (2069 lines) and `src/services/worker/http/routes/SearchRoutes.ts`
|
||
|
||
### Tasks
|
||
|
||
1. **Route rewire**: `SearchRoutes.ts` handlers call `SearchOrchestrator.search(params)` directly for structured results, then `renderSearchResults(results, format)` (new small helper extracted from current SearchManager) for markdown.
|
||
2. **Delete from `SearchManager.ts`**:
|
||
- `queryChroma` (`:59`, `@deprecated`) — delete all call sites first (grep).
|
||
- `searchChromaForTimeline` (`:70`) — delete.
|
||
- Any method whose body is `return this.orchestrator.foo(...)` with no other work.
|
||
3. **Keep** the result-combining / grouping / markdown-table code in `SearchManager.search()` as a `renderSearchResults(results, opts)` module. This is real work (V11). Put it in `src/services/worker/search/ResultRenderer.ts` if not already there.
|
||
4. **Delete** `filterByRecency` default 90-day filter. Callers pass `dateRange` explicitly.
|
||
|
||
### Verification
|
||
|
||
- [ ] `grep -n "class SearchManager" src/` → file either deleted or reduced to < 200 lines of display logic.
|
||
- [ ] `grep -n "queryChroma\|searchChromaForTimeline" src/` → zero.
|
||
- [ ] `grep -n "filterByRecency" src/` → zero.
|
||
- [ ] Integration: `curl '/api/search?q=test&project=cm&format=markdown'` and `format=json` — both return expected shapes.
|
||
|
||
### Anti-pattern guards
|
||
|
||
- D: A method that forwards must die.
|
||
- C: If Chroma is disabled and `q` is set, return 503 with `error: 'chroma_unavailable'` — don't silently run a SQLite fallback.
|
||
|
||
### Blast radius
|
||
|
||
`SearchManager.ts`, `SearchRoutes.ts`, new `ResultRenderer.ts`. No schema changes.
|
||
|
||
---
|
||
|
||
## Phase 5 — Delete worker `ProcessRegistry` facade
|
||
|
||
**Outcome**: Worker talks to `src/supervisor/process-registry.ts` directly. `src/services/worker/ProcessRegistry.ts` becomes a small module of free functions for spawning and SIGTERM→SIGKILL escalation (not a registry).
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.8, Part 2 Decision D3
|
||
- Verified-findings V5, V6
|
||
- `src/services/worker/ProcessRegistry.ts` (527 lines), `src/supervisor/process-registry.ts` (408 lines), `src/services/worker-service.ts` (uses both)
|
||
|
||
### Tasks
|
||
|
||
1. **Audit `worker/ProcessRegistry.ts` exports** and rehome:
|
||
- `registerProcess`, `unregisterProcess`, `getProcessBySession`, `getActiveCount`, `waitForSlot`, `getActiveProcesses`, `reapOrphanedProcesses` → these wrap the supervisor's registry. Delete the worker copies; callers switch to `getSupervisor().getRegistry().foo(…)` (already what they ultimately hit).
|
||
- `ensureProcessExit` (`:185`, SIGTERM→SIGKILL escalation) → keep as a free function in a new `src/services/worker/process-control.ts`. Inline the 5-s wait + SIGKILL. Remove the ladder-framework packaging.
|
||
- `createPidCapturingSpawn` (`:393`) → move to `process-control.ts`.
|
||
- `startOrphanReaper` (`:508`) → **delete in Phase 6** (replaced by ReaperTick).
|
||
2. **Delete** `src/services/worker/ProcessRegistry.ts` when it's empty.
|
||
3. **Update all imports** (grep for `from.*worker/ProcessRegistry` and re-point).
|
||
|
||
### Verification
|
||
|
||
- [ ] `test -f src/services/worker/ProcessRegistry.ts` → false.
|
||
- [ ] `grep -rn "worker/ProcessRegistry" src/` → zero.
|
||
- [ ] All worker + tests still compile: `npx tsc --noEmit`.
|
||
- [ ] Manual test: start worker, spawn a summarize subprocess, SIGTERM it → observe SIGKILL after 5 s.
|
||
|
||
### Anti-pattern guards
|
||
|
||
- D: Do not add a "compatibility shim" that re-exports the deleted symbols.
|
||
- A: `ensureProcessExit` is five lines — don't build a class for it.
|
||
|
||
### Blast radius
|
||
|
||
Big import fan-out. Compile-time breakage until all imports are fixed. Runtime: identical behavior (supervisor registry was always the backing store).
|
||
|
||
---
|
||
|
||
## Phase 6 — `ReaperTick`: single 30-s timer with three checks
|
||
|
||
**Outcome**: One `setInterval(30_000)` in `worker-service.ts`. Three skippable checks: prune dead PIDs (every tick), kill hung generators (every 4 ticks), delete abandoned sessions (every 4 ticks). The per-claim 60-s stale reset runs once at boot instead.
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.8 subgraph `OneReaper`, Part 4 timer census
|
||
- Verified-findings V6, V19
|
||
- Phase 5 **MUST** be done.
|
||
|
||
### Tasks
|
||
|
||
1. **Create `src/services/worker/reaper.ts`**:
|
||
```ts
|
||
export function startReaperTick(deps: {
|
||
processRegistry: ProcessRegistry;
|
||
sessionManager: SessionManager;
|
||
pendingStore: PendingMessageStore;
|
||
thresholds?: { generatorIdleMs?: number; sessionIdleMs?: number };
|
||
}): { stop(): void };
|
||
```
|
||
Internally: tick counter, `reapDeadPids()` every tick, `reapHungGenerators()` + `reapAbandonedSessions()` every 4 ticks. Thresholds: `generatorIdleMs=5*60_000`, `sessionIdleMs=15*60_000`.
|
||
2. **Delete `startOrphanReaper`** (`ProcessRegistry.ts:508`) and `staleSessionReaperInterval` (`worker-service.ts:547`). Delete `reapOrphanedProcesses`, `killSystemOrphans`, `killIdleDaemonChildren` as separate functions; fold their bodies into `reapDeadPids`.
|
||
3. **Move `PendingMessageStore.claimNextMessage`** stale reset from inside the claim (lines `:99–145`) into a new `PendingMessageStore.recoverStuckProcessing()` method called once at worker boot in `worker-service.ts` after the DB is ready. The claim becomes a clean `SELECT ... LIMIT 1 FOR UPDATE`-equivalent transaction.
|
||
4. **Update `worker-service.ts`** shutdown path to `stop()` the ReaperTick before orphan reaper (it's the same thing now).
|
||
|
||
### Verification
|
||
|
||
- [ ] `grep -n "setInterval" src/services/worker*/` → exactly one call (inside `reaper.ts`).
|
||
- [ ] `grep -n "staleSessionReaperInterval\|startOrphanReaper" src/` → zero.
|
||
- [ ] `grep -A3 "STALE_PROCESSING_THRESHOLD_MS" src/services/sqlite/PendingMessageStore.ts` → threshold used only in `recoverStuckProcessing`.
|
||
- [ ] Integration test: kill the SDK subprocess for a running session; within 30 s the ProcessRegistry has unregistered and SessionManager entry is gone.
|
||
- [ ] Boot recovery test: insert `pending_messages` row with `status=processing, started_processing_at_epoch=epoch-2hr`; start worker; assert row flipped back to `pending` within boot.
|
||
|
||
### Anti-pattern guards
|
||
|
||
- B: No polling loops. `claimNextMessage` must not do self-healing on each call.
|
||
- A: No `Reaper` class unless a second state ever has to live there. Start as a function.
|
||
|
||
### Blast radius
|
||
|
||
Worker lifecycle + SQLite claim path. Risk: reaper timing regression. Mitigation: keep the three thresholds identical to today.
|
||
|
||
---
|
||
|
||
## Phase 7 — Transcript watcher cleanup
|
||
|
||
**Outcome**: `fs.watch(parent_dir, {recursive: true})` instead of 5-s rescan. No `pendingTools` state map (match by `tool_use_id` at line boundary). Direct `ingestObservation` call; no HTTP loopback from inside worker.
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.12
|
||
- Verified-finding V18
|
||
- Phases 2, 5, 6 **MUST** be done.
|
||
|
||
### Tasks
|
||
|
||
1. **Rewrite `src/services/transcripts/watcher.ts`**:
|
||
- Replace periodic rescan (`setInterval(… 5000)`) with `fs.watch(parentDir, { recursive: true }, onFileEvent)`. Handle `rename` events to add new files, `change` events to tail existing ones.
|
||
- Delete `rescanIntervalMs` config option and the watcher-internal timer.
|
||
2. **Rewrite `src/services/transcripts/processor.ts`**:
|
||
- Remove `pendingTools: Map<string, {name?, input?}>` from `SessionState`.
|
||
- When a JSONL line is a `tool_use` → enqueue into a per-file map keyed by `tool_use_id`. When a later line is a `tool_result` with the same `tool_use_id`, emit one `IngestObservationPayload` and drop the entry. If a tool_use has no tool_result after N lines (say, 10 MB of JSONL read), timeout-log and drop.
|
||
3. **Replace HTTP loopback** with `import { ingestObservation } from '…/worker/ingest'` and direct call.
|
||
4. **Project-exclusion**: let `ingestObservation` handle it; remove the re-check in the transcript processor.
|
||
|
||
### Verification
|
||
|
||
- [ ] `grep -n "setInterval" src/services/transcripts/` → zero.
|
||
- [ ] `grep -n "pendingTools" src/` → zero.
|
||
- [ ] `grep -n "workerHttpRequest" src/services/transcripts/ src/cli/handlers/observation.ts` → count ≥ 0 (CLI handler can still HTTP the worker; only the *in-process* loopback is forbidden).
|
||
- [ ] Integration: drop a new Cursor transcript file into the watched dir; within 1 s a `pending_messages` row appears.
|
||
|
||
### Anti-pattern guards
|
||
|
||
- B: No fallback polling "in case fs.watch misses an event". Parent-recursive watch is the contract.
|
||
- E: The transcript ingest path and the hook ingest path both call `ingestObservation`. One function, two callers.
|
||
|
||
### Blast radius
|
||
|
||
Transcript watcher only. Kept user-facing: Cursor, OpenCode, Gemini-CLI JSONL ingest still works.
|
||
|
||
---
|
||
|
||
## Phase 8 — Unified `renderObservations(obs, strategy)`
|
||
|
||
**Outcome**: One traversal, four strategy configs. `AgentFormatter`, `HumanFormatter`, `ResultFormatter`, and `CorpusRenderer` become strategy definitions that plug into the single renderer.
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.5 (context-injection) + Part 2 Decision D4
|
||
- Files: `src/services/context/formatters/{AgentFormatter,HumanFormatter}.ts`, `src/services/worker/search/ResultFormatter.ts`, `src/services/worker/knowledge/CorpusRenderer.ts`, all section renderers under `src/services/context/sections/`
|
||
|
||
### Tasks
|
||
|
||
1. **Design the renderer contract** in `src/services/rendering/renderObservations.ts`:
|
||
```ts
|
||
export interface RenderStrategy {
|
||
name: 'agent' | 'human' | 'search' | 'corpus';
|
||
columns: Array<'title'|'narrative'|'facts'|'file'|'date'|'session'|'tokens'>;
|
||
density: 'compact' | 'normal' | 'verbose';
|
||
grouping?: 'none' | 'by-day' | 'by-file' | 'by-session';
|
||
colorize?: boolean; // terminal ANSI
|
||
tokenBudget?: number;
|
||
}
|
||
export function renderObservations(obs: Observation[], strategy: RenderStrategy): string;
|
||
```
|
||
2. **Replace** each of the four formatters with a `RenderStrategy` object (e.g., `AgentContextStrategy`, `HumanContextStrategy`, `SearchResultStrategy`, `CorpusDetailStrategy`). The strategies live in their respective feature folders; the renderer is shared.
|
||
3. **Move one-off logic** (ANSI coloring, token budgeting, day-grouping) from the four formatters into the renderer, gated by strategy flags.
|
||
4. **Keep** mode filtering + section ordering in the *builder* (`ContextBuilder`) — only the final render step unifies.
|
||
|
||
### Verification
|
||
|
||
- [ ] `grep -rn "formatObservation\|renderObservation" src/ | wc -l` — one shared renderer, four strategy files.
|
||
- [ ] Snapshot tests: for each strategy, feed the same fixture `Observation[]` and assert output is byte-equal to the old formatter's output.
|
||
- [ ] `npm run build-and-sync` + SessionStart injects a context block identical to pre-refactor bytes (modulo strategy-flagged differences).
|
||
|
||
### Anti-pattern guards
|
||
|
||
- E: No "almost the same" paths remain. All four formatters end up as thin `export const FooStrategy: RenderStrategy = …` files.
|
||
- A: No `RendererFactory`. The renderer is a pure function.
|
||
|
||
### Blast radius
|
||
|
||
Pure code reorganization, lowest risk. Snapshot tests are the safety net.
|
||
|
||
---
|
||
|
||
## Phase 9 — SQLite consolidation
|
||
|
||
**Outcome**: Fresh DBs use `schema.sql` (current state). Upgrade-only migrations run for old DBs. `UNIQUE(session_id, tool_use_id)` added. 30-s content-hash dedup window removed. Python repair script gone; user-facing `claude-mem repair` command added.
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.3, Part 5 ledger rows for SQLite
|
||
- Verified-findings V12, V13, V14
|
||
- `src/services/sqlite/migrations/runner.ts`, `src/services/sqlite/observations/store.ts`, `tests/services/sqlite/schema-repair.test.ts`
|
||
|
||
### Tasks
|
||
|
||
1. **Add `observations.tool_use_id` column** in a new migration (if not already there — grep the schema). Add `UNIQUE(session_id, tool_use_id)` constraint. For observations without a `tool_use_id` (legacy rows), set a synthetic value like `legacy:<id>` so the UNIQUE doesn't collide.
|
||
2. **Rewrite `observations/store.ts`**:
|
||
- Use `INSERT ... ON CONFLICT (session_id, tool_use_id) DO NOTHING RETURNING id`.
|
||
- On conflict, re-SELECT the existing row and return its `id`. Idempotent.
|
||
- Delete `DEDUP_WINDOW_MS`, `findDuplicateObservation`, and the content-hash dedup query. **Keep** the `content_hash` column — it's useful for cross-machine dedup analytics; just don't use it as a dedup gate.
|
||
3. **Create `src/services/sqlite/schema.sql`** with the current schema. On fresh DB, run `schema.sql` then write `schema_versions` row at current version. On existing DB, skip `schema.sql` and run only migrations with `version > max(schema_versions.version)`.
|
||
4. **Delete the Python repair path** (`execSync('python3 …')`). Add a new CLI subcommand `claude-mem repair` that runs the Python script on demand — this is for users who hit corruption from v<X. Document in a new `docs/public/troubleshooting/repair.mdx` page.
|
||
5. **Consolidate migration boilerplate**. 22+ migrations with `CREATE TABLE IF NOT EXISTS` patterns become: `schema.sql` covers everything; remaining upgrade migrations only do `ALTER TABLE` / `CREATE INDEX IF NOT EXISTS` / data migrations.
|
||
|
||
### Verification
|
||
|
||
- [ ] Fresh-install test: delete `~/.claude-mem/claude-mem.db`; start worker; assert `schema_versions.version = N` and all expected tables exist.
|
||
- [ ] Upgrade test: start worker on an old DB from v6.0; assert all migrations run and the final schema matches `schema.sql`.
|
||
- [ ] Dup test: insert two `observations` rows with the same `(session_id, tool_use_id)`; assert second INSERT returns the first row's id and no duplicate row exists.
|
||
- [ ] `grep -n "execSync.*python" src/` → zero.
|
||
- [ ] `claude-mem repair` command executes without error on a known-corrupt DB fixture.
|
||
|
||
### Anti-pattern guards
|
||
|
||
- A: No "schema migration framework". bun:sqlite + a `schema_versions` table + a list of migration functions is enough.
|
||
- E: Don't keep both content-hash dedup and UNIQUE(session_id, tool_use_id) as two gates. Pick one (the constraint).
|
||
|
||
### Blast radius
|
||
|
||
Highest-risk migration in the plan. Requires backfill of `tool_use_id` for rows that don't have it. Run in a staged release with the `claude-mem repair` fallback.
|
||
|
||
---
|
||
|
||
## Phase 10 — Chroma rewrite
|
||
|
||
**Outcome**: One doc per observation (title + narrative + facts concatenated). Stable ID `obs:<sqlite_rowid>`. Upsert instead of delete-then-add. `chroma_synced` boolean column on `observations`; backfill only rows where the flag is false. Full-project scan on boot deleted.
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.4
|
||
- Verified-findings V15, V16, V17
|
||
- `src/services/sync/ChromaSync.ts:125–545`
|
||
- Phase 9 **MUST** be done (so `chroma_synced` migration can land alongside).
|
||
|
||
### Tasks
|
||
|
||
1. **Migration**: add `chroma_synced INTEGER DEFAULT 0` column to `observations` and `session_summaries`.
|
||
2. **Rewrite `ChromaSync.formatObservationAsDoc`**: one doc per observation. Text = `title + "\n\n" + narrative + "\n\n" + facts.join("\n")`. ID = `obs:${sqliteRowId}`. Metadata keeps project, session_id, timestamp, type. Same for summaries (one doc, stable ID).
|
||
3. **Replace `chromaSync.syncObservation`** write path: `chroma_mcp.upsert(id, text, metadata)`. On success: `UPDATE observations SET chroma_synced=1 WHERE id=?`. On failure: `logger.warn`, leave flag 0.
|
||
4. **Replace `ensureBackfilled` + `runBackfillPipeline` + `getExistingChromaIds`** with a simple `backfillUnsynced(limit=1000)` called **once at boot**. Query: `SELECT id FROM observations WHERE chroma_synced=0 LIMIT 1000`. For each: format → upsert → mark.
|
||
5. **Delete** `backfillAllProjects` (static), `ensureBackfilled`, `runBackfillPipeline`, `getExistingChromaIds`, `formatObservationsAsDocs`, `formatSummariesAsDocs` (multi-doc), and the delete-then-add conflict handler.
|
||
|
||
### Verification
|
||
|
||
- [ ] Chroma index contains one doc per observation (not 7). Query Chroma directly: `chroma_count_documents(collection)` = `SELECT COUNT(*) FROM observations WHERE chroma_synced=1`.
|
||
- [ ] Idempotent re-sync: call `syncObservation` twice with same ID; assert no conflict, one doc.
|
||
- [ ] Boot with Chroma down: observations sync'd to SQLite normally, `chroma_synced=0`. Start Chroma, restart worker: those rows upserted within boot.
|
||
- [ ] `grep -n "backfillAllProjects\|ensureBackfilled\|getExistingChromaIds" src/` → zero.
|
||
|
||
### Anti-pattern guards
|
||
|
||
- C: On Chroma failure at write time, do **not** throw — leave flag 0 and move on. The backfill path covers recovery.
|
||
- A: No `ChromaBackfillScheduler`. One function, called at boot, done.
|
||
|
||
### Blast radius
|
||
|
||
Chroma index regenerates under the new doc shape. Users see the old index until the first boot-time backfill completes (may take minutes on large corpora).
|
||
|
||
---
|
||
|
||
## Phase 11 — Endpoint consolidation
|
||
|
||
**Outcome**: 10 session endpoints → 4. `/api/session/start` returns context + semantic in one call. `/api/session/end` blocks until summary written or 110-s timeout (no hook-side polling). `/api/context/inject` + `/api/context/semantic` deleted or folded.
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.1, section 3.9 (Routes inventory), Part 2 Decision D6
|
||
- Verified-findings V8, V9, V10
|
||
- `src/services/worker/http/routes/SessionRoutes.ts`, `src/services/worker/http/routes/SearchRoutes.ts`, `src/cli/handlers/{context,user-message,summarize,session-complete}.ts`
|
||
|
||
### Tasks
|
||
|
||
1. **New endpoints** (4 total):
|
||
- `POST /api/session/start` — body: `{project, claudeSessionId}`. Returns `{sessionDbId, contextMarkdown, semanticMarkdown}`. Internally: calls `ContextBuilder.generateContext` + `SearchOrchestrator.search`.
|
||
- `POST /api/session/prompt` — body: `{sessionDbId, prompt}`. Returns `{promptId}`.
|
||
- `POST /api/session/observation` — body: `{sessionDbId, tool_use_id, name, input, output}`. Returns `{observationId|null, skipped}`.
|
||
- `POST /api/session/end` — body: `{sessionDbId, last_assistant_message}`. **Blocks** until the queue is drained and the summary row is written (or 110-s timeout). Returns `{summaryId|null}`.
|
||
2. **Blocking `/api/session/end`**: implement via a per-session `Deferred<SummaryResult>`. When `ResponseProcessor` writes the summary row, resolve the deferred. Route handler `await`s the promise with a 110-s race.
|
||
3. **Delete the old 10 endpoints** under `/sessions/:sessionDbId/*` and `/api/sessions/*` after all hook-side callers are switched. Also delete `/api/context/inject` and `/api/context/semantic`.
|
||
4. **Rewrite hook handlers** (`context.ts`, `user-message.ts`, `summarize.ts`, `session-complete.ts`) to use the 4 new endpoints. Delete the 500-ms polling loop in `summarize.ts:117–150`.
|
||
5. **Hook-side `ensureWorkerRunning` cache**: create `src/hooks/worker-cache.ts` that caches `alive=true` in module scope for the hook process. First call spawns/HTTPs `/health`; subsequent calls skip. Switch all 8 handlers to import from this module.
|
||
|
||
### Verification
|
||
|
||
- [ ] `grep -n "router\.\(get\|post\|delete\)" src/services/worker/http/routes/SessionRoutes.ts` → 4 routes.
|
||
- [ ] `grep -n "/api/context/inject\|/api/context/semantic" src/` → zero.
|
||
- [ ] `grep -n "POLL_INTERVAL_MS\|MAX_WAIT_FOR_SUMMARY_MS" src/cli/handlers/` → zero.
|
||
- [ ] Integration: run a full session lifecycle; assert Stop hook returns within ~110 s (or earlier) with a `summaryId`, and no /status polling requests hit the worker.
|
||
- [ ] Perf: SessionStart latency ≤ previous latency (one request vs two).
|
||
|
||
### Anti-pattern guards
|
||
|
||
- B: No polling. Blocking + timeout replaces it.
|
||
- D: `/api/session/start` must not be a facade over `/api/context/inject`; the old endpoints are deleted.
|
||
|
||
### Blast radius
|
||
|
||
Hook ↔ worker HTTP contract changes. Needs coordinated plugin rebuild (`npm run build-and-sync`). Old hooks calling old endpoints will 404 — land after a version bump.
|
||
|
||
---
|
||
|
||
## Phase 12 — Zod validator middleware
|
||
|
||
**Outcome**: Per-route Zod schema + one `validateBody(schema)` middleware. Per-route hand-rolled validation gone.
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.9
|
||
- `src/services/worker/http/routes/*.ts` (8 files with inline validation)
|
||
|
||
### Tasks
|
||
|
||
1. **Add `zod`** to `package.json` dependencies (confirm not already present; if it is, skip).
|
||
2. **Create `src/services/worker/http/middleware/validateBody.ts`**:
|
||
```ts
|
||
export function validateBody<T>(schema: z.ZodType<T>): RequestHandler { … }
|
||
```
|
||
On parse failure: `res.status(400).json({ error: 'validation_failed', fields: result.error.flatten() })`.
|
||
3. **Per-route schemas** in a parallel `schemas/` directory (or inline at top of each route file). One `z.object({…})` per endpoint.
|
||
4. **Delete** per-route boilerplate: manual `typeof x !== 'string'` checks, `if (!body.foo) return res.status(400)…`.
|
||
|
||
### Verification
|
||
|
||
- [ ] `grep -n "res.status(400)" src/services/worker/http/routes/ | wc -l` significantly reduced (only routes that return 400 for domain reasons, not shape validation).
|
||
- [ ] Error-shape tests: each endpoint, with invalid body, returns `{error, message, code, fields}`.
|
||
- [ ] No behavioral regression on happy path (snapshot test of responses).
|
||
|
||
### Anti-pattern guards
|
||
|
||
- A: Don't invent `ZodUtil.assertBody` — use `safeParse` directly.
|
||
- E: Single middleware, not one per route.
|
||
|
||
### Blast radius
|
||
|
||
HTTP error shape might change slightly (field names in 400s). Client (viewer UI) must tolerate `fields` key.
|
||
|
||
---
|
||
|
||
## Phase 13 — KnowledgeAgent simplification
|
||
|
||
**Outcome**: No `session_id` persistence in `corpus.json`. No `prime` endpoint. No auto-reprime regex. `build` IS prime; every `query` loads the corpus fresh as system prompt.
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.11
|
||
- `src/services/worker/knowledge/KnowledgeAgent.ts`, `CorpusStore.ts`, `CorpusBuilder.ts`, corresponding routes in `CorpusRoutes.ts`
|
||
|
||
### Tasks
|
||
|
||
1. **Delete** `KnowledgeAgent.prime` and the `reprime` endpoint. Update the OpenAPI/route table to drop them.
|
||
2. **Simplify `CorpusStore`**: corpus JSON contains `{name, filters, renderedCorpus, generatedAt}`. No `session_id`.
|
||
3. **Rewrite `KnowledgeAgent.query`** to always pass `systemPrompt = renderedCorpus` to the SDK. Claude prompt-caching reduces cost when the same corpus is queried repeatedly within the 5-min TTL.
|
||
4. **Delete** the session-expiration regex match and auto-reprime path.
|
||
|
||
### Verification
|
||
|
||
- [ ] `grep -n "session_id" src/services/worker/knowledge/` → zero.
|
||
- [ ] `grep -n "reprime\|auto.*reprime" src/` → zero.
|
||
- [ ] Cost test: query the same corpus 3× within 5 min; assert cache hits (the SDK returns `cache_read_input_tokens > 0`).
|
||
- [ ] `POST /api/corpus/:name/rebuild` still works; `POST /api/corpus/:name/prime` returns 404.
|
||
|
||
### Anti-pattern guards
|
||
|
||
- C: Don't try to "detect session expiration". Always pass fresh system prompt; let the SDK cache decide.
|
||
|
||
### Blast radius
|
||
|
||
Corpus JSON format changes (drops `session_id`). Existing corpora still load (extra field ignored or migrated on read).
|
||
|
||
---
|
||
|
||
## Phase 14 — HTTP cleanup
|
||
|
||
**Outcome**: Rate limiter deleted. Static file reads cached at boot.
|
||
|
||
### Context this phase needs
|
||
|
||
- `05-clean-flowcharts.md` section 3.9
|
||
- Verified-finding V20
|
||
- `src/services/worker/http/middleware.ts:45–79`, `ViewerRoutes.ts`
|
||
|
||
### Tasks
|
||
|
||
1. **Delete `src/services/worker/http/middleware.ts:45–79`** (the rate limiter) and its registration in `Middleware.ts`.
|
||
2. **Cache `viewer.html`** and `/api/instructions` content in memory at boot; serve from `Buffer` instead of `fs.readFile`.
|
||
3. **Delete** the legacy `SessionRoutes.handleObservations` no-privacy-strip endpoint (already handled in Phase 2 if the route is rewired; this is the cleanup pass).
|
||
|
||
### Verification
|
||
|
||
- [ ] `grep -n "RATE_LIMIT_WINDOW_MS\|RATE_LIMIT_MAX_REQUESTS" src/` → zero.
|
||
- [ ] Boot time: `viewer.html` hits don't cause `fs.readFile` calls (measure with lsof or a log statement).
|
||
|
||
### Anti-pattern guards
|
||
|
||
- B: Don't re-introduce the rate limiter as a "config flag". Localhost trust model is explicit.
|
||
|
||
### Blast radius
|
||
|
||
Minimal. The rate limiter was theater on a localhost server.
|
||
|
||
---
|
||
|
||
## Phase 15 — Final verification
|
||
|
||
**Outcome**: Whole system behaves per the clean flowcharts. Timer census reads 1 repeating timer. No polling loops. No silent fallbacks. Deleted-lines counter ≥ 2500 net.
|
||
|
||
### Tasks
|
||
|
||
1. **Run the timer census**:
|
||
```
|
||
grep -rn "setInterval\|setTimeout.*recursive\|setTimeout.*repeat" src/ | grep -v test
|
||
```
|
||
Expected: one `setInterval` in `reaper.ts`; one per-session idle timeout; one EventSource reconnect (UI); no others. Compare against `05-clean-flowcharts.md` Part 4.
|
||
2. **Anti-pattern grep pass**:
|
||
- `grep -rn "coerceObservationToSummary\|consecutiveSummaryFailures\|DEDUP_WINDOW_MS\|STALE_PROCESSING_THRESHOLD_MS.*claimNextMessage\|backfillAllProjects\|getExistingChromaIds\|stripMemoryTagsFromJson\|stripMemoryTagsFromPrompt\|POLL_INTERVAL_MS" src/` → zero matches.
|
||
- `grep -rn "res.status(503)" src/` includes `chroma_unavailable` path (positive check).
|
||
3. **Deleted-lines count**: `git diff main --stat | tail -1` — compare against the audit's Part 5 estimate (~2500 net).
|
||
4. **Run full test suite**: `npm test`.
|
||
5. **Run plugin end-to-end**: `npm run build-and-sync` → trigger all 5 lifecycle hooks in a real Claude Code session → verify SSE events, viewer UI renders, search works, corpus builds + queries, transcript watcher picks up a synthetic Cursor log.
|
||
6. **Document**: update `docs/public/architecture.mdx` (or equivalent) to point at `05-clean-flowcharts.md` as the canonical architecture doc.
|
||
|
||
### Verification
|
||
|
||
- [ ] Timer census matches `05-clean-flowcharts.md` Part 4 "after" column.
|
||
- [ ] All grep anti-pattern checks return zero matches.
|
||
- [ ] Full test suite green.
|
||
- [ ] End-to-end plugin test passes.
|
||
|
||
---
|
||
|
||
## Phase dependency graph
|
||
|
||
```
|
||
P1 ─┐
|
||
├─> P2 ─┬─> P3
|
||
│ ├─> P6 ─> P7
|
||
│ └─> P11
|
||
│
|
||
P4 (independent)
|
||
P5 ──> P6 (already sequenced above)
|
||
P8 (independent — can run anytime)
|
||
P9 ──> P10
|
||
P11 ──> P12 (Zod lands after endpoint shape is final)
|
||
P13 (independent)
|
||
P14 (after P11 so legacy route delete is clean)
|
||
P15 gates merge.
|
||
```
|
||
|
||
Parallelizable tracks: (P1→P2→P3), (P4), (P5→P6→P7), (P8), (P9→P10), (P13). Merge order: P1,P2,P3,P4,P5,P6,P7,P8,P9,P10,P11,P12,P13,P14,P15.
|
||
|
||
---
|
||
|
||
## Estimated effort
|
||
|
||
Per `05-clean-flowcharts.md` Part 6: ~18 engineer-days for full clean-through. Phase 1 alone closes the P1 security gap (<1 day).
|
||
|
||
## Success criteria
|
||
|
||
- One `setInterval` in the worker codebase.
|
||
- Zero polling loops on the hook side.
|
||
- 40 bullshit items from `05-clean-flowcharts.md` Part 1 all deleted (verified by grep).
|
||
- All 12 user-facing features from Pathfinder Phase 0 still work.
|
||
- Net LOC deleted ≥ 1800.
|