test(memory): characterization baselines for #1309 (research, not closing)#1499
Conversation
…sing) 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
🟡 Impact Analysis — PR #1499Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affectedroot (3 files)
tests (5 files)
This report is generated automatically for every PR. See #733 for details. |
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 3 commits — consider squashing before review |
| ✅ | Not in draft | Ready for review |
| ❌ | Branch up to date | dev is 1 commit(s) ahead — rebase recommended |
| ❌ | Copilot review | No Copilot review yet — it may still be processing |
| ✅ | Changeset present | No source files changed — changeset not required |
| ✅ | Scope clean | No .squad/ or docs/proposals/ files |
| ✅ | No merge conflicts | No merge conflicts |
| ✅ | Copilot threads resolved | No Copilot review threads |
| ❌ | CI passing | 1 check(s) still running |
Files Changed (8 files, +990 −0)
| File | +/− |
|---|---|
test-fixtures/characterization/.gitattributes |
+4 −0 |
test-fixtures/characterization/README.md |
+33 −0 |
test-fixtures/characterization/spawn-context-baseline.json |
+123 −0 |
test/characterization/_helpers/hermetic-root.ts |
+132 −0 |
test/characterization/classification-forbidden.characterization.test.ts |
+279 −0 |
test/characterization/history-shadow.characterization.test.ts |
+223 −0 |
test/characterization/local-memory-store.characterization.test.ts |
+152 −0 |
test/characterization/spawn-context-baseline.characterization.test.ts |
+44 −0 |
Total: +990 −0
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
… 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
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
There was a problem hiding this comment.
Pull request overview
Adds a new set of characterization (baseline) tests and fixtures for the current memory subsystem behavior (spawn-context benchmark report, LocalMemoryStore invariants, history-shadow append semantics, and classification/forbidden governance), to make future tiered-memory work (#1309) measurable as diffs against pinned “known current behavior”.
Changes:
- Introduces a hermetic temp-root helper plus a write-guarded
StorageProviderproxy for characterization suites. - Adds characterization test suites covering history-shadow append behavior, LocalMemoryStore concurrent writes + delete ordering, forbidden/classification heuristics, and a golden-file baseline for
runMemoryValueBenchmark(). - Adds committed golden JSON + fixture README +
.gitattributesto stabilize byte-exact baselines across platforms.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/characterization/_helpers/hermetic-root.ts | Adds hermetic temp-root runner and write-guarded storage proxy for characterization tests. |
| test/characterization/history-shadow.characterization.test.ts | Pins current history-shadow append/merge behaviors (including known bug behavior) in hermetic tests. |
| test/characterization/local-memory-store.characterization.test.ts | Characterizes LocalMemoryStore concurrency and delete ordering invariants. |
| test/characterization/classification-forbidden.characterization.test.ts | Characterizes forbidden-pattern rejection + classification heuristics with pinned expectations. |
| test/characterization/spawn-context-baseline.characterization.test.ts | Asserts byte-exact runMemoryValueBenchmark() JSON output against a committed golden. |
| test-fixtures/characterization/spawn-context-baseline.json | Golden JSON fixture for spawn-context benchmark baseline. |
| test-fixtures/characterization/README.md | Documents provenance and rules for characterization fixtures. |
| test-fixtures/characterization/.gitattributes | Forces LF line endings for fixtures to keep byte-exact comparisons stable. |
| 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'); | ||
| }); | ||
| }, | ||
| ); |
| * 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. |
Stage 1 characterization spike for #1309 (research only)
Refs #1309. This PR does NOT close #1309. It captures pinned baselines for
the current memory subsystem so any future tiered-memory work can be evaluated
as a diff against known behavior.
Safety posture (must read before reviewing)
(
vite@8.1.4PolicyOverride,postcss@8.5.18404 onpackagefeedproxy.microsoft.io;--registry https://registry.npmjs.orgalso failed with
Exit handler never called). CI is the first real testsignal for this PR. First green run should be treated as the acceptance
baseline, not this description.
mkdtemptemp directories and use a write-guardedStorageProvider Proxy to prove zero mutation of repository
.squadstate.no workflow, no script, no template, and no
.squadfile is modified inthis PR. Diff boundary is enforced below.
Diff boundary (enforced)
Only these paths are changed:
test/characterization/_helpers/hermetic-root.tstest/characterization/history-shadow.characterization.test.tstest/characterization/local-memory-store.characterization.test.tstest/characterization/classification-forbidden.characterization.test.tstest/characterization/spawn-context-baseline.characterization.test.tstest-fixtures/characterization/spawn-context-baseline.jsontest-fixtures/characterization/README.mdtest-fixtures/characterization/.gitattributesWhat is characterized
unknown-section append fallback, timestamp regex.
distinct ids and JSONL invariants.
storage.delete(sourceAbs)runs, observed via Proxy interception.Rejected as forbidden memory: <suffix>and safe audit titleRejected governed memory, with 12 near-miss negatives.COPILOT_MEMORY/LOCAL plus ambiguous rows asserting current heuristic
behavior (these are the rows most likely to need a CI-first assertion
adjustment; any fix stays inside
test/characterization/**).runMemoryValueBenchmark()output vs the committed golden JSON..gitattributeslocks the fixture to LF so Windows CI does not silentlybreak byte identity. T5 runs; it does not skip.
Changelog gate
No files under
packages/*/src/**change, so a changeset is not required.If
changelog-gatefires anyway, apply the existingskip-changeloglabel; do not add new labels from this PR.
Explicitly out of scope for this PR
no spawn-template change, no feature flag, no wiki writer.
.squadmemory or history at test time.