# v1.7.4 B3 Follow-ups Implementation Plan

> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.

**Goal:** Close 3 of the 5 B3 dlPFC follow-up items deferred from v0.39.0: lift the goal-stack boost from CLI-only into `api.recall` so MCP/REST callers get it; add `--no-propagate` flag on `goal complete`; refactor depth-cap duplication. Internal hygiene release.

**Architecture (revised post outside-voice review, 2026-05-07):** Task 1 lifts the existing ~150-line CLI goal-boost block into a reusable `applyGoalStackBoost` helper exported from `src/goals.ts`, operating on entry-backed scored rows BEFORE any `RecallResultItem` projection. The helper is called from THREE sites: (a) `cmdRecall` (CLI, the existing caller), (b) `api.recall`'s primary BM25 band (new — appendix rows are NOT boosted to preserve fresh-tail / summary-substitution placement), and (c) MCP `hippo_recall`'s physics/hybrid result list before `formatMemories` (new — MCP's user-visible primary ordering does NOT come from `api.recall`). Session id is plumbed via a NEW `RecallOpts.sessionId?: string` field — NOT `Context.sessionId` — because `Context` is shared by remember/recall/assemble/outcome and only recall consumes the boost. `RecallOpts.goalTag?: string` is also added so the explicit-goal-tag override (today CLI-only via the `goalTag === ''` gate) is expressible on MCP/HTTP. Task 2 adds `noPropagate?: boolean` to `CompleteGoalOpts` and a `--no-propagate` CLI flag. Task 3 extracts the depth-cap SQL into a private `enforceDepthCapWithinTxWithinTx` helper (name explicit about its transactional precondition).

**Tech Stack:** TypeScript 5, vitest, better-sqlite3 (real DB).

**Branch:** `feat/v1.7.4-b3-followups` off `master`. **Do not commit on master.**

**Source items:** `C:\Users\skf_s\hippo\TODOS.md` lines 145-160 (`v0.39.0 — B3 dlPFC depth follow-ups`). 3 of 5 items shipped here. Remaining 2: sequential-learning adapter contract → v1.7.5; vlPFC interference → v1.8.0.

**Pre-investigated facts (anchor before any code touches; revised post-review):**

- CLI goal-boost block lives at `src/cli.ts:988-1140` (~150 lines). It:
  1. Reads `sessionId` from `--session-id` flag or `HIPPO_SESSION_ID` env (line 1004-1008)
  2. Calls `getActiveGoalsWithDb`
  3. Boosts results whose tags overlap active goal names; composes `retrieval_policy` multipliers
  4. Hard-caps at `MAX_FINAL_MULTIPLIER = 3.0` (`src/goals.ts:49`)
  5. Filters `goal_recall_log` writes to LOCAL memory ids only (`src/cli.ts:1093-1106`) — global memory ids would FK-fail
  6. Logs to `goal_recall_log` with `INSERT OR IGNORE` (`UNIQUE(memory_id, goal_id)` makes re-recall during same goal life a no-op)
  7. Reads `r.entry.tags`, `r.entry.id`, `r.entry.schema_fit` — operates on entry-backed rows, NOT `RecallResultItem`
- `Context` interface: `src/api.ts:57` — `{ hippoRoot, tenantId, actor }`. **DECISION: do NOT add `sessionId` here.** `Context` is shared by remember/recall/assemble/outcome.
- `RecallOpts`: see `src/api.ts` near `RecallContractError`. **NEW field this release:** `sessionId?: string` and `goalTag?: string`.
- `api.recall` entry point: `src/api.ts:346` — has NO goal-boost code today. Already opens a db handle for `appendAuditEvent` near line 549 — thread that handle to the boost block instead of opening a second one.
- **`api.recall` returns `RecallResultItem` (`src/api.ts:284`) — `{id, content, score, layer, strength}`.** No `entry`, no `tags`, no `schema_fit`. The boost helper MUST operate on entry-backed rows BEFORE the `RecallResultItem` projection.
- **`api.recall` appendix rows (fresh-tail + level-2 summary substitutions) have semantically special placement.** Boost the PRIMARY band only, then append fresh-tail / summary rows per existing rules, then recompute `tokens`.
- **MCP `hippo_recall`'s primary ordering** comes from `physicsSearch` / `hybridSearch` in `src/mcp/server.ts:424` — NOT `api.recall`. Lifting into `api.recall` alone leaves MCP's user-visible main result block UNBOOSTED. Helper must also be called from MCP's physics/hybrid path before `formatMemories`.
- **MCP `hippo_recall` schema does NOT have `session_id` today.** `session_id` is on `hippo_assemble` (line 200, 221). `hippo_recall` has `fresh_tail_session_id` only. **This release adds `session_id` to `hippo_recall` input schema** (parse, trim, 256-char cap mirroring `fresh_tail_session_id`).
- HTTP `/v1/memories` does NOT have `session_id` query param today. **This release adds it** (256-char cap pattern from `fresh_tail_session_id`).
- `completeGoal`: `src/goals.ts:218-275`. Propagation block at lines 245-264. Status check at 233-236 short-circuits second call BEFORE reading `opts.noPropagate` — second-call-with-noPropagate after a propagating-first-call is a true no-op (propagation already happened).
- Depth-cap duplication: `pushGoalWithDb` at `src/goals.ts:100-119`; `resumeGoal` at `src/goals.ts:299-316`. Same SQL shape. Both call sites already inside `BEGIN IMMEDIATE` — helper requires this precondition; name reflects it.
- `RetrievalPolicy` type lives in `src/goals.ts`; `src/cli.ts` imports it. Lift must keep it in `src/goals.ts` only — do not duplicate.
- `src/index.ts` does NOT re-export from `src/api.ts`. `Context` and `RecallOpts` are the public types via `api.ts`. Adding `RecallOpts.sessionId?` and `RecallOpts.goalTag?` is safely additive (optional fields).

**Pre-flight (run once before Task 1):**

```bash
git status --short
git switch -c feat/v1.7.4-b3-followups
npm install
npx vitest run --reporter=dot 2>&1 | tail -10
```

Expected: clean baseline, 1400+ tests passing. **STOP if red.**

---

## Task 1: Lift goal-stack boost from CLI into `api.recall` via `Context.sessionId`

**Why:** Today only the CLI applies the goal-stack boost. MCP `hippo_recall` and HTTP `/v1/memories` go through `api.recall` and get NO boost. Lifting the logic into a shared helper and threading `sessionId` through `Context` gives all three transports the boost without each re-implementing.

**Strategy (root-cause shape, revised post outside-voice review):**
- Extract the CLI's goal-boost block as `applyGoalStackBoost<R extends { entry: MemoryEntry; score: number }>(db, results, opts)` in `src/goals.ts`. Pure function over entry-backed scored rows. Side effect: writes to `goal_recall_log` with `INSERT OR IGNORE`, filtered to LOCAL memory ids only (preserves the existing FK-safety filter from the CLI block).
- Session id flows via NEW `RecallOpts.sessionId?: string` — NOT `Context.sessionId`. (Codex finding: `Context` is shared by remember/recall/assemble/outcome and only recall consumes the boost; keeping it on `Context` is leaky.)
- NEW `RecallOpts.goalTag?: string` — explicit-tag override. When set, the boost is suppressed (mirrors CLI's `goalTag === ''` gate today). Document precedence: explicit goal-tag wins.
- `api.recall` calls the helper on the PRIMARY BM25 band ONLY, BEFORE projection to `RecallResultItem`. Appendix rows (fresh-tail, summary substitutions) keep their semantically-special placement. Tokens recomputed after boost. Single db handle threaded through `api.recall` (reuses the existing `appendAuditEvent` handle near line 549).
- MCP `hippo_recall`'s primary ordering comes from `physicsSearch`/`hybridSearch` in `src/mcp/server.ts:424` — NOT `api.recall`. Helper called from THERE too, against the physics/hybrid result list, before `formatMemories`.
- MCP `hippo_recall` input schema gains `session_id` (does NOT have it today; was a wrong fact in the v1 plan brief). 256-char cap, trim, validate — mirror `fresh_tail_session_id`.
- HTTP `/v1/memories` adds `session_id` query param. Same 256-char cap.
- `cmdRecall` migrates from inline block to helper call. Drops ~140 lines.

**Files:**
- Modify: `src/api.ts` (around line 57 — leave `Context` alone; add `sessionId?: string` and `goalTag?: string` to `RecallOpts`)
- Modify: `src/api.ts:346` — `api.recall` calls `applyGoalStackBoost` on the primary band when `opts.sessionId` set AND `opts.goalTag` not set; threads its existing audit db handle to the helper
- Modify: `src/goals.ts` — export new `applyGoalStackBoost` helper (`@internal`); lift the CLI logic verbatim minus the goalTag gate (caller-managed) and minus environment reads (caller passes `sessionId`/`tenantId`/`limit` explicitly)
- Modify: `src/cli.ts:988-1140` — replace inline block with `applyGoalStackBoost` call; CLI stays the only consumer of `HIPPO_SESSION_ID` env
- Modify: `src/mcp/server.ts` — (a) add `session_id` field to `hippo_recall` input schema (parse, trim, 256-cap), (b) call `applyGoalStackBoost` against the physics/hybrid result list before `formatMemories` when `session_id` set
- Modify: `src/server.ts` (HTTP) — accept `session_id` query param (256-cap), pass into `RecallOpts`
- Test: `tests/goals-apply-goal-stack-boost.test.ts` (new) — 4 unit tests on the helper DIRECTLY
- Test: `tests/api-recall-goal-boost.test.ts` (new) — api.recall integration: ranking, side effects, primary-band-only
- Test: `tests/mcp-recall-goal-boost.test.ts` (new) — MCP integration: schema field accepted, physics/hybrid result list boosted
- Test: `tests/http-recall-goal-boost.test.ts` (new) — HTTP integration: query param respected, 256-cap rejected

### Step 1: Write 4 failing unit tests on the helper directly

Create `tests/goals-apply-goal-stack-boost.test.ts`. These pin the helper's behaviour independent of any transport:

```ts
/**
 * v1.7.4 — applyGoalStackBoost helper, lifted from src/cli.ts:988-1140 in
 * v0.38.0's CLI-only B3 dlPFC implementation. Pinned here at the helper
 * boundary so api.recall + MCP + HTTP integrations can build on a
 * known-correct primitive.
 */

import { describe, it, expect, beforeEach } from 'vitest';
import path from 'node:path';
import os from 'node:os';
import fs from 'node:fs';
import { initStore, openHippoDb, closeHippoDb } from '../src/db.js';
import { applyGoalStackBoost, pushGoal } from '../src/goals.js';
import type { MemoryEntry } from '../src/memory.js';

interface ScoredRow { entry: MemoryEntry; score: number; }

describe('applyGoalStackBoost (v1.7.4)', () => {
  let hippoRoot: string;
  const tenantId = 'default';
  const sessionId = 'sess-1.7.4';

  beforeEach(async () => {
    hippoRoot = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'hippo-1.7.4-'));
    initStore(hippoRoot);
  });

  function makeRow(id: string, tags: string[], score: number): ScoredRow {
    return { entry: { id, content: `c-${id}`, tags, layer: 'episodic', strength: 1.0 } as MemoryEntry, score };
  }

  it('boosts rows whose tags match an active goal name', () => {
    pushGoal(hippoRoot, { sessionId, tenantId, goalName: 'fix-auth' });
    const rows = [makeRow('m1', ['fix-auth'], 0.5), makeRow('m2', ['ui'], 0.6)];
    const db = openHippoDb(hippoRoot);
    try {
      const out = applyGoalStackBoost(db, rows, { sessionId, tenantId, limit: 10 });
      expect(out[0]?.entry.id).toBe('m1'); // boosted above m2 despite lower base score
    } finally { closeHippoDb(db); }
  });

  it('writes one goal_recall_log row per (boosted_memory, goal) — INSERT OR IGNORE on repeat', () => {
    const goal = pushGoal(hippoRoot, { sessionId, tenantId, goalName: 'fix-auth' });
    const rows = [makeRow('m1', ['fix-auth'], 0.5)];
    const db = openHippoDb(hippoRoot);
    try {
      applyGoalStackBoost(db, rows, { sessionId, tenantId, limit: 10 });
      applyGoalStackBoost(db, rows, { sessionId, tenantId, limit: 10 }); // re-recall
      const count = (db.prepare(
        `SELECT COUNT(*) AS c FROM goal_recall_log WHERE goal_id = ? AND memory_id = ?`,
      ).get(goal.id, 'm1') as { c: number }).c;
      expect(count).toBe(1); // UNIQUE(memory_id, goal_id) idempotency
    } finally { closeHippoDb(db); }
  });

  it('does NOT write goal_recall_log rows for memories absent from the local memories table (FK safety)', () => {
    pushGoal(hippoRoot, { sessionId, tenantId, goalName: 'fix-auth' });
    // m-global was never written via remember() — simulates global-only id
    const rows = [makeRow('m-global', ['fix-auth'], 0.5)];
    const db = openHippoDb(hippoRoot);
    try {
      applyGoalStackBoost(db, rows, { sessionId, tenantId, limit: 10 });
      const count = (db.prepare(
        `SELECT COUNT(*) AS c FROM goal_recall_log WHERE memory_id = 'm-global'`,
      ).get() as { c: number }).c;
      expect(count).toBe(0); // global-only id filtered before INSERT
    } finally { closeHippoDb(db); }
  });

  it('respects tenant isolation — same sessionId in tenant B does NOT load tenant A goals', () => {
    pushGoal(hippoRoot, { sessionId, tenantId: 'A', goalName: 'fix-auth' });
    const rows = [makeRow('m1', ['fix-auth'], 0.5), makeRow('m2', ['ui'], 0.6)];
    const db = openHippoDb(hippoRoot);
    try {
      const out = applyGoalStackBoost(db, rows, { sessionId, tenantId: 'B', limit: 10 });
      expect(out[0]?.entry.id).toBe('m2'); // no boost — tenant B has no active goals
    } finally { closeHippoDb(db); }
  });
});
```

Run: `npx vitest run tests/goals-apply-goal-stack-boost.test.ts`
Expected: FAIL — `applyGoalStackBoost` not exported.

### Step 2: Add `sessionId?` and `goalTag?` to `RecallOpts`

In `src/api.ts`, find the `RecallOpts` interface (near `RecallContractError`), add:

```ts
/**
 * v1.7.4 — when set AND `(ctx.tenantId, sessionId)` has active goals AND
 * `goalTag` is unset, `api.recall` applies the dlPFC goal-stack boost lifted
 * from CLI cmdRecall. Pre-v1.7.4 the boost was CLI-only (env-driven via
 * HIPPO_SESSION_ID). Undefined preserves v1.7.3 behaviour (no boost).
 *
 * Why on RecallOpts and not Context: Context is shared by remember/recall/
 * assemble/outcome. Goal-stack boost is recall-scoped only.
 */
sessionId?: string;

/**
 * v1.7.4 — explicit goal-tag override. When set, the goal-stack boost is
 * SUPPRESSED (mirrors the CLI's `goalTag === ''` gate from v0.38). Use to
 * pin recall ranking against one specific goal/tag without the multi-goal
 * stack interfering.
 */
goalTag?: string;
```

`Context` is left unchanged.

### Step 3: Extract `applyGoalStackBoost` helper into `src/goals.ts`

Lift the block at `src/cli.ts:1009-1138` into a new exported function in `src/goals.ts`. The helper operates on entry-backed rows (NOT `RecallResultItem`) so callers must pass rows that still have `entry.tags`, `entry.id`, `entry.schema_fit`. Caller is responsible for the `goalTag` gate (do not call when `goalTag` is set).

```ts
/**
 * v1.7.4 — dlPFC goal-stack boost helper. Applies the multi-goal boost to a
 * list of entry-backed scored rows when (tenant, session) has active goals.
 * Pre-v1.7.4 this logic lived inline in cmdRecall; lifting here lets
 * api.recall (primary band only) AND MCP physics/hybrid call it.
 *
 * Caller responsibilities:
 *   - Do NOT call when an explicit `goalTag` is set (caller's gate)
 *   - Pass entry-backed rows (with `entry.tags`, `entry.id`, optional `entry.schema_fit`)
 *   - Manage the db handle lifecycle (helper neither opens nor closes)
 *   - Recompute `tokens` after if returned rows are projected to a budgeted shape
 *
 * Side effects:
 *   - INSERT OR IGNORE into `goal_recall_log` for each (boosted, goal) pair
 *   - Local memory id filter applied before INSERT (skips global-only ids
 *     to preserve FK invariant on goal_recall_log.memory_id)
 *
 * @internal v1.7.4 — internal recall ranking helper. Subject to change.
 */
export function applyGoalStackBoost<R extends { entry: MemoryEntry; score: number }>(
  db: DatabaseSyncLike,
  results: R[],
  opts: {
    sessionId: string;
    tenantId: string;
    limit: number;
  },
): R[] {
  // Verbatim port of `src/cli.ts:1014-1138`:
  //   - rename `dbForGoals` → `db` (now a parameter)
  //   - replace `tenantIdForGoals` with `opts.tenantId`
  //   - replace `sessionId` (closure) with `opts.sessionId`
  //   - replace `limit` (closure) with `opts.limit`
  //   - drop the `goalTag === ''` gate at the top of the lifted block (caller-managed)
  //   - keep the local-id filter at lines 1093-1106 verbatim
  // The body is mechanical — no logic change.
}
```

`RetrievalPolicy` type stays in `src/goals.ts`. After lift, audit `src/cli.ts` to confirm it imports `RetrievalPolicy` from `src/goals.ts` (single home) — if duplicated, delete the duplicate.

### Step 4: Wire helper into `api.recall` (primary band, single handle)

Locate `api.recall` in `src/api.ts:346` and find the existing audit-event db handle (around line 549). Thread it. Boost the PRIMARY BM25 band BEFORE projection to `RecallResultItem`, BEFORE fresh-tail / summary appendix is appended:

```ts
// After the primary BM25 / hybrid scorer fills `baseEntries` (entry-backed scored rows)
// and BEFORE the appendix paths (fresh-tail, summarize-overflow) modify the result set:
if (opts.sessionId && !opts.goalTag) {
  baseEntries = applyGoalStackBoost(db, baseEntries, {
    sessionId: opts.sessionId,
    tenantId: ctx.tenantId,
    limit: opts.limit ?? DEFAULT_RECALL_LIMIT,
  });
}
// ... existing appendix logic appends fresh-tail + summary substitutions ...
// ... then project to RecallResultItem and recompute `tokens` ...
```

Pre-flight: read the current `api.recall` flow to confirm the entry-backed row variable name (likely `baseEntries` or similar) and the appendix insertion point. Adjust the code above to match the actual variable + insertion point. If the existing handle isn't open at the right point, refactor to keep one open from the audit event point through the boost.

### Step 5: Migrate `cmdRecall` to call the helper

Replace `src/cli.ts:988-1140` with:

```ts
const sessionId = (
  flags['session-id'] !== undefined
    ? String(flags['session-id'])
    : process.env.HIPPO_SESSION_ID ?? ''
).trim();
if (sessionId && goalTag === '') {
  const dbForGoals = openHippoDb(hippoRoot);
  try {
    results = applyGoalStackBoost(dbForGoals, results, { sessionId, tenantId, limit });
  } finally {
    closeHippoDb(dbForGoals);
  }
}
```

CLI keeps its env-driven path (`HIPPO_SESSION_ID`) — this is the operator-local convenience that doesn't go through `api.recall`. ~140 lines drop. All existing CLI tests must still pass.

### Step 6: MCP — add `session_id` to `hippo_recall` schema AND boost the physics/hybrid result list

`hippo_recall`'s primary ordering does NOT come from `api.recall` — it comes from `physicsSearch`/`hybridSearch` in `src/mcp/server.ts:424`. Two changes here:

(a) Extend `hippo_recall` input schema (around line 175-200) with `session_id`:

```ts
session_id: {
  type: 'string',
  maxLength: 256,
  description: 'Optional session id. When set AND (tenant, session) has active goals, applies the dlPFC goal-stack boost to the primary result band before formatting.',
},
```

(b) In the `hippo_recall` handler around line 424, after `physicsSearch`/`hybridSearch` returns its result list, BEFORE `formatMemories`, call:

```ts
const sessionIdRaw = typeof args.session_id === 'string' ? args.session_id.trim() : '';
if (sessionIdRaw && sessionIdRaw.length > 0 && sessionIdRaw.length <= 256) {
  const dbForBoost = openHippoDb(hippoRoot);
  try {
    physicsResults = applyGoalStackBoost(dbForBoost, physicsResults, {
      sessionId: sessionIdRaw,
      tenantId: apiCtx.tenantId,
      limit: budget,
    });
  } finally {
    closeHippoDb(dbForBoost);
  }
}
```

Adjust the variable name `physicsResults` to whatever the actual `hippo_recall` handler holds. Caller still also passes `sessionId` into `RecallOpts` for `apiRecall` so the audit / continuity / appendix paths see it consistently.

Pre-flight grep before code: `grep -n "physicsSearch\|hybridSearch\|formatMemories" src/mcp/server.ts` to confirm exact call sites.

### Step 7: HTTP — add `session_id` query param to `/v1/memories`

In `src/server.ts` GET `/v1/memories` handler (near line 469-501), add:

```ts
const sessionIdRaw = query.get('session_id');
if (sessionIdRaw !== null && sessionIdRaw.length > 256) {
  throw new HttpError(400, 'session_id exceeds 256-character cap');
}
const sessionId = sessionIdRaw && sessionIdRaw.trim().length > 0 ? sessionIdRaw.trim() : undefined;
// pass into RecallOpts
const recallOpts: RecallOpts = { ...existingOpts, sessionId };
```

### Step 8: Run all tests

```bash
npx vitest run --reporter=dot 2>&1 | tail -10
npx tsc --noEmit
```

Expected: full suite green, zero TS errors. Existing CLI goal-boost tests must still pass (Step 5 migration is verbatim).

### Step 9: Add api/MCP/HTTP integration tests

Three test files driving the boost end-to-end through each transport. Each should at minimum assert: (a) the goal-tagged memory ranks first when `sessionId` set, (b) `goal_recall_log` row written, (c) explicit-tag override (`goalTag` on api/HTTP, or appropriate MCP equivalent) suppresses the boost. HTTP test additionally pins 256-cap rejection.

### Step 10: Commit

```bash
git add src/api.ts src/goals.ts src/cli.ts src/mcp/server.ts src/server.ts \
  tests/goals-apply-goal-stack-boost.test.ts \
  tests/api-recall-goal-boost.test.ts \
  tests/mcp-recall-goal-boost.test.ts \
  tests/http-recall-goal-boost.test.ts
git commit -m "feat: lift goal-stack boost into api.recall + MCP physics path via RecallOpts.sessionId (v1.7.4)"
```

**Success criteria:**
- `RecallOpts.sessionId?: string` and `RecallOpts.goalTag?: string` added with JSDoc. `Context` unchanged.
- `applyGoalStackBoost` exported from `src/goals.ts`, `@internal`, NOT re-exported from `src/index.ts`. Operates on entry-backed rows.
- `cmdRecall` shrinks by ~140 lines via helper call. CLI env path (`HIPPO_SESSION_ID`) preserved. CLI tests still green.
- `api.recall` boosts the primary BM25 band BEFORE appendix; appendix placement preserved; tokens recomputed correctly.
- `api.recall` uses ONE db handle (the existing audit handle), not a second short-lived one.
- MCP `hippo_recall` schema accepts `session_id`. Boost applied to physics/hybrid result list before `formatMemories`.
- HTTP `/v1/memories?session_id=...` accepted with 256-cap. Rejected when over-length.
- 4 helper unit tests + 3 transport integration tests all pass. Full suite green.
- One atomic commit.

---

## Task 2: `--no-propagate` flag on `goal complete`

**Why:** Some users want to close a goal without propagating strength multipliers to recalled memories. Default stays propagate (current behaviour); flag opts out.

**Files:**
- Modify: `src/goals.ts` — `CompleteGoalOpts` gains `noPropagate?: boolean`; `completeGoal` skips the propagation block when set
- Modify: `src/cli.ts` — `goal complete` command gains `--no-propagate` flag
- Test: `tests/goals-complete-no-propagate.test.ts` (new) — 3 cases (propagate / no-propagate / second-call-after-propagate is a true no-op)

### Step 1: Write the failing test

```ts
/**
 * v1.7.4 — `completeGoal` honours `noPropagate` to skip strength
 * multiplier side-effects on recalled memories.
 */

import { describe, it, expect, beforeEach } from 'vitest';
import path from 'node:path';
import os from 'node:os';
import fs from 'node:fs';
import { remember, recall, type Context } from '../src/api.js';
import { pushGoal, completeGoal } from '../src/goals.js';
import { openHippoDb, closeHippoDb } from '../src/db.js';

describe('completeGoal noPropagate flag (v1.7.4)', () => {
  let hippoRoot: string;
  let ctx: Context;
  const tenantId = 'default';
  const sessionId = 'sess-no-prop';

  beforeEach(async () => {
    hippoRoot = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'hippo-1.7.4-'));
    ctx = { hippoRoot, tenantId, actor: 'cli' };
  });

  function getStrength(memId: string): number {
    const db = openHippoDb(hippoRoot);
    try {
      const row = db.prepare('SELECT strength FROM memories WHERE id = ?').get(memId) as { strength: number };
      return row.strength;
    } finally {
      closeHippoDb(db);
    }
  }

  it('default propagates strength multiplier on positive outcome', async () => {
    const m = await remember(ctx, { content: 'fix auth bug', tags: ['fix-auth'] });
    const goal = pushGoal(hippoRoot, { sessionId, tenantId, goalName: 'fix-auth' });
    recall(ctx, { query: 'auth', sessionId }); // logs to goal_recall_log via Task 1's helper
    const beforeStrength = getStrength(m.id);
    completeGoal(hippoRoot, goal.id, { outcomeScore: 1.0 }); // positive outcome
    const afterStrength = getStrength(m.id);
    expect(afterStrength).toBeGreaterThan(beforeStrength);
  });

  it('noPropagate skips strength side-effects', async () => {
    const m = await remember(ctx, { content: 'fix auth bug', tags: ['fix-auth'] });
    const goal = pushGoal(hippoRoot, { sessionId, tenantId, goalName: 'fix-auth' });
    recall(ctx, { query: 'auth', sessionId });
    const beforeStrength = getStrength(m.id);
    completeGoal(hippoRoot, goal.id, { outcomeScore: 1.0, noPropagate: true });
    const afterStrength = getStrength(m.id);
    expect(afterStrength).toBe(beforeStrength); // unchanged
  });

  it('second call with noPropagate is a true no-op after first call propagated (idempotency)', async () => {
    // Status check at src/goals.ts:233-236 short-circuits the second call BEFORE
    // reading opts.noPropagate. So a second call with noPropagate after a propagating
    // first call leaves strength as the post-first-call value (propagation already
    // happened on call 1; call 2 is a no-op regardless of noPropagate).
    const m = await remember(ctx, { content: 'fix auth bug', tags: ['fix-auth'] });
    const goal = pushGoal(hippoRoot, { sessionId, tenantId, goalName: 'fix-auth' });
    recall(ctx, { query: 'auth', sessionId });
    completeGoal(hippoRoot, goal.id, { outcomeScore: 1.0 }); // call 1: propagates
    const afterFirstCall = getStrength(m.id);
    completeGoal(hippoRoot, goal.id, { outcomeScore: 1.0, noPropagate: true }); // call 2
    expect(getStrength(m.id)).toBe(afterFirstCall); // call 2 is a no-op
  });
});
```

Run: `npx vitest run tests/goals-complete-no-propagate.test.ts`
Expected: FAIL — `noPropagate` not a known field; or strength changes anyway.

### Step 2: Update `CompleteGoalOpts` and `completeGoal`

In `src/goals.ts`, find `CompleteGoalOpts` interface, add:

```ts
/**
 * v1.7.4 — when true, skip the strength-multiplier propagation block.
 * Default false (propagate). The goal's status still transitions to
 * 'completed' and `outcome_score` is still recorded; only the side-effect
 * on recalled memories' strength is suppressed.
 */
noPropagate?: boolean;
```

In `completeGoal` (line 245), wrap the propagation block:

```ts
if (score !== null && !opts.noPropagate) {
  // ... existing propagation logic ...
}
```

### Step 3: Add `--no-propagate` flag to CLI `goal complete`

Find the CLI `goal complete` command (grep `goal complete` in src/cli.ts). Add `--no-propagate` to flag parsing, forward to `completeGoal({ noPropagate: flags['no-propagate'] === true })`.

Add a help-text line documenting the flag.

### Step 4: Run tests

```bash
npx vitest run tests/goals-complete-no-propagate.test.ts
npx vitest run --reporter=dot 2>&1 | tail -5
npx tsc --noEmit
```

Expected: 2 new cases pass, full suite green, TSC clean.

### Step 5: Commit

```bash
git add src/goals.ts src/cli.ts tests/goals-complete-no-propagate.test.ts
git commit -m "feat: --no-propagate flag on goal complete (v1.7.4)"
```

**Success criteria:**
- `CompleteGoalOpts.noPropagate?: boolean` added with JSDoc.
- `completeGoal` skips propagation when `noPropagate` true.
- CLI `--no-propagate` flag wired and documented.
- 3 new tests pass (propagate / no-propagate / second-call idempotency); full suite green.
- One atomic commit.

---

## Task 3: Extract `enforceDepthCapWithinTx` helper

**Why:** `pushGoalWithDb` (lines 100-119) and `resumeGoal` (lines 299-316) duplicate the same SQL shape: count active goals → if at cap, suspend oldest. Pure DRY refactor surfaced in v0.38 plan-eng-review. No behaviour change.

**Files:**
- Modify: `src/goals.ts` — extract `enforceDepthCapWithinTx(db, tenantId, sessionId)` helper, replace both call sites
- Test: `tests/goals-enforce-depth-cap.test.ts` (new) — direct unit test on the helper

### Step 1: Write the failing test

```ts
/**
 * v1.7.4 — enforceDepthCapWithinTx helper extracted from pushGoalWithDb and
 * resumeGoal. Pure refactor: pin behaviour with a direct test.
 */

import { describe, it, expect, beforeEach } from 'vitest';
import path from 'node:path';
import os from 'node:os';
import fs from 'node:fs';
import { openHippoDb, closeHippoDb, initStore } from '../src/db.js';
import { pushGoal, enforceDepthCapWithinTx, MAX_ACTIVE_GOAL_DEPTH } from '../src/goals.js';

describe('enforceDepthCapWithinTx helper (v1.7.4)', () => {
  let hippoRoot: string;
  const tenantId = 'default';
  const sessionId = 'sess-cap';

  beforeEach(async () => {
    hippoRoot = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'hippo-1.7.4-'));
    initStore(hippoRoot);
  });

  it('no-op when active goal count is below cap (caller in BEGIN IMMEDIATE)', () => {
    pushGoal(hippoRoot, { sessionId, tenantId, goalName: 'g1' });
    const db = openHippoDb(hippoRoot);
    try {
      // Helper requires caller-managed transaction. Wrap explicitly so the
      // test does NOT normalize unsafe out-of-tx usage (codex P1 finding).
      db.exec('BEGIN IMMEDIATE');
      try {
        enforceDepthCapWithinTx(db, tenantId, sessionId);
        db.exec('COMMIT');
      } catch (err) { db.exec('ROLLBACK'); throw err; }
      const active = db.prepare(
        `SELECT COUNT(*) AS c FROM goal_stack WHERE status = 'active' AND tenant_id = ? AND session_id = ?`,
      ).get(tenantId, sessionId) as { c: number };
      expect(active.c).toBe(1); // unchanged
    } finally {
      closeHippoDb(db);
    }
  });

  it('suspends the oldest active goal when at cap (caller in BEGIN IMMEDIATE)', () => {
    for (let i = 0; i < MAX_ACTIVE_GOAL_DEPTH; i++) {
      pushGoal(hippoRoot, { sessionId, tenantId, goalName: `g${i}` });
    }
    const db = openHippoDb(hippoRoot);
    try {
      db.exec('BEGIN IMMEDIATE');
      try {
        enforceDepthCapWithinTx(db, tenantId, sessionId);
        db.exec('COMMIT');
      } catch (err) { db.exec('ROLLBACK'); throw err; }
      const active = db.prepare(
        `SELECT COUNT(*) AS c FROM goal_stack WHERE status = 'active' AND tenant_id = ? AND session_id = ?`,
      ).get(tenantId, sessionId) as { c: number };
      expect(active.c).toBe(MAX_ACTIVE_GOAL_DEPTH - 1);
    } finally {
      closeHippoDb(db);
    }
  });
});
```

Run: `npx vitest run tests/goals-enforce-depth-cap.test.ts`
Expected: FAIL — `enforceDepthCapWithinTx` not exported.

### Step 2: Extract the helper

In `src/goals.ts`, add (above `pushGoal`):

```ts
/**
 * v1.7.4 — depth-cap enforcer extracted from pushGoalWithDb and resumeGoal.
 * If the (tenant, session) has >= MAX_ACTIVE_GOAL_DEPTH active goals,
 * suspend the oldest `overflow` ones.
 *
 * **Precondition: caller MUST already be inside a `BEGIN IMMEDIATE`
 * transaction.** Helper does not open or commit — name reflects this so it
 * is impossible to misread the contract at a call site. Both existing call
 * sites (pushGoalWithDb, resumeGoal) wrap in `BEGIN IMMEDIATE` already.
 *
 * @internal v1.7.4 — internal goal-stack invariant. Subject to change.
 */
export function enforceDepthCapWithinTx(
  db: DatabaseSyncLike,
  tenantId: string,
  sessionId: string,
): void {
  const activeCount = (db.prepare(`
    SELECT COUNT(*) AS c
    FROM goal_stack
    WHERE tenant_id = ? AND session_id = ? AND status = 'active'
  `).get(tenantId, sessionId) as { c: number }).c;

  if (activeCount >= MAX_ACTIVE_GOAL_DEPTH) {
    const overflow = activeCount - MAX_ACTIVE_GOAL_DEPTH + 1;
    db.prepare(`
      UPDATE goal_stack
      SET status = 'suspended'
      WHERE id IN (
        SELECT id FROM goal_stack
        WHERE tenant_id = ? AND session_id = ? AND status = 'active'
        ORDER BY created_at ASC
        LIMIT ?
      )
    `).run(tenantId, sessionId, overflow);
  }
}
```

### Step 3: Replace inline blocks at both call sites

In `pushGoalWithDb` (`src/goals.ts:100-119`), replace the inline block with `enforceDepthCapWithinTx(db, opts.tenantId, opts.sessionId);` (preserving the surrounding `BEGIN IMMEDIATE` / `COMMIT`).

In `resumeGoal` (`src/goals.ts:299-316`), replace the inline block with `enforceDepthCapWithinTx(db, row.tenant_id, row.session_id);`.

### Step 4: Run tests

```bash
npx vitest run tests/goals-enforce-depth-cap.test.ts
npx vitest run --reporter=dot 2>&1 | tail -5
npx tsc --noEmit
```

Expected: 2 new tests pass; full suite green (existing `pushGoal` and `resumeGoal` tests must still pass — they exercise the helper transparently); TSC clean.

### Step 5: Commit

```bash
git add src/goals.ts tests/goals-enforce-depth-cap.test.ts
git commit -m "refactor: extract enforceDepthCapWithinTx helper from pushGoal/resumeGoal (v1.7.4)"
```

**Success criteria:**
- `enforceDepthCapWithinTx` exported from `src/goals.ts`, `@internal`, NOT in `src/index.ts`.
- `pushGoalWithDb` and `resumeGoal` shed ~18 lines each via helper call.
- 2 new tests pass; existing pushGoal/resumeGoal tests still pass.
- TSC clean. Full suite green.
- One atomic commit.

---

## Final phase: ship

After all 3 tasks committed:

### S1: `/self-review`

`git log --oneline master..HEAD` + `git diff master..HEAD --stat`. Verify each commit traces to a TODOS.md item. No drift.

### S2: `/review`

Pre-landing review against `master`. Address any P0/P1. Defer P2/P3 with one-liner in TODOS.md.

### S3: `/ship-check`

Confirm: 3 items closed, full suite green, TSC clean, /review clean, worth a release.

### S4: `/publish-repo`

Drives bump 1.7.3 → 1.7.4, CHANGELOG, README "What's new in v1.7.4", build, test, commit, PR + merge, npm publish, tag, GH release. Ground rules: no `--no-verify`, commit email `skfsk27@gmail.com`.

CHANGELOG content (provide to publish-repo):

```markdown
## 1.7.4 (YYYY-MM-DD)

Internal hygiene release closing 3 of the 5 B3 dlPFC follow-ups deferred from v0.39.0. Adds optional `RecallOpts.sessionId` (and `RecallOpts.goalTag`) so MCP `hippo_recall` and HTTP `GET /v1/memories` callers get the dlPFC goal-stack boost — previously CLI-only. Adds `--no-propagate` flag on `goal complete`. Refactors `enforceDepthCapWithinTx` helper.

### Added

- **`RecallOpts.sessionId?: string`.** Optional session id on the recall input. When set AND `(tenant, session)` has active goals AND `goalTag` is unset, `api.recall` AND MCP `hippo_recall`'s primary band BOTH apply the dlPFC goal-stack boost lifted from v0.38's CLI-only path. Undefined preserves v1.7.3 behaviour (no boost). Lives on `RecallOpts` rather than `Context` because `Context` is shared across remember/recall/assemble/outcome and the boost is recall-only.
- **`RecallOpts.goalTag?: string`.** Optional explicit-tag override. When set, the goal-stack boost is suppressed (mirrors the CLI `--goal X` precedence from v0.38).
- **`applyGoalStackBoost` helper** (`@internal`, exported from `src/goals.ts`). Lifted ~140 lines of CLI ranking logic into a reusable helper. Operates on entry-backed scored rows (NOT projected `RecallResultItem`) so it can run before fresh-tail / summary-substitution appendix rendering. Side effects: `goal_recall_log INSERT OR IGNORE` filtered to local memory ids.
- **MCP `hippo_recall.session_id`** input schema field added (256-char cap; mirrors `fresh_tail_session_id`). Was previously absent — `session_id` was on `hippo_assemble` only.
- **MCP `hippo_recall` primary band boosted.** Boost applied to `physicsSearch`/`hybridSearch` result list before `formatMemories`. Pre-v1.7.4 the MCP primary ordering bypassed `api.recall` and so got no boost even when callers thought they were getting one.
- **HTTP `GET /v1/memories?session_id=...`** query param added (256-char cap, 400 rejected when over).
- **`completeGoal` accepts `noPropagate?: boolean`.** Defaults false (propagate, as in v1.6.x). When true, skips strength multiplier side-effects on recalled memories. CLI: `goal complete --no-propagate`. Status-check idempotency unaffected: a second call after a propagating first call is a true no-op regardless of `noPropagate`.

### Refactored (no behaviour change)

- **`enforceDepthCapWithinTx` helper** extracted from `pushGoalWithDb` and `resumeGoal` (`@internal`). Pure DRY cleanup. Name is explicit about its precondition (caller must already be inside `BEGIN IMMEDIATE`).

### Deferred to v1.7.5

- Sequential-learning adapter contract (`pushGoal/completeGoal` hooks on `benchmarks/sequential-learning/adapters/interface.mjs`). Demonstrate or honestly retire the −10pp trap-rate claim. Needs benchmark runs + honest reporting → own release.

### Deferred to v1.8.0

- vlPFC interference suppression. Real feature work per RESEARCH.md. Own plan + outside voice.
```

README "What's new in v1.7.4" content:

```markdown
### What's new in v1.7.4

- **Goal-stack boost on MCP + HTTP.** Set `RecallOpts.sessionId` (or HTTP `?session_id=...`, or MCP `hippo_recall { session_id }`) and the dlPFC goal-stack boost — previously CLI-only — applies on MCP and HTTP too. Both `api.recall` (primary BM25 band, before fresh-tail / summary appendix) AND MCP's separate `physicsSearch`/`hybridSearch` path are boosted. New `RecallOpts.goalTag` lets callers opt out per-call.
- **`goal complete --no-propagate`.** New CLI flag and `CompleteGoalOpts.noPropagate` field for users who want to close a goal without strength side-effects on recalled memories. Default unchanged (propagate).
- **Internal: `applyGoalStackBoost` and `enforceDepthCapWithinTx` helpers.** Lifted ~140 lines of duplicated logic into shared helpers. `@internal`, not on the public API surface.
```

### S5: `hippo capture` + close 3 of the 5 B3 items in TODOS.md

Edit `TODOS.md` lines 145-160. Mark the 3 shipped items `[x]`, leave the 2 deferred items in place with notes:
- `[x] B3 follow-up: MCP/REST session_id plumbing` (shipped v1.7.4 — via `RecallOpts.sessionId` + MCP physics/hybrid path boost)
- `[x] B3 follow-up: --no-propagate flag on goal complete` (shipped v1.7.4)
- `[x] B3 v0.39 follow-up: factor enforceDepthCapWithinTx helper` (shipped v1.7.4)
- `[ ] B3 follow-up: sequential-learning adapter contract` → v1.7.5
- `[ ] B3 follow-up: vlPFC interference handling` → v1.8.0

Commit `docs: close 3 of 5 B3 follow-ups in TODOS (shipped v1.7.4)`. Push to master.

---

## Risk register

| Risk | Likelihood | Mitigation |
|------|-----------|------------|
| Lifting CLI block introduces subtle behaviour drift in goal-boost ranking | Medium | Lift verbatim; do not refactor logic during the move. Existing CLI tests pin the behaviour. 4 new helper-level unit tests pin the lifted behaviour at the helper boundary. |
| Result-shape mismatch — boost helper expects entry-backed rows, `api.recall` projects to `RecallResultItem` | High (was the P0 from review) | Helper signature uses `R extends { entry: MemoryEntry; score: number }`. `api.recall` calls helper BEFORE projection — operate on the entry-backed `baseEntries`, not `RecallResultItem`. |
| Boost reorders `api.recall` appendix rows (fresh-tail / summary substitutions) | High (was the P1 from review) | Boost the PRIMARY band only. Fresh-tail and summary substitutions appended AFTER. Recompute `tokens` after final assembly. |
| MCP `hippo_recall` primary ordering bypasses `api.recall` so the boost wouldn't apply | High (was the P1 from codex) | Helper called from BOTH `api.recall` AND MCP's `physicsSearch`/`hybridSearch` path before `formatMemories`. |
| `RecallOpts.sessionId` change ripples to unexpected callers | Low | Field is optional, additive. Pre-flight: `grep -rn "RecallOpts" src/ tests/` to enumerate construction sites. |
| `api.recall` opening a second db handle for the boost is wasteful | Low | Thread the existing audit-event handle through the boost. Single handle, single open/close. |
| MCP/HTTP `session_id` validation drift | Low | Apply the same 256-char cap and trim semantics as `fresh_tail_session_id`. Pin with HTTP test that asserts 400 on >256 chars. |
| `noPropagate` interacts with idempotency (second `completeGoal` call) | Low | Status check (`src/goals.ts:233-236`) short-circuits second call regardless of `noPropagate`. Third test case pins this explicitly. |
| `enforceDepthCapWithinTx` test normalizes unsafe out-of-tx usage | Low | Test wraps direct call in explicit `BEGIN IMMEDIATE` / `COMMIT`. Helper name is explicit about the precondition. |
| `enforceDepthCapWithinTx` SQL semantics drift between push vs resume sites | Low | Verbatim port. Existing pushGoal/resumeGoal tests pin transparently. |
| `RetrievalPolicy` type duplication after lift | Low | Type stays in `src/goals.ts`. Audit `src/cli.ts` after lift to confirm single import — delete any duplicate. |

---

## Constraints (from CLAUDE.md + project memory)

- **Real DB for tests.** All Task 1, 2, 3 tests touch SQLite. No mocks.
- **Never `--no-verify`** on commits.
- **Atomic commit per task.** No squashing across tasks.
- **TDD: failing test first** on Tasks 1, 2, 3.
- **Branch off master.** Do not commit on master.
- **Pre-flight grep before code touches:** confirm `HIPPO_SESSION_ID` env path (verified: `src/cli.ts:988-1140`). Confirm `physicsSearch`/`hybridSearch`/`formatMemories` exact call sites in `src/mcp/server.ts` (around line 424). Confirm `api.recall`'s primary BM25 result variable name and where the appendix is appended.
- **No public API change** beyond the additive `RecallOpts.sessionId?: string` and `RecallOpts.goalTag?: string`. `Context` is unchanged. `applyGoalStackBoost` and `enforceDepthCapWithinTx` are `@internal`, NOT re-exported from `src/index.ts`.

## Estimated time

| Task | Time |
|------|------|
| Pre-flight | 5 min |
| Task 1 (helper + api.recall primary band + MCP physics path + HTTP + 4 unit tests + 3 transport tests) | 120-180 min |
| Task 2 (`--no-propagate` flag + 3 tests) | 30 min |
| Task 3 (`enforceDepthCapWithinTx` helper + 2 tests) | 20 min |
| Ship phase (S1-S5) | 30-45 min |
| **Total** | **~4-5 hours** |

Caveat: Task 1 is the meaningful refactor and grew after the post-review revision (MCP physics-path wiring + helper-level unit tests + appendix-placement preservation). The other two are mechanical. Most of the risk + outside-voice surface area is on Task 1.

---

## Plan history

- **2026-05-07 v1** — initial draft.
- **2026-05-07 v2 (this version)** — post outside-voice review (senior-code-reviewer + Codex). 13 consolidated revisions applied, biggest reshapes: (a) `sessionId` moved from `Context` to `RecallOpts`, (b) helper operates on entry-backed rows BEFORE `RecallResultItem` projection, (c) MCP physics/hybrid path wired separately from `api.recall` (codex caught that MCP's primary ordering bypasses `api.recall`), (d) `hippo_recall` schema gains `session_id` (was wrong fact in v1), (e) appendix rows excluded from boost to preserve fresh-tail / summary placement, (f) helper renamed `enforceDepthCapWithinTx` to make the transactional precondition impossible to misread, (g) test coverage expanded from 3 transport-only to 4 helper unit + 3 transport + 1 noPropagate idempotency.
