From 6904f28258ef3ada4b1d4df0a4866380272d00a5 Mon Sep 17 00:00:00 2001 From: Copilot Date: Sat, 18 Jul 2026 12:21:51 +0300 Subject: [PATCH 1/3] test(memory): characterization baselines for #1309 (research, not closing) Adds hermetic, non-mutating characterization tests that pin current behavior of history-shadow section append, LocalMemoryStore write/delete ordering, forbidden-content and classify governance, and the spawn-context byte baseline emitted by runMemoryValueBenchmark. Scope is deliberately test-only. No production source, config, dependency, script, template, workflow, or .squad file is modified. All tests operate under temporary directories created via mkdtemp and use a write-guarded storage Proxy to prove zero mutation of repository .squad state. Refs #1309 (research spike only; do not close). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 44f53a4e-d384-4c60-a43d-cfc158160f1f --- test-fixtures/characterization/.gitattributes | 4 + test-fixtures/characterization/README.md | 33 +++ .../spawn-context-baseline.json | 123 +++++++++ .../_helpers/hermetic-root.ts | 107 ++++++++ ...ication-forbidden.characterization.test.ts | 236 ++++++++++++++++++ .../history-shadow.characterization.test.ts | 128 ++++++++++ ...ocal-memory-store.characterization.test.ts | 152 +++++++++++ ...-context-baseline.characterization.test.ts | 44 ++++ 8 files changed, 827 insertions(+) create mode 100644 test-fixtures/characterization/.gitattributes create mode 100644 test-fixtures/characterization/README.md create mode 100644 test-fixtures/characterization/spawn-context-baseline.json create mode 100644 test/characterization/_helpers/hermetic-root.ts create mode 100644 test/characterization/classification-forbidden.characterization.test.ts create mode 100644 test/characterization/history-shadow.characterization.test.ts create mode 100644 test/characterization/local-memory-store.characterization.test.ts create mode 100644 test/characterization/spawn-context-baseline.characterization.test.ts diff --git a/test-fixtures/characterization/.gitattributes b/test-fixtures/characterization/.gitattributes new file mode 100644 index 000000000..da2e04ffa --- /dev/null +++ b/test-fixtures/characterization/.gitattributes @@ -0,0 +1,4 @@ +# Lock line endings for characterization fixtures so the byte-exact +# spawn-context baseline assertion is stable on Windows and Linux checkouts. +*.json text eol=lf +*.md text eol=lf diff --git a/test-fixtures/characterization/README.md b/test-fixtures/characterization/README.md new file mode 100644 index 000000000..d56c6b0f5 --- /dev/null +++ b/test-fixtures/characterization/README.md @@ -0,0 +1,33 @@ +# Characterization test fixtures + +All files in this directory are **hand-authored synthetic content**, safe for a +public repository. No real secrets, no real PII, no real customer data, no +snapshot of any user `.squad` state. + +## Provenance + +- `spawn-context-baseline.json` + - Purpose: pins the byte-exact JSON output of + `runMemoryValueBenchmark()` on the default fixture, so future changes to + context assembly, the fixture, or the benchmark itself produce a visible + diff. + - Generated once by invoking `runMemoryValueBenchmark()` and serializing the + result with `JSON.stringify(report, null, 2) + '\n'`, then committed after + human review. + - Regenerate by running the same invocation. Any diff must be reviewed + intentionally and referenced in the PR description. + +- `history-shadow/` + - Reserved for any small golden snippets that the history-shadow + characterization test needs to compare against. Only committed if the + corresponding test uses them. + +## Rules + +- Never commit content sourced from a user private repo. +- Never commit content sourced from any real audit log, real memory store, or + real conversation. +- Never introduce real credentials, real tokens, or real personal identifiers. + Use obviously synthetic values (for example `ghp_ABCDEFGHIJKLMNOPQR12`, + `hunter2extra`, `123-45-6789`) whose only role is to exercise the + classification rules. diff --git a/test-fixtures/characterization/spawn-context-baseline.json b/test-fixtures/characterization/spawn-context-baseline.json new file mode 100644 index 000000000..919a39f43 --- /dev/null +++ b/test-fixtures/characterization/spawn-context-baseline.json @@ -0,0 +1,123 @@ +{ + "taskResults": [ + { + "taskId": "aspire-docs", + "baselineLoadedIds": [ + "policy-aspire-config", + "archive-raw-env-config", + "local-memory-diagnostics", + "local-memory-governance", + "archive-ci-rotation", + "never-secret", + "policy-headed-browser", + "local-teams-reporting" + ], + "governedLoadedIds": [ + "policy-aspire-config", + "policy-headed-browser" + ], + "baselineContextBytes": 942, + "governedContextBytes": 251, + "baselinePrecision": 0.125, + "governedPrecision": 0.5, + "baselineRecall": 1, + "governedRecall": 1, + "baselineDecisionConsistent": false, + "governedDecisionConsistent": true + }, + { + "taskId": "memory-diagnostics", + "baselineLoadedIds": [ + "policy-aspire-config", + "archive-raw-env-config", + "local-memory-diagnostics", + "local-memory-governance", + "archive-ci-rotation", + "never-secret", + "policy-headed-browser", + "local-teams-reporting" + ], + "governedLoadedIds": [ + "policy-aspire-config", + "policy-headed-browser", + "local-memory-diagnostics", + "local-memory-governance", + "local-teams-reporting" + ], + "baselineContextBytes": 942, + "governedContextBytes": 620, + "baselinePrecision": 0.25, + "governedPrecision": 0.4, + "baselineRecall": 1, + "governedRecall": 1, + "baselineDecisionConsistent": false, + "governedDecisionConsistent": true + }, + { + "taskId": "browser-tests", + "baselineLoadedIds": [ + "policy-aspire-config", + "archive-raw-env-config", + "local-memory-diagnostics", + "local-memory-governance", + "archive-ci-rotation", + "never-secret", + "policy-headed-browser", + "local-teams-reporting" + ], + "governedLoadedIds": [ + "policy-aspire-config", + "policy-headed-browser" + ], + "baselineContextBytes": 942, + "governedContextBytes": 251, + "baselinePrecision": 0.125, + "governedPrecision": 0.5, + "baselineRecall": 1, + "governedRecall": 1, + "baselineDecisionConsistent": true, + "governedDecisionConsistent": true + }, + { + "taskId": "teams-update", + "baselineLoadedIds": [ + "policy-aspire-config", + "archive-raw-env-config", + "local-memory-diagnostics", + "local-memory-governance", + "archive-ci-rotation", + "never-secret", + "policy-headed-browser", + "local-teams-reporting" + ], + "governedLoadedIds": [ + "policy-aspire-config", + "policy-headed-browser", + "local-memory-diagnostics", + "local-teams-reporting" + ], + "baselineContextBytes": 942, + "governedContextBytes": 479, + "baselinePrecision": 0.125, + "governedPrecision": 0.25, + "baselineRecall": 1, + "governedRecall": 1, + "baselineDecisionConsistent": true, + "governedDecisionConsistent": true + } + ], + "baselineContextBytes": 3768, + "governedContextBytes": 1601, + "baselineEstimatedTokens": 942, + "governedEstimatedTokens": 401, + "contextReductionPercent": 57.510615711252655, + "baselinePrecision": 0.15625, + "governedPrecision": 0.4125, + "baselineRecall": 1, + "governedRecall": 1, + "baselineDecisionConsistency": 0.5, + "governedDecisionConsistency": 1, + "staleOrUnsafeFactsAvoided": 12, + "staleOrUnsafeFactsLoadedByBaseline": 12, + "verdict": "pass" +} diff --git a/test/characterization/_helpers/hermetic-root.ts b/test/characterization/_helpers/hermetic-root.ts new file mode 100644 index 000000000..7121a9844 --- /dev/null +++ b/test/characterization/_helpers/hermetic-root.ts @@ -0,0 +1,107 @@ +/** + * Hermetic helpers for characterization tests. + * + * Every characterization suite MUST run inside a fresh temp directory under + * `os.tmpdir()`. Repository `.squad/**` state is never test input and is + * never read, hashed, or listed by these tests. + * + * `makeWriteGuardedStorage` wraps any `StorageProvider` and asserts that + * every mutating call resolves to an absolute path that lives inside the + * hermetic root. Any escape throws a descriptive error that names only the + * offending operation and the temp root prefix; caller-machine paths outside + * the temp root are never surfaced in failure messages. + * + * This file lives under `_helpers/` so vitest's `test/**\/*.test.ts` include + * pattern does not treat it as a test file; it is imported by tests only. + */ + +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import type { StorageProvider } from '../../../packages/squad-sdk/src/storage/storage-provider.js'; + +/** + * Create a hermetic temp directory, run `fn` inside it, and remove it in a + * `finally` block. The directory lives under `os.tmpdir()` and never under + * the repo working directory. + */ +export async function withHermeticRoot( + fn: (root: string) => Promise, + prefix = 'squad-characterization-', +): Promise { + const root = await fs.mkdtemp(path.join(os.tmpdir(), prefix)); + try { + return await fn(root); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } +} + +const MUTATING_METHODS = new Set([ + 'write', + 'writeSync', + 'append', + 'appendSync', + 'delete', + 'deleteSync', + 'deleteDir', + 'deleteDirSync', + 'mkdir', + 'mkdirSync', + 'rename', + 'renameSync', + 'copy', + 'copySync', +]); + +/** Methods that take two path arguments (source, destination). */ +const TWO_PATH_METHODS = new Set(['rename', 'renameSync', 'copy', 'copySync']); + +function assertInsideRoot(root: string, target: string, operation: string): void { + const resolved = path.resolve(target); + const normalizedRoot = path.resolve(root); + const rootWithSep = normalizedRoot.endsWith(path.sep) ? normalizedRoot : normalizedRoot + path.sep; + if (resolved !== normalizedRoot && !resolved.startsWith(rootWithSep)) { + throw new Error( + `Hermetic write guard: refused ${operation} outside hermetic root (${normalizedRoot}). ` + + `A test attempted to write to a path that is not contained by this suite's temp directory.`, + ); + } +} + +/** + * Wrap a `StorageProvider` so every mutating method asserts the target path + * is contained by `root`. Non-mutating methods are forwarded unchanged. + */ +export function makeWriteGuardedStorage(base: StorageProvider, root: string): StorageProvider { + return new Proxy(base, { + get(target, propertyKey, receiver) { + const original = Reflect.get(target, propertyKey, receiver); + if (typeof original !== 'function' || typeof propertyKey !== 'string') { + return original; + } + if (!MUTATING_METHODS.has(propertyKey)) { + return original.bind(target); + } + return (...args: unknown[]): unknown => { + const first = args[0]; + if (typeof first !== 'string') { + throw new Error( + `Hermetic write guard: refused ${propertyKey} with non-string first argument.`, + ); + } + assertInsideRoot(root, first, propertyKey); + if (TWO_PATH_METHODS.has(propertyKey)) { + const second = args[1]; + if (typeof second !== 'string') { + throw new Error( + `Hermetic write guard: refused ${propertyKey} with non-string second argument.`, + ); + } + assertInsideRoot(root, second, propertyKey); + } + return (original as (...a: unknown[]) => unknown).apply(target, args); + }; + }, + }) as StorageProvider; +} diff --git a/test/characterization/classification-forbidden.characterization.test.ts b/test/characterization/classification-forbidden.characterization.test.ts new file mode 100644 index 000000000..dd80e9f79 --- /dev/null +++ b/test/characterization/classification-forbidden.characterization.test.ts @@ -0,0 +1,236 @@ +/** + * Characterization tests for classification and forbidden-content governance. + * + * Covers: + * Gap C1 - one row per FORBIDDEN_PATTERNS entry, positive rejection plus + * negative near-miss, with audit safety pinned exactly. + * Gap C2 - one canonical + one ambiguous sample per non-forbidden auto-class. + * + * All inputs are hand-authored synthetic strings, safe for a public repo. + * No real secrets, no real PII, no real customer data. If any row is observed + * to leak raw synthetic input into the audit record (contradicting the current + * safe-by-default design), that row must be marked `it.todo` with a + * "SECURITY REGRESSION CANDIDATE" comment and escalated in the PR body; the + * production source is not edited by this PR. + */ + +import { describe, expect, it } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; + +import { FSStorageProvider } from '../../packages/squad-sdk/src/storage/fs-storage-provider.js'; +import { LocalMemoryStore } from '../../packages/squad-sdk/src/memory/index.js'; + +import { makeWriteGuardedStorage, withHermeticRoot } from './_helpers/hermetic-root.js'; + +const REJECTION_REASON_PREFIX = 'Rejected as forbidden memory: '; +const SAFE_AUDIT_TITLE = 'Rejected governed memory'; + +async function readJsonl(file: string): Promise>> { + const raw = await fs.readFile(file, 'utf8'); + return raw + .split('\n') + .filter((line) => line.length > 0) + .map((line) => JSON.parse(line) as Record); +} + +interface ForbiddenRow { + label: string; + reasonSuffix: string; + positive: string; + negative: string; +} + +// One row per pattern in FORBIDDEN_PATTERNS (12 total). The `reasonSuffix` is +// the exact string appended to the pattern's rejection reason by classify(). +const FORBIDDEN_ROWS: ForbiddenRow[] = [ + { + label: 'private key material', + reasonSuffix: 'private key material', + positive: '-----BEGIN RSA PRIVATE KEY-----\nsynthetic body\n-----END RSA PRIVATE KEY-----', + negative: 'Discussion about key rotation policy for private keys next quarter', + }, + { + label: 'access token', + reasonSuffix: 'access token', + positive: 'Token used in the test fixture: ghp_ABCDEFGHIJKLMNOPQR12', + negative: 'Rotate tokens quarterly across all repos', + }, + { + label: 'credential-like assignment', + reasonSuffix: 'credential-like assignment', + positive: 'password: hunter2extra', + negative: 'We rotate passwords quarterly', + }, + { + label: 'connection string secret', + reasonSuffix: 'connection string secret', + positive: 'AccountKey=syntheticFixtureValue123', + negative: 'Rotating the storage account key annually', + }, + { + label: 'PII-like identifier', + reasonSuffix: 'PII-like identifier', + positive: 'Reference 123-45-6789 in the synthetic fixture', + negative: 'Order 12345-6789 shipped on schedule', + }, + { + label: 'internal network topology', + reasonSuffix: 'internal network topology', + positive: 'Test server reachable at 192.168.1.5', + negative: 'Public resolver at 8.8.8.8 works fine', + }, + { + label: 'raw diagnostic payload', + reasonSuffix: 'raw diagnostic payload', + positive: 'Attached raw logs from the failing job', + negative: 'Log rotation runs nightly', + }, + { + label: 'transient CI/PR status', + reasonSuffix: 'transient CI/PR status', + positive: 'CI failed on the main branch', + negative: 'CI hygiene review scheduled', + }, + { + label: 'private customer data', + reasonSuffix: 'private customer data', + positive: 'Do not share private customer data externally', + negative: 'Customer feedback survey shows a positive trend', + }, + { + label: 'private customer data (alt)', + reasonSuffix: 'private customer data', + positive: 'Customer PII must be masked in exports', + negative: 'Customer engagement metrics rose', + }, + { + label: 'unreviewed vulnerability disclosure', + reasonSuffix: 'unreviewed vulnerability disclosure', + positive: 'Filed unreviewed vulnerabilities in the tracker', + negative: 'Vulnerability review process runs weekly', + }, + { + label: 'zero-day disclosure', + reasonSuffix: 'unreviewed vulnerability disclosure', + positive: 'Suspected zero-day in library X', + negative: 'Daytime standups happen at ten', + }, +]; + +describe('classification and forbidden-content characterization (C1)', () => { + it.each(FORBIDDEN_ROWS)( + 'rejects and audits safely for pattern: $label', + async (row) => { + await withHermeticRoot(async (root) => { + const storage = makeWriteGuardedStorage(new FSStorageProvider(), root); + const store = new LocalMemoryStore(storage, root); + + const result = await store.write({ + content: row.positive, + title: 'do not care', + author: 'characterization-suite', + }); + + expect(result.stored).toBe(false); + expect(result.classification.class).toBe('FORBIDDEN'); + expect(result.classification.allowed).toBe(false); + expect(result.classification.reason).toBe(REJECTION_REASON_PREFIX + row.reasonSuffix); + + const auditPath = path.join(root, '.squad', 'memory', 'audit.jsonl'); + const auditRecords = await readJsonl(auditPath); + const rejectRecords = auditRecords.filter((r) => r.action === 'reject'); + expect(rejectRecords).toHaveLength(1); + const record = rejectRecords[0]; + + // Pin current safe-by-default behavior exactly. Any regression that + // starts leaking raw content into `title` or `reason` will fail here. + expect(record.title).toBe(SAFE_AUDIT_TITLE); + expect(record.reason).toBe(REJECTION_REASON_PREFIX + row.reasonSuffix); + expect(record.class).toBe('FORBIDDEN'); + }); + }, + ); + + it.each(FORBIDDEN_ROWS)( + 'accepts the near-miss negative for pattern: $label', + async (row) => { + await withHermeticRoot(async (root) => { + const storage = makeWriteGuardedStorage(new FSStorageProvider(), root); + const store = new LocalMemoryStore(storage, root); + + const classification = await store.classify({ content: row.negative }); + expect(classification.class).not.toBe('FORBIDDEN'); + }); + }, + ); +}); + +interface ClassifyRow { + label: string; + canonical: { content: string; expected: string }; + ambiguous: { content: string; expected: string }; +} + +// One canonical + one ambiguous input per non-forbidden auto-class. Ambiguous +// expectations are inline snapshots of current behavior; comments note that +// #1309 may revisit these boundaries. This PR does not encode target behavior. +const CLASSIFY_ROWS: ClassifyRow[] = [ + { + label: 'TRANSIENT', + canonical: { content: 'CI failed on the release branch', expected: 'TRANSIENT' }, + // "build velocity" contains no CI/PR/build+status pair, so falls through + // to LOCAL under the current heuristic. + ambiguous: { content: 'Interested in build velocity', expected: 'LOCAL' }, + }, + { + label: 'POLICY', + canonical: { content: 'Always run tests before merging', expected: 'POLICY' }, + // Leading "We" defeats the ^-anchored POLICY regex, so the sentence + // falls through to DECISION via "decision" or LOCAL. Current behavior: + // "must" is not at start-of-string so no POLICY match; content mentions + // no decision/adopt/etc, so LOCAL. May be revisited by #1309. + ambiguous: { content: 'We must document milestones weekly', expected: 'LOCAL' }, + }, + { + label: 'DECISION', + canonical: { content: 'Decided to adopt TypeScript strict mode', expected: 'DECISION' }, + // "decision-making" contains the token "decision" bounded by \b, so the + // current heuristic classifies as DECISION. May be revisited by #1309. + ambiguous: { content: 'The decision-making process is opaque', expected: 'DECISION' }, + }, + { + label: 'COPILOT_MEMORY', + canonical: { content: 'Persist this in copilot memory', expected: 'COPILOT_MEMORY' }, + // "semantic memory" phrase matches the COPILOT_MEMORY heuristic even + // outside a first-person "persist" framing. May be revisited by #1309. + ambiguous: { content: 'Semantic memory research is trending', expected: 'COPILOT_MEMORY' }, + }, + { + label: 'LOCAL', + canonical: { content: 'Refactored the parser module for clarity', expected: 'LOCAL' }, + // Short observational content with no trigger tokens falls through to LOCAL. + ambiguous: { content: 'Testing edge cases in the workflow', expected: 'LOCAL' }, + }, +]; + +describe('classify heuristic characterization (C2)', () => { + it.each(CLASSIFY_ROWS)('canonical sample classifies as $label', async (row) => { + await withHermeticRoot(async (root) => { + const storage = makeWriteGuardedStorage(new FSStorageProvider(), root); + const store = new LocalMemoryStore(storage, root); + const classification = await store.classify({ content: row.canonical.content }); + expect(classification.class).toBe(row.canonical.expected); + }); + }); + + it.each(CLASSIFY_ROWS)('ambiguous sample near $label classifies to current class', async (row) => { + await withHermeticRoot(async (root) => { + const storage = makeWriteGuardedStorage(new FSStorageProvider(), root); + const store = new LocalMemoryStore(storage, root); + const classification = await store.classify({ content: row.ambiguous.content }); + // Current behavior; may be revisited by #1309. + expect(classification.class).toBe(row.ambiguous.expected); + }); + }); +}); diff --git a/test/characterization/history-shadow.characterization.test.ts b/test/characterization/history-shadow.characterization.test.ts new file mode 100644 index 000000000..c2271ccc0 --- /dev/null +++ b/test/characterization/history-shadow.characterization.test.ts @@ -0,0 +1,128 @@ +/** + * Characterization tests for history-shadow round-trip and merge behavior. + * + * Covers Gap H1: round-trip byte-identity per canonical section, unknown + * section append fallback, and timestamp format stability. Concurrency and + * happy-path lifecycle are already covered by `test/history-shadow.test.ts` + * and are intentionally not duplicated here. + * + * All I/O is contained inside a hermetic temp root; repository `.squad/**` + * is never touched. + */ + +import { describe, expect, it } from 'vitest'; +import * as path from 'node:path'; +import * as fs from 'node:fs/promises'; + +import { FSStorageProvider } from '../../packages/squad-sdk/src/storage/fs-storage-provider.js'; +import { + appendToHistory, + createHistoryShadow, + type HistorySection, +} from '../../packages/squad-sdk/src/agents/history-shadow.js'; + +import { makeWriteGuardedStorage, withHermeticRoot } from './_helpers/hermetic-root.js'; + +const CANONICAL_SECTIONS: HistorySection[] = [ + 'Context', + 'Learnings', + 'Decisions', + 'Patterns', + 'Issues', + 'References', +]; + +const TIMESTAMP_LINE = /^### (\d{4}-\d{2}-\d{2})$/m; + +describe('history-shadow characterization (H1)', () => { + it.each(CANONICAL_SECTIONS)( + 'appends a byte-identical block to the %s section', + async (section) => { + await withHermeticRoot(async (root) => { + const storage = makeWriteGuardedStorage(new FSStorageProvider(), root); + const agent = 'char-agent'; + const content = `sample body for ${section} section with punctuation: !? and unicode \u2603`; + + const shadowPath = await createHistoryShadow(root, agent, undefined, storage); + await appendToHistory(root, agent, section, content, storage); + + const raw = await fs.readFile(shadowPath, 'utf8'); + + const sectionHeader = `## ${section}`; + const headerIndex = raw.indexOf(sectionHeader); + expect(headerIndex, `section header ${sectionHeader} missing`).toBeGreaterThanOrEqual(0); + + // Slice from the section header to the next top-level header (or EOF). + const afterHeader = raw.slice(headerIndex + sectionHeader.length); + const nextHeaderMatch = afterHeader.match(/\n## /); + const sectionBody = nextHeaderMatch + ? afterHeader.slice(0, nextHeaderMatch.index) + : afterHeader; + + // The most recent entry, as emitted by production code, is: + // "\n### \n\n\n" + // We assert the byte-exact tail of the section body matches this shape. + const timestampMatch = sectionBody.match(TIMESTAMP_LINE); + expect(timestampMatch, 'timestamp header missing').not.toBeNull(); + const timestamp = timestampMatch![1]; + const expectedEntry = `\n### ${timestamp}\n\n${content}\n`; + expect(sectionBody.endsWith(expectedEntry)).toBe(true); + }); + }, + ); + + it('appends an unknown section at the end when no matching header exists', async () => { + await withHermeticRoot(async (root) => { + const storage = makeWriteGuardedStorage(new FSStorageProvider(), root); + const agent = 'char-agent-unknown'; + const shadowPath = await createHistoryShadow(root, agent, undefined, storage); + const before = await fs.readFile(shadowPath, 'utf8'); + + // Cast is required because HistorySection is a closed union at compile + // time; we characterize the *runtime* fallback path for a section name + // that is not in the canonical set. + const unknownSection = 'ExperimentalNotes' as HistorySection; + const content = 'entry into a novel section'; + await appendToHistory(root, agent, unknownSection, content, storage); + + const after = await fs.readFile(shadowPath, 'utf8'); + + // Current behavior: the unknown section is appended verbatim at the tail + // in the shape `\n\n##
\n### \n\n\n`. + const trimmed = before.trimEnd(); + const timestampMatch = after.match(TIMESTAMP_LINE); + expect(timestampMatch, 'timestamp header missing').not.toBeNull(); + const timestamp = timestampMatch![1]; + const expectedTail = `${trimmed}\n\n## ${unknownSection}\n### ${timestamp}\n\n${content}\n`; + expect(after).toBe(expectedTail); + }); + }); + + it('produces a YYYY-MM-DD timestamp header for each entry', async () => { + await withHermeticRoot(async (root) => { + const storage = makeWriteGuardedStorage(new FSStorageProvider(), root); + const agent = 'char-agent-timestamp'; + const shadowPath = await createHistoryShadow(root, agent, undefined, storage); + await appendToHistory(root, agent, 'Context', 'first entry', storage); + + const raw = await fs.readFile(shadowPath, 'utf8'); + const match = raw.match(TIMESTAMP_LINE); + expect(match).not.toBeNull(); + // Round-trip the same regex source to make future format drift diff loudly. + const echo = new RegExp(TIMESTAMP_LINE.source, TIMESTAMP_LINE.flags); + expect(match![0]).toMatch(echo); + }); + }); + + it('write guard reports zero out-of-root writes for this suite', async () => { + // The suite passes only if every prior test's writes stayed inside the + // hermetic root. Any escape would have thrown from `makeWriteGuardedStorage`. + // This test exists to make the guarantee grep-obvious in the suite output. + await withHermeticRoot(async (root) => { + const storage = makeWriteGuardedStorage(new FSStorageProvider(), root); + const target = path.join(root, 'inside.txt'); + await storage.write(target, 'ok'); + expect(await storage.exists(target)).toBe(true); + }); + }); +}); diff --git a/test/characterization/local-memory-store.characterization.test.ts b/test/characterization/local-memory-store.characterization.test.ts new file mode 100644 index 000000000..09b4ddf4e --- /dev/null +++ b/test/characterization/local-memory-store.characterization.test.ts @@ -0,0 +1,152 @@ +/** + * Characterization tests for LocalMemoryStore. + * + * Covers: + * Gap M1 - index-lock no-loss invariant on N concurrent writes. + * Gap M2 - tombstone-before-source-delete ordering invariant on delete(). + * + * Full write/search/promote/audit lifecycle coverage lives in + * `test/memory-governance.test.ts` and is intentionally not duplicated here. + */ + +import { describe, expect, it } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; + +import { FSStorageProvider } from '../../packages/squad-sdk/src/storage/fs-storage-provider.js'; +import { LocalMemoryStore } from '../../packages/squad-sdk/src/memory/index.js'; + +import { makeWriteGuardedStorage, withHermeticRoot } from './_helpers/hermetic-root.js'; + +async function readJsonl(file: string): Promise { + const raw = await fs.readFile(file, 'utf8'); + return raw + .split('\n') + .filter((line) => line.length > 0) + .map((line, idx) => { + try { + return JSON.parse(line); + } catch (err) { + throw new Error( + `audit.jsonl line ${idx + 1} is not valid JSON: ${(err as Error).message}. Raw line length=${line.length}.`, + ); + } + }); +} + +describe('LocalMemoryStore characterization', () => { + it.each([5, 20])('serializes %d concurrent writes without loss (M1)', async (n) => { + await withHermeticRoot(async (root) => { + const storage = makeWriteGuardedStorage(new FSStorageProvider(), root); + const store = new LocalMemoryStore(storage, root); + + const results = await Promise.all( + Array.from({ length: n }, (_, i) => + store.write({ + content: `synthetic characterization payload ${i}`, + title: `entry ${i}`, + author: 'characterization-suite', + }), + ), + ); + + const stored = results.filter((r) => r.stored); + expect(stored).toHaveLength(n); + const idSet = new Set(stored.map((r) => r.id!)); + expect(idSet.size).toBe(n); + + const indexPath = path.join(root, '.squad', 'memory', 'index.json'); + const rawIndex = await fs.readFile(indexPath, 'utf8'); + const parsedIndex = JSON.parse(rawIndex) as Array<{ id: string; status: string }>; + expect(parsedIndex).toHaveLength(n); + const indexIds = new Set(parsedIndex.map((e) => e.id)); + expect(indexIds).toEqual(idSet); + + // audit.jsonl invariant: every line parses; write actions for our ids + // are all present; no partial or truncated lines. + const auditPath = path.join(root, '.squad', 'memory', 'audit.jsonl'); + const auditRecords = (await readJsonl(auditPath)) as Array<{ + action: string; + id?: string; + }>; + const writeIds = new Set( + auditRecords.filter((r) => r.action === 'write').map((r) => r.id), + ); + for (const id of idSet) { + expect(writeIds.has(id), `audit missing write action for id ${id}`).toBe(true); + } + }); + }); + + it('writes tombstone to disk before removing the source file (M2)', async () => { + await withHermeticRoot(async (root) => { + const baseStorage = makeWriteGuardedStorage(new FSStorageProvider(), root); + const store = new LocalMemoryStore(baseStorage, root); + + const writeResult = await store.write({ + content: 'synthetic entry to be deleted', + title: 'delete-me', + author: 'characterization-suite', + }); + expect(writeResult.stored).toBe(true); + const id = writeResult.id!; + const tombstonePath = path.join(root, '.squad', 'memory', 'tombstones', `${id}.json`); + const sourceAbsolute = path.join(root, writeResult.path!); + + // Sanity: tombstone does not exist yet; source does. + await expect(fs.access(tombstonePath)).rejects.toThrow(); + await expect(fs.access(sourceAbsolute)).resolves.toBeUndefined(); + + // Wrap storage.delete so we can observe on-disk state at the moment the + // source is unlinked. LocalMemoryStore is reflected onto our wrapper via + // its private `storage` field so we do not need a second instance. + const observations: Array<{ target: string; tombstoneExistsAtCall: boolean }> = []; + const wrapped: typeof baseStorage = new Proxy(baseStorage, { + get(target, propertyKey, receiver) { + if (propertyKey === 'delete') { + const original = Reflect.get(target, propertyKey, receiver) as ( + p: string, + ) => Promise; + return async (targetPath: string) => { + let exists = false; + try { + await fs.access(tombstonePath); + exists = true; + } catch { + exists = false; + } + observations.push({ target: targetPath, tombstoneExistsAtCall: exists }); + return original.call(target, targetPath); + }; + } + return Reflect.get(target, propertyKey, receiver); + }, + }); + // Swap the store's storage reference to the observer. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (store as any).storage = wrapped; + + const deleted = await store.delete(id, 'characterization-suite'); + expect(deleted).toBe(true); + + // At least one delete call was made against the source; every such call + // saw the tombstone already on disk. + const sourceDeletes = observations.filter((o) => o.target === sourceAbsolute); + expect(sourceDeletes.length).toBeGreaterThanOrEqual(1); + for (const obs of sourceDeletes) { + expect(obs.tombstoneExistsAtCall).toBe(true); + } + + // Post-conditions: source removed, tombstone persisted, audit updated. + await expect(fs.access(sourceAbsolute)).rejects.toThrow(); + await expect(fs.access(tombstonePath)).resolves.toBeUndefined(); + const auditPath = path.join(root, '.squad', 'memory', 'audit.jsonl'); + const auditRecords = (await readJsonl(auditPath)) as Array<{ + action: string; + id?: string; + }>; + const deleteRecord = auditRecords.find((r) => r.action === 'delete' && r.id === id); + expect(deleteRecord).toBeDefined(); + }); + }); +}); diff --git a/test/characterization/spawn-context-baseline.characterization.test.ts b/test/characterization/spawn-context-baseline.characterization.test.ts new file mode 100644 index 000000000..e8b7594e4 --- /dev/null +++ b/test/characterization/spawn-context-baseline.characterization.test.ts @@ -0,0 +1,44 @@ +/** + * Characterization test for spawn-context byte counts (Gap S1). + * + * Pins byte-exact JSON output of `runMemoryValueBenchmark()` for the + * default fixture. Any future change to the benchmark output, the fixture, + * or the assembly logic will diff loudly against the committed golden. + * + * This is a baseline of *current* behavior; it does not encode any target + * tier design and it does not evaluate the #1309 proposal. + */ + +import { describe, expect, it } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { runMemoryValueBenchmark } from '../../packages/squad-sdk/src/runtime/memory-value-benchmark.js'; + +import { withHermeticRoot } from './_helpers/hermetic-root.js'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const GOLDEN_PATH = path.resolve( + HERE, + '..', + '..', + 'test-fixtures', + 'characterization', + 'spawn-context-baseline.json', +); + +describe('spawn-context baseline characterization (S1)', () => { + it('runMemoryValueBenchmark output matches the committed golden byte for byte', async () => { + // Defensive: run under a hermetic root so any accidental future filesystem + // side effect is caught. The current benchmark is pure and in-memory. + await withHermeticRoot(async (_root) => { + const report = runMemoryValueBenchmark(); + const actual = JSON.stringify(report, null, 2) + '\n'; + const goldenRaw = await fs.readFile(GOLDEN_PATH, 'utf8'); + // Byte-exact equality; failure prints only the two JSON blobs, never + // caller-machine paths. + expect(actual).toBe(goldenRaw); + }); + }); +}); From 0c6dd7b0ba31a9769bb312df57fb12d80b00e854 Mon Sep 17 00:00:00 2001 From: Copilot Date: Sat, 18 Jul 2026 17:24:11 +0300 Subject: [PATCH 2/3] test(memory): correct characterization assertions to match production behavior Revise three characterization test files to describe what the code actually does today, not what an earlier draft assumed it does. No production source is touched. classification-forbidden.characterization.test.ts: - The forbidden-write audit test expected every rejected write with a safe caller title to be renamed to the SAFE_AUDIT_TITLE placeholder. In fact safeAuditTitle() preserves a caller-supplied title verbatim unless the title itself matches one of the FORBIDDEN_PATTERNS entries. All twelve canonical rows here supply the literal safe title do not care, so the assertion now correctly expects that literal string. - The TRANSIENT canonical sample CI failed on the release branch was expected to auto-classify as TRANSIENT. Its content is byte-for-byte identical to a FORBIDDEN_PATTERNS entry, and the forbidden loop in classify() runs first and unconditionally, so this content always classifies FORBIDDEN. The heuristic branch for TRANSIENT is unreachable through content alone. The row now expects FORBIDDEN, with a comment documenting the shadowing, plus a new test showing the one genuine reachable path to TRANSIENT: an explicit requestedClass override on benign content that matches no forbidden pattern. history-shadow.characterization.test.ts: - appendToHistory's section-boundary regex uses a Perl-style end-of-string anchor that JavaScript parses as a literal Z character, not an anchor. This produces two distinct effects: a duplicated section header when appending to the last section, and a missing blank line separator when appending to any non-last section. The suite now pins both effects as currently-observed behavior rather than asserting the previously assumed, and incorrect, well-formed output. A companion issue describing the underlying production defect is filed separately; production code is not changed here. hermetic-root.ts: - The write guard compared a mutating call's first argument against the hermetic root using string containment only, without first resolving a relative target against that root. A relative path could pass the containment check while the underlying storage call still resolved it against process.cwd(), landing outside the guarded directory. The guard now resolves every target against the hermetic root before the containment check and forwards the resolved, root-anchored path to the wrapped storage call, so relative and absolute targets are handled identically and consistently land inside the temp root. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 44f53a4e-d384-4c60-a43d-cfc158160f1f --- .../_helpers/hermetic-root.ts | 24 +++- ...ication-forbidden.characterization.test.ts | 51 ++++++- .../history-shadow.characterization.test.ts | 125 +++++++++++++++--- 3 files changed, 176 insertions(+), 24 deletions(-) diff --git a/test/characterization/_helpers/hermetic-root.ts b/test/characterization/_helpers/hermetic-root.ts index 7121a9844..019645ad9 100644 --- a/test/characterization/_helpers/hermetic-root.ts +++ b/test/characterization/_helpers/hermetic-root.ts @@ -57,9 +57,18 @@ const MUTATING_METHODS = new Set([ /** Methods that take two path arguments (source, destination). */ const TWO_PATH_METHODS = new Set(['rename', 'renameSync', 'copy', 'copySync']); -function assertInsideRoot(root: string, target: string, operation: string): void { - const resolved = path.resolve(target); +function resolveInsideRoot(root: string, target: string, operation: string): string { const normalizedRoot = path.resolve(root); + // Resolve a relative target against the hermetic root, not against + // process.cwd(). path.resolve(normalizedRoot, target) ignores the first + // argument when target is already absolute, so this is safe for both + // absolute and relative targets and keeps containment deterministic + // regardless of the test runner's working directory. The resolved, + // root-anchored path is returned so the caller can forward it to the + // underlying storage instead of the original (possibly relative) target, + // otherwise a relative target would still hit the real filesystem + // relative to process.cwd(), defeating the containment check entirely. + const resolved = path.resolve(normalizedRoot, target); const rootWithSep = normalizedRoot.endsWith(path.sep) ? normalizedRoot : normalizedRoot + path.sep; if (resolved !== normalizedRoot && !resolved.startsWith(rootWithSep)) { throw new Error( @@ -67,6 +76,7 @@ function assertInsideRoot(root: string, target: string, operation: string): void `A test attempted to write to a path that is not contained by this suite's temp directory.`, ); } + return resolved; } /** @@ -90,7 +100,11 @@ export function makeWriteGuardedStorage(base: StorageProvider, root: string): St `Hermetic write guard: refused ${propertyKey} with non-string first argument.`, ); } - assertInsideRoot(root, first, propertyKey); + // Forward the resolved, root-anchored path rather than the raw + // argument so a relative target actually lands inside the hermetic + // root on disk, not just passes the containment check. + const resolvedArgs = [...args]; + resolvedArgs[0] = resolveInsideRoot(root, first, propertyKey); if (TWO_PATH_METHODS.has(propertyKey)) { const second = args[1]; if (typeof second !== 'string') { @@ -98,9 +112,9 @@ export function makeWriteGuardedStorage(base: StorageProvider, root: string): St `Hermetic write guard: refused ${propertyKey} with non-string second argument.`, ); } - assertInsideRoot(root, second, propertyKey); + resolvedArgs[1] = resolveInsideRoot(root, second, propertyKey); } - return (original as (...a: unknown[]) => unknown).apply(target, args); + return (original as (...a: unknown[]) => unknown).apply(target, resolvedArgs); }; }, }) as StorageProvider; diff --git a/test/characterization/classification-forbidden.characterization.test.ts b/test/characterization/classification-forbidden.characterization.test.ts index dd80e9f79..6bab14fa9 100644 --- a/test/characterization/classification-forbidden.characterization.test.ts +++ b/test/characterization/classification-forbidden.characterization.test.ts @@ -24,6 +24,14 @@ import { LocalMemoryStore } from '../../packages/squad-sdk/src/memory/index.js'; import { makeWriteGuardedStorage, withHermeticRoot } from './_helpers/hermetic-root.js'; const REJECTION_REASON_PREFIX = 'Rejected as forbidden memory: '; + +// The placeholder title `safeAuditTitle()` substitutes when a title is +// missing or is itself forbidden-shaped. It is NOT what gets recorded when +// the caller supplies a safe title (see the FORBIDDEN_ROWS test below, which +// always supplies the safe title 'do not care'). The missing-title path is +// already covered by test/memory-governance.test.ts +// ("uses a safe placeholder title for no-title rejected sensitive writes"), +// so it is intentionally not duplicated here. const SAFE_AUDIT_TITLE = 'Rejected governed memory'; async function readJsonl(file: string): Promise>> { @@ -143,9 +151,14 @@ describe('classification and forbidden-content characterization (C1)', () => { expect(rejectRecords).toHaveLength(1); const record = rejectRecords[0]; - // Pin current safe-by-default behavior exactly. Any regression that - // starts leaking raw content into `title` or `reason` will fail here. - expect(record.title).toBe(SAFE_AUDIT_TITLE); + // Pin current behavior exactly: safeAuditTitle() preserves a caller + // supplied title verbatim as long as the title itself does not match + // a forbidden pattern, so the recorded title is the literal 'do not + // care' string this suite always supplies, not the SAFE_AUDIT_TITLE + // placeholder (that placeholder is only substituted when the title + // is missing or is itself forbidden-shaped). Any regression that + // starts leaking raw content into `reason` will still fail below. + expect(record.title).toBe('do not care'); expect(record.reason).toBe(REJECTION_REASON_PREFIX + row.reasonSuffix); expect(record.class).toBe('FORBIDDEN'); }); @@ -178,7 +191,17 @@ interface ClassifyRow { const CLASSIFY_ROWS: ClassifyRow[] = [ { label: 'TRANSIENT', - canonical: { content: 'CI failed on the release branch', expected: 'TRANSIENT' }, + // The TRANSIENT auto-classification heuristic + // (/\b(CI|PR|build)\s+(status|failed|passed|output|log)\b/i) is shadowed + // by an identical pattern in FORBIDDEN_PATTERNS (reason: "transient + // CI/PR status"), which classify() checks first, unconditionally. Any + // content that would trip the TRANSIENT heuristic trips the FORBIDDEN + // check first, so this canonical sample classifies FORBIDDEN, not + // TRANSIENT. The TRANSIENT heuristic branch is unreachable via + // content-only input; see the dedicated + // "TRANSIENT is reachable only via an explicit requestedClass override" + // test below for the one real public path that does produce TRANSIENT. + canonical: { content: 'CI failed on the release branch', expected: 'FORBIDDEN' }, // "build velocity" contains no CI/PR/build+status pair, so falls through // to LOCAL under the current heuristic. ambiguous: { content: 'Interested in build velocity', expected: 'LOCAL' }, @@ -233,4 +256,24 @@ describe('classify heuristic characterization (C2)', () => { expect(classification.class).toBe(row.ambiguous.expected); }); }); + + it('TRANSIENT is reachable only via an explicit requestedClass override', async () => { + await withHermeticRoot(async (root) => { + const storage = makeWriteGuardedStorage(new FSStorageProvider(), root); + const store = new LocalMemoryStore(storage, root); + // This content matches none of the FORBIDDEN_PATTERNS entries, so the + // unconditional forbidden check at the top of classify() falls through + // and the requestedClass override below is honored as-is. This is the + // one real, currently-reachable public path to a TRANSIENT + // classification; the content-matching heuristic for TRANSIENT is + // otherwise unreachable, see the CLASSIFY_ROWS 'TRANSIENT' comment above. + const classification = await store.classify({ + content: 'Quick scratch note for today', + requestedClass: 'TRANSIENT', + }); + expect(classification.class).toBe('TRANSIENT'); + expect(classification.allowed).toBe(false); + expect(classification.reason).toBe('Transient task state is not persisted as durable memory'); + }); + }); }); diff --git a/test/characterization/history-shadow.characterization.test.ts b/test/characterization/history-shadow.characterization.test.ts index c2271ccc0..2e71d3aa1 100644 --- a/test/characterization/history-shadow.characterization.test.ts +++ b/test/characterization/history-shadow.characterization.test.ts @@ -1,10 +1,25 @@ /** * Characterization tests for history-shadow round-trip and merge behavior. * - * Covers Gap H1: round-trip byte-identity per canonical section, unknown - * section append fallback, and timestamp format stability. Concurrency and - * happy-path lifecycle are already covered by `test/history-shadow.test.ts` - * and are intentionally not duplicated here. + * Covers Gap H1: append shape per canonical section, unknown section append + * fallback, and timestamp format stability. Concurrency and happy-path + * lifecycle are already covered by `test/history-shadow.test.ts` and are + * intentionally not duplicated here. + * + * Current-bug note: the section-boundary regex in `appendToHistory` + * (`packages/squad-sdk/src/agents/history-shadow.ts`) is built as + * `` (?=^##\s|\Z) `` and intends `\Z` to mean "end of string". JavaScript has + * no `\Z` end-of-string escape; it is parsed as the literal character "Z". + * For every section except the last one, a following `## ` header still + * lets the lookahead succeed, so those appends work, but the same + * `trimEnd()`-before-splice step also collapses the blank line that used to + * separate sections. For the terminal section (`References`, which has no + * following header and no literal "Z" in its body) the lookahead can never + * succeed, so the existing header is never matched and a *second* + * `## References` header is appended at the tail of the file instead of the + * entry being inserted inline. Both effects are characterized below exactly + * as they behave today; neither assertion below endorses the behavior as + * desired. See the tracked follow-up issue for the production fix. * * All I/O is contained inside a hermetic temp root; repository `.squad/**` * is never touched. @@ -32,11 +47,18 @@ const CANONICAL_SECTIONS: HistorySection[] = [ 'References', ]; +// The terminal section is characterized separately below: current +// production behavior duplicates its header instead of appending inline +// (see file-level comment). The remaining sections are each followed by +// another section header, so their append path succeeds. +const NON_TERMINAL_SECTIONS = CANONICAL_SECTIONS.slice(0, -1); +const TERMINAL_SECTION = CANONICAL_SECTIONS[CANONICAL_SECTIONS.length - 1]; + const TIMESTAMP_LINE = /^### (\d{4}-\d{2}-\d{2})$/m; describe('history-shadow characterization (H1)', () => { - it.each(CANONICAL_SECTIONS)( - 'appends a byte-identical block to the %s section', + it.each(NON_TERMINAL_SECTIONS)( + 'appends a block to the %s section, immediately abutting the next header', async (section) => { await withHermeticRoot(async (root) => { const storage = makeWriteGuardedStorage(new FSStorageProvider(), root); @@ -49,24 +71,80 @@ describe('history-shadow characterization (H1)', () => { const raw = await fs.readFile(shadowPath, 'utf8'); const sectionHeader = `## ${section}`; + expect( + raw.split(sectionHeader).length - 1, + `${sectionHeader} should appear exactly once`, + ).toBe(1); + const headerIndex = raw.indexOf(sectionHeader); expect(headerIndex, `section header ${sectionHeader} missing`).toBeGreaterThanOrEqual(0); - // Slice from the section header to the next top-level header (or EOF). + // Slice from the section header to the next top-level header. const afterHeader = raw.slice(headerIndex + sectionHeader.length); const nextHeaderMatch = afterHeader.match(/\n## /); - const sectionBody = nextHeaderMatch - ? afterHeader.slice(0, nextHeaderMatch.index) - : afterHeader; + expect(nextHeaderMatch, 'a following section header is expected').not.toBeNull(); + const sectionBody = afterHeader.slice(0, nextHeaderMatch!.index); - // The most recent entry, as emitted by production code, is: - // "\n### \n\n\n" - // We assert the byte-exact tail of the section body matches this shape. const timestampMatch = sectionBody.match(TIMESTAMP_LINE); expect(timestampMatch, 'timestamp header missing').not.toBeNull(); const timestamp = timestampMatch![1]; - const expectedEntry = `\n### ${timestamp}\n\n${content}\n`; - expect(sectionBody.endsWith(expectedEntry)).toBe(true); + + // Current behavior: appendToHistory trims trailing whitespace off + // the prior section content before splicing in the new entry. That + // collapses the blank line that originally separated sections, so + // the entry's own trailing newline becomes the single newline + // immediately preceding the next `## ` header rather than a + // preserved blank line plus a separate separator. The section body, + // as sliced up to that boundary, therefore ends with the entry text + // minus its final newline (that newline is the boundary itself). + const entryWithoutTrailingNewline = `\n### ${timestamp}\n\n${content}`; + expect(sectionBody.endsWith(entryWithoutTrailingNewline)).toBe(true); + }); + }, + ); + + it( + `current bug: appending to the terminal ${TERMINAL_SECTION} section duplicates its header ` + + 'instead of inserting inline', + async () => { + await withHermeticRoot(async (root) => { + const storage = makeWriteGuardedStorage(new FSStorageProvider(), root); + const agent = 'char-agent-terminal'; + const content = `sample body for ${TERMINAL_SECTION} section with punctuation: !? and unicode \u2603`; + + const shadowPath = await createHistoryShadow(root, agent, undefined, storage); + const before = await fs.readFile(shadowPath, 'utf8'); + await appendToHistory(root, agent, TERMINAL_SECTION, content, storage); + const after = await fs.readFile(shadowPath, 'utf8'); + + const sectionHeader = `## ${TERMINAL_SECTION}`; + + // See file-level comment: the `\Z` typo means the section-boundary + // regex never matches for the terminal section, so appendToHistory + // falls back to its "section not found" branch and appends a brand + // new header at the tail instead of writing into the existing one. + const occurrences = after.split(sectionHeader).length - 1; + expect(occurrences, 'current bug duplicates the header').toBe(2); + + const firstIndex = after.indexOf(sectionHeader); + const secondIndex = after.indexOf(sectionHeader, firstIndex + sectionHeader.length); + expect(secondIndex).toBeGreaterThan(firstIndex); + + // The original section's content is left untouched by the append + // (aside from trailing-whitespace normalization performed by the + // "section not found" branch's own trimEnd(), which happens before + // the duplicate header is spliced in). + const originalSectionSlice = before.slice(before.indexOf(sectionHeader)); + const untouchedSlice = after.slice(firstIndex, secondIndex); + expect(untouchedSlice.trimEnd()).toBe(originalSectionSlice.trimEnd()); + + // The duplicated (second) header starts a fresh section containing + // only the new entry, and is the exact tail of the file. + const timestampMatch = after.match(TIMESTAMP_LINE); + expect(timestampMatch, 'timestamp header missing').not.toBeNull(); + const timestamp = timestampMatch![1]; + const expectedTail = `${sectionHeader}\n### ${timestamp}\n\n${content}\n`; + expect(after.slice(secondIndex)).toBe(expectedTail); }); }, ); @@ -125,4 +203,21 @@ describe('history-shadow characterization (H1)', () => { expect(await storage.exists(target)).toBe(true); }); }); + + it('write guard resolves a relative target against the hermetic root, not process.cwd()', async () => { + // A relative target must be judged against the hermetic root regardless + // of the test runner's current working directory. A relative path that + // stays inside the root is permitted; a relative path that traverses + // above the root is refused. + await withHermeticRoot(async (root) => { + const storage = makeWriteGuardedStorage(new FSStorageProvider(), root); + + await storage.write('nested/inside.txt', 'ok'); + expect(await storage.exists(path.join(root, 'nested', 'inside.txt'))).toBe(true); + + await expect(storage.write('../escape.txt', 'nope')).rejects.toThrow( + /Hermetic write guard/, + ); + }); + }); }); From ec987bb468d236e30b12762c2206b77b047b6769 Mon Sep 17 00:00:00 2001 From: Copilot Date: Sat, 18 Jul 2026 17:36:29 +0300 Subject: [PATCH 3/3] Fix write guard promise contract for async storage methods The Finding 4 regression test for resolveInsideRoot exposed a bug in makeWriteGuardedStorage: the proxy wrapper for mutating methods was a plain synchronous function, so when resolveInsideRoot threw for an out-of-root target, the exception escaped synchronously instead of becoming a rejected Promise. This crashed wait expect(storage.write(...)).rejects.toThrow(...) because the throw happened while evaluating the argument to expect(), before the rejects matcher could attach. Fix (test-only, hermetic-root.ts): the wrapper now branches on the method name. Sync-suffixed methods (writeSync, deleteSync, etc.) keep throwing synchronously, matching their synchronous contract. All other mutating methods (write, append, delete, deleteDir, mkdir, rename, copy) are now wrapped in an async function so a synchronous throw from resolveInsideRoot is converted into a rejected Promise, matching the underlying StorageProvider interface's Promise-returning contract. No production code touched. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 44f53a4e-d384-4c60-a43d-cfc158160f1f --- test/characterization/_helpers/hermetic-root.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/characterization/_helpers/hermetic-root.ts b/test/characterization/_helpers/hermetic-root.ts index 019645ad9..5fc633a55 100644 --- a/test/characterization/_helpers/hermetic-root.ts +++ b/test/characterization/_helpers/hermetic-root.ts @@ -93,7 +93,7 @@ export function makeWriteGuardedStorage(base: StorageProvider, root: string): St if (!MUTATING_METHODS.has(propertyKey)) { return original.bind(target); } - return (...args: unknown[]): unknown => { + const invoke = (args: unknown[]): unknown => { const first = args[0]; if (typeof first !== 'string') { throw new Error( @@ -116,6 +116,17 @@ export function makeWriteGuardedStorage(base: StorageProvider, root: string): St } return (original as (...a: unknown[]) => unknown).apply(target, resolvedArgs); }; + // The *Sync methods have a synchronous contract, so a containment + // failure must throw synchronously to match that contract. The + // non-Sync methods return a Promise, so a containment failure is + // wrapped in an async function here: without this, resolveInsideRoot's + // synchronous throw would escape as an unhandled synchronous exception + // instead of a rejected Promise, which breaks callers using + // `await expect(storage.write(...)).rejects.toThrow(...)`. + if (propertyKey.endsWith('Sync')) { + return (...args: unknown[]): unknown => invoke(args); + } + return async (...args: unknown[]): Promise => invoke(args); }, }) as StorageProvider; }