Mid-session edits to hindsight.bankId / bankIdPrefix / scoping kept the active HindsightSessionState pinned to the bank selected at session start, so retain/recall/reflect calls landed in the stale bank. Settings hooks now fire onHindsightScopeChanged; the backend rebuilds the primary state against the recomputed scope, disposing the previous one after flushing its queue so queued tool-initiated retains still land in the bank they were enqueued for. Also: - Renamed ensureBankMission to ensureBankExists. The old version skipped creation entirely when bankMission was blank, so the first mental-model POST (auto-seed) could land against a never-PUT bank. Bank creation is now idempotent and unconditional, and runs before mental-model bootstrap. - Fixed AgentSession.dispose to flush the retain queue BEFORE clearing the session state pointer. Reversed, HindsightRetainQueue.#doFlush's identity guard would see the cleared pointer and drop the spliced batch with a 'session vanished' warning. - Snapshotted hindsightScopeCallbacks before iterating because each rebuild subscribes a fresh callback inside the same fire; iterating the live Set would spin. Fixes #1902
178 lines
6.5 KiB
TypeScript
178 lines
6.5 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it, type Mock, vi } from "bun:test";
|
|
import { computeBankScope, deriveBankId, ensureBankExists } from "@oh-my-pi/pi-coding-agent/hindsight/bank";
|
|
import { HindsightApi } from "@oh-my-pi/pi-coding-agent/hindsight/client";
|
|
import type { HindsightConfig } from "@oh-my-pi/pi-coding-agent/hindsight/config";
|
|
|
|
const baseConfig = (overrides: Partial<HindsightConfig> = {}): HindsightConfig => ({
|
|
hindsightApiUrl: "http://localhost:8888",
|
|
hindsightApiToken: null,
|
|
bankId: null,
|
|
bankIdPrefix: "",
|
|
scoping: "global",
|
|
bankMission: "",
|
|
retainMission: null,
|
|
autoRecall: true,
|
|
autoRetain: true,
|
|
retainMode: "full-session",
|
|
retainEveryNTurns: 3,
|
|
retainOverlapTurns: 2,
|
|
retainContext: "omp",
|
|
recallBudget: "mid",
|
|
recallMaxTokens: 1024,
|
|
recallTypes: ["world", "experience"],
|
|
recallContextTurns: 1,
|
|
recallMaxQueryChars: 800,
|
|
recallPromptPreamble: "preamble",
|
|
debug: false,
|
|
mentalModelsEnabled: false,
|
|
mentalModelAutoSeed: false,
|
|
mentalModelRefreshIntervalMs: 5 * 60 * 1000,
|
|
mentalModelMaxRenderChars: 16_000,
|
|
...overrides,
|
|
});
|
|
|
|
describe("computeBankScope", () => {
|
|
describe("scoping=global", () => {
|
|
it("returns the configured bank id verbatim", () => {
|
|
expect(computeBankScope(baseConfig({ bankId: "team-a" }), "/work/proj")).toEqual({
|
|
bankId: "team-a",
|
|
});
|
|
});
|
|
|
|
it("falls back to the default bank name when bankId is unset", () => {
|
|
expect(computeBankScope(baseConfig(), "/whatever")).toEqual({ bankId: "omp" });
|
|
});
|
|
|
|
it("applies the configured prefix", () => {
|
|
expect(computeBankScope(baseConfig({ bankId: "team", bankIdPrefix: "prod" }), "/cwd")).toEqual({
|
|
bankId: "prod-team",
|
|
});
|
|
});
|
|
|
|
it("does not surface tag fields", () => {
|
|
const scope = computeBankScope(baseConfig(), "/work/proj");
|
|
expect(scope.retainTags).toBeUndefined();
|
|
expect(scope.recallTags).toBeUndefined();
|
|
expect(scope.recallTagsMatch).toBeUndefined();
|
|
});
|
|
});
|
|
|
|
describe("scoping=per-project", () => {
|
|
it("appends the cwd basename to the base bank id", () => {
|
|
expect(computeBankScope(baseConfig({ scoping: "per-project" }), "/work/proj")).toEqual({
|
|
bankId: "omp-proj",
|
|
});
|
|
});
|
|
|
|
it("appends `unknown` for an empty cwd", () => {
|
|
expect(computeBankScope(baseConfig({ scoping: "per-project" }), "")).toEqual({
|
|
bankId: "omp-unknown",
|
|
});
|
|
});
|
|
|
|
it("composes prefix + bankId + project", () => {
|
|
const scope = computeBankScope(
|
|
baseConfig({ scoping: "per-project", bankId: "team", bankIdPrefix: "prod" }),
|
|
"/work/cool-app",
|
|
);
|
|
expect(scope.bankId).toBe("prod-team-cool-app");
|
|
});
|
|
|
|
it("does not surface tag fields (isolation is at the bank level)", () => {
|
|
const scope = computeBankScope(baseConfig({ scoping: "per-project" }), "/work/proj");
|
|
expect(scope.retainTags).toBeUndefined();
|
|
expect(scope.recallTags).toBeUndefined();
|
|
});
|
|
});
|
|
|
|
describe("scoping=per-project-tagged", () => {
|
|
it("keeps the base bank id and emits project tags with `any` match", () => {
|
|
expect(computeBankScope(baseConfig({ scoping: "per-project-tagged" }), "/work/proj")).toEqual({
|
|
bankId: "omp",
|
|
retainTags: ["project:proj"],
|
|
recallTags: ["project:proj"],
|
|
recallTagsMatch: "any",
|
|
});
|
|
});
|
|
|
|
it("uses the same project label for retain and recall tags", () => {
|
|
const scope = computeBankScope(baseConfig({ scoping: "per-project-tagged" }), "/repo/cool-app");
|
|
expect(scope.retainTags).toEqual(["project:cool-app"]);
|
|
expect(scope.recallTags).toEqual(["project:cool-app"]);
|
|
});
|
|
|
|
it("falls back to project:unknown when cwd is empty", () => {
|
|
const scope = computeBankScope(baseConfig({ scoping: "per-project-tagged" }), "");
|
|
expect(scope.retainTags).toEqual(["project:unknown"]);
|
|
expect(scope.recallTags).toEqual(["project:unknown"]);
|
|
});
|
|
});
|
|
});
|
|
|
|
describe("deriveBankId (legacy wrapper)", () => {
|
|
it("returns the bankId field of the resolved scope", () => {
|
|
expect(deriveBankId(baseConfig({ bankId: "team", bankIdPrefix: "prod" }), "/cwd")).toBe("prod-team");
|
|
expect(deriveBankId(baseConfig({ scoping: "per-project" }), "/work/proj")).toBe("omp-proj");
|
|
expect(deriveBankId(baseConfig({ scoping: "per-project-tagged" }), "/work/proj")).toBe("omp");
|
|
});
|
|
});
|
|
|
|
describe("ensureBankExists", () => {
|
|
let client: HindsightApi;
|
|
let createSpy: Mock<HindsightApi["createBank"]> | undefined;
|
|
|
|
beforeEach(() => {
|
|
client = new HindsightApi({ baseUrl: "http://localhost:8888" });
|
|
});
|
|
|
|
afterEach(() => {
|
|
createSpy?.mockRestore();
|
|
});
|
|
|
|
it("calls createBank exactly once per bank id and forwards the mission body", async () => {
|
|
createSpy = vi.spyOn(HindsightApi.prototype, "createBank").mockResolvedValue({} as never);
|
|
const seen = new Set<string>();
|
|
const config = baseConfig({ bankMission: "remember everything", retainMission: "extract facts" });
|
|
|
|
await ensureBankExists(client, "bank-a", config, seen);
|
|
await ensureBankExists(client, "bank-a", config, seen);
|
|
await ensureBankExists(client, "bank-b", config, seen);
|
|
|
|
expect(createSpy).toHaveBeenCalledTimes(2);
|
|
expect(createSpy).toHaveBeenCalledWith(
|
|
"bank-a",
|
|
expect.objectContaining({ reflectMission: "remember everything", retainMission: "extract facts" }),
|
|
);
|
|
expect(createSpy).toHaveBeenCalledWith("bank-b", expect.any(Object));
|
|
expect(seen.has("bank-a")).toBe(true);
|
|
expect(seen.has("bank-b")).toBe(true);
|
|
});
|
|
|
|
// Regression: mental-model auto-seed used to POST `createMentalModel` against
|
|
// a never-created bank when `bankMission` was blank, because the old
|
|
// `ensureBankMission` skipped creation entirely without a mission.
|
|
it("still PUTs the bank when no mission is configured (so the bank gets created)", async () => {
|
|
createSpy = vi.spyOn(HindsightApi.prototype, "createBank").mockResolvedValue({} as never);
|
|
const seen = new Set<string>();
|
|
|
|
await ensureBankExists(client, "bank", baseConfig({ bankMission: "" }), seen);
|
|
await ensureBankExists(client, "bank", baseConfig({ bankMission: " " }), seen);
|
|
|
|
expect(createSpy).toHaveBeenCalledTimes(1);
|
|
expect(createSpy).toHaveBeenCalledWith(
|
|
"bank",
|
|
expect.objectContaining({ reflectMission: undefined, retainMission: undefined }),
|
|
);
|
|
expect(seen.has("bank")).toBe(true);
|
|
});
|
|
|
|
it("swallows API failures and does not mark the bank as initialised", async () => {
|
|
createSpy = vi.spyOn(HindsightApi.prototype, "createBank").mockRejectedValue(new Error("HTTP 500"));
|
|
const seen = new Set<string>();
|
|
const config = baseConfig({ bankMission: "do the thing" });
|
|
|
|
await expect(ensureBankExists(client, "bank-x", config, seen)).resolves.toBeUndefined();
|
|
expect(seen.has("bank-x")).toBe(false);
|
|
});
|
|
});
|