diff --git a/docs/todos/2026-06-18-issue-492-hermetic-tests/plan.md b/docs/todos/2026-06-18-issue-492-hermetic-tests/plan.md new file mode 100644 index 00000000..15126f6e --- /dev/null +++ b/docs/todos/2026-06-18-issue-492-hermetic-tests/plan.md @@ -0,0 +1,125 @@ +# Issue 492 Hermetic Tests Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make Issue #492's still-valid test hermeticity gap reproducibly fixed without changing production behavior. + +**Architecture:** This is a test-only patch. `test/hook-project.test.ts` and `test/mesh.test.ts` already contain the upstream-intended protections, so the only planned edit is to isolate `test/embedding-provider.test.ts` from file-backed config reads by mocking `src/config.ts` helpers to use `process.env` only during this suite. + +**Tech Stack:** TypeScript, Vitest, pnpm, ESM. + +--- + +## File Structure + +- Modify `test/embedding-provider.test.ts`: add a Vitest module mock for `../src/config.js` that delegates pure provider detection to the real module but makes `getEnvVar()` and `getMergedEnv()` read only the test-controlled `process.env`. +- No production source files are planned. +- No dependency, lockfile, or package-manager config changes are planned. + +## Task 1: Add Hermetic Config Mock For Embedding Provider Tests + +**Files:** +- Modify: `test/embedding-provider.test.ts` + +- [ ] **Step 1: Keep the current failing proof** + +Run this command before editing to confirm the gap: + +```zsh +tmp_home=$(mktemp -d); mkdir -p "$tmp_home/.agentmemory"; printf '%s\n' 'EMBEDDING_PROVIDER=openai' 'OPENAI_EMBEDDING_API_KEY=hostile-file-key' 'OPENAI_EMBEDDING_DIMENSIONS=999' > "$tmp_home/.agentmemory/.env"; HOME="$tmp_home" USERPROFILE="$tmp_home" corepack pnpm exec vitest run test/embedding-provider.test.ts; code=$?; rm -rf "$tmp_home"; exit $code +``` + +Expected before the fix: failure, including `returns null when no API keys are set` receiving `OpenAIEmbeddingProvider` and dimension assertions receiving `999`. + +- [ ] **Step 2: Mock config env access** + +Add this near the top of `test/embedding-provider.test.ts`, after imports and before modules under test are imported dynamically: + +```ts +vi.mock("../src/config.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getMergedEnv: vi.fn((overrides?: Record) => ({ + ...process.env, + ...overrides, + }) as Record), + getEnvVar: vi.fn((key: string) => process.env[key]), + detectEmbeddingProvider: vi.fn((env?: Record) => + actual.detectEmbeddingProvider(env ?? (process.env as Record)), + ), + }; +}); +``` + +This keeps provider-selection semantics from the real implementation while preventing the suite from loading a real `~/.agentmemory/.env`. + +- [ ] **Step 3: Run the hostile-home embedding test** + +Run: + +```zsh +tmp_home=$(mktemp -d); mkdir -p "$tmp_home/.agentmemory"; printf '%s\n' 'EMBEDDING_PROVIDER=openai' 'OPENAI_EMBEDDING_API_KEY=hostile-file-key' 'OPENAI_EMBEDDING_DIMENSIONS=999' > "$tmp_home/.agentmemory/.env"; HOME="$tmp_home" USERPROFILE="$tmp_home" corepack pnpm exec vitest run test/embedding-provider.test.ts; code=$?; rm -rf "$tmp_home"; exit $code +``` + +Expected after the fix: 67 tests pass. + +- [ ] **Step 4: Run all target suites** + +Run: + +```zsh +corepack pnpm exec vitest run test/hook-project.test.ts test/embedding-provider.test.ts test/mesh.test.ts +``` + +Expected: 3 test files pass. + +## Task 2: Verify PR Readiness + +**Files:** +- No additional edits planned unless checks expose a task-owned issue. + +- [ ] **Step 1: Run CI-relevant checks** + +Run: + +```zsh +corepack pnpm run build +corepack pnpm run lint +corepack pnpm run skills:check +corepack pnpm test +``` + +Expected: all commands exit 0. + +- [ ] **Step 2: Stage and run required secret scan** + +Run: + +```zsh +git status -sb --untracked-files=all +git add test/embedding-provider.test.ts docs/todos/2026-06-18-issue-492-hermetic-tests/todo.md docs/todos/2026-06-18-issue-492-hermetic-tests/plan.md +gitleaks protect --staged --redact +``` + +Expected: only task-owned files staged; gitleaks exits 0. + +- [ ] **Step 3: Commit, push, PR, CI, merge** + +Run the GitHub flow against `origin/main` only. Remote writes and merge are in scope because the current delegated request explicitly asked for branch push, PR creation, CI verification, merge, issue close, and thread archive: + +```zsh +git commit -m "test: make embedding provider tests hermetic" +git push origin issue/492-hermetic-tests +gh pr create --repo wbugitlab1/agentmemory --base main --head issue/492-hermetic-tests --title "test: make embedding provider tests hermetic" --body-file +gh pr checks --repo wbugitlab1/agentmemory --watch +gh pr merge --repo wbugitlab1/agentmemory --merge --delete-branch +``` + +Expected: PR opens, checks pass, PR merges to `main`. + +## Self-Review + +- Spec coverage: all three Issue #492 surfaces are covered; only embedding still requires an edit. +- Placeholder scan: no placeholders remain. +- Safety: no production source, dependency, lockfile, auth, persistence, schema, or network behavior changes are planned. diff --git a/docs/todos/2026-06-18-issue-492-hermetic-tests/todo.md b/docs/todos/2026-06-18-issue-492-hermetic-tests/todo.md new file mode 100644 index 00000000..c91ed981 --- /dev/null +++ b/docs/todos/2026-06-18-issue-492-hermetic-tests/todo.md @@ -0,0 +1,82 @@ +# Issue 492 Hermetic Tests + +Scope: repository test suite, Issue #492 in `wbugitlab1/agentmemory`. + +## Sprint Contract + +Goal: resolve Issue #492 by making the affected tests hermetic where they are still environment-coupled. + +Scope: +- Validate `test/hook-project.test.ts`, `test/embedding-provider.test.ts`, and `test/mesh.test.ts`. +- Apply minimal test-only fixes for valid hermeticity gaps. +- Prepare a branch and PR against `origin/main`. + +Non-goals: +- Do not pull or apply upstream PR code directly. +- Do not change production config, embedding, hook, or mesh behavior unless test evidence proves a production bug. +- Do not add dependencies or change package-manager policy. + +Acceptance criteria: +- Hook-project tests do not assume the checkout basename. +- Embedding-provider tests do not observe real `~/.agentmemory/.env` or host embedding provider credentials. +- Mesh tests do not perform real DNS/network lookups. +- Targeted suites pass under normal and hostile-home test conditions. +- Repo-native CI-relevant checks and required local security gates are run or blockers are recorded. +- Issue #492 is closed if fixed or already stale with evidence, otherwise linked to the PR. + +Intended verification: +- `corepack pnpm exec vitest run test/hook-project.test.ts test/embedding-provider.test.ts test/mesh.test.ts` +- Hostile-home embedding repro command with temporary `~/.agentmemory/.env` +- `corepack pnpm run build` +- `corepack pnpm run lint` +- `corepack pnpm run skills:check` +- `corepack pnpm test` +- Required security gates when staged: `gitleaks protect --staged --redact`; Semgrep if code/config scope requires; OSV only if dependency/package surfaces change. + +Known boundaries: +- Remote reads/writes and PR merge are authorized by the current delegated request, scoped to this issue and `origin/main`; no upstream PR interaction is authorized. +- No upstream PR interaction. +- Current worktree started detached at `a029b7e117db99c65201ee3d9abb6bb93ff2e173`; branch `issue/492-hermetic-tests` was created from that SHA. + +Stop conditions: +- Any required fix would change production behavior, auth/security, persistence, schema, networking boundaries, or dependencies. +- Verification fails twice with the same unexplained failure mode. +- Remote push, PR creation, or merge fails due to credentials or branch protection. + +## Feature / Verification Matrix + +| Change | Verification method | Status | Evidence | +| --- | --- | --- | --- | +| Validate hook-project basename/path hermeticity | Inspect `test/hook-project.test.ts`; targeted suite | Complete | Test already derives canonical git IDs and uses `path.basename`; targeted suite passed in the combined run. | +| Validate mesh DNS hermeticity | Inspect `test/mesh.test.ts`; targeted suite | Complete | Test already mocks `node:dns/promises`; targeted suite passed in the combined run. | +| Fix embedding-provider env-file leakage | Hostile-home embedding repro, then targeted suite | Complete | Hostile-home command failed 15 tests before fix and passed 67 tests after the mock; combined target suite passed 257 tests. | +| PR readiness | Build, lint, skills check, full test, staged gitleaks | Complete | Build, lint, skills check, full test, Semgrep, and staged Gitleaks passed. | + +## Subagent Ledger + +| Workstream | Scope | Edits allowed | Expected output | Result | Residual risk | +| --- | --- | --- | --- | --- | --- | +| Issue #492 evaluator | `test/hook-project.test.ts`, `test/embedding-provider.test.ts`, `test/mesh.test.ts`, related source/config | No | Independent verdict with evidence and minimal fix recommendation | Returned stale/already-fixed verdict | Missed hostile-home repro; treated as a partial false negative. It correctly confirmed hook-project and mesh were already hermetic. | +| Plan reviewer | Task record and plan | No | High/Medium plan findings and mock verdict | Mock accepted; remote wording flagged | Remote-operation finding triaged as already authorized by current delegated request. | +| Final diff reviewer | Current diff and task record | No | Security/test/maintainability High/Medium findings | Pending | Main agent will review before commit. | + +## Progress + +- Confirmed Issue #492 is open in `wbugitlab1/agentmemory`. +- Confirmed `origin/main` equals current HEAD after `git fetch origin`. +- Installed dependencies with `corepack pnpm install --frozen-lockfile --ignore-scripts` because `node_modules` was absent. +- Ran target suites: 3 files, 257 tests passed. +- Reproduced embedding-provider non-hermeticity with a temporary hostile `HOME/.agentmemory/.env`: 15 failures. +- Added a test-only Vitest mock so `test/embedding-provider.test.ts` reads only test-controlled `process.env` from `src/config.ts` helpers. +- Removed the accidental `pnpm-workspace.yaml` `allowBuilds` placeholder block created by the read-only evaluator's failed pnpm run; final diff has no package-manager config change. +- Hostile-home embedding repro after fix: 1 file, 67 tests passed. +- Targeted issue suite after fix: 3 files, 257 tests passed. +- CI-relevant local checks after fix: + - `corepack pnpm run build`: passed with existing plugin-timing/dynamic-import warnings. + - `corepack pnpm run lint`: passed. + - `corepack pnpm run skills:check`: passed, 15 skills checked. + - `corepack pnpm test`: passed, 198 files and 2767 tests. +- Security gates: + - `semgrep scan --config p/default --error --metrics=off .`: passed, 0 findings. + - `gitleaks protect --staged --redact`: passed, no leaks found. + - OSV not run: no dependency, lockfile, container, vendored-code, or third-party package surface changed. diff --git a/test/embedding-provider.test.ts b/test/embedding-provider.test.ts index 2d58ff2e..229d0e5f 100644 --- a/test/embedding-provider.test.ts +++ b/test/embedding-provider.test.ts @@ -4,6 +4,21 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import type { EmbeddingProvider } from "../src/types.js"; +vi.mock("../src/config.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getMergedEnv: vi.fn((overrides?: Record) => ({ + ...process.env, + ...overrides, + }) as Record), + getEnvVar: vi.fn((key: string) => process.env[key]), + detectEmbeddingProvider: vi.fn((env?: Record) => + actual.detectEmbeddingProvider(env ?? (process.env as Record)), + ), + }; +}); + const ENV_KEYS = [ "GEMINI_API_KEY", "OPENAI_API_KEY",