Testing Strategy

September 14, 2026 · View on GitHub

This document describes the layered test approach to continuously expand coverage without exploding maintenance cost.

Layers

  1. Unit (pure logic)
    • Classification normalization invariants (property-based).
    • Governance hash projection & risk scoring.
  2. Service (in-process)
    • Handlers: add/import/get/list/remove/enrich/groom/governance-update.
    • Error surfacing (loader errors, schema failures).
  3. Protocol (black-box)
    • JSON-RPC lifecycle: initialize -> tool calls -> graceful shutdown.
    • Persistence across restart (round-trip) & disappearance regression.
  4. Concurrency / Stress
    • Rapid duplicate adds / overwrites (deterministic end state, no crashes).
    • Interleaved add + groom (future).
    • Simulated partial write (future when atomicity gaps removed) -> ensure loader error not disappearance.
  5. Property-Based / Fuzz
    • Instruction generation explores category normalization, priority tier derivation, deterministic hashing.
    • Future: corpus of malformed JSON, truncated files, schema edge cases to assert they surface in loadErrors not silent skip.
  6. Governance Drift / Integrity
    • Hash stability tests assert no change when unrelated fields mutate.
    • Drift detection only when governance projection changes.

Expansion Playbook

For every new defect class discovered:

  1. Reproduce minimal failing scenario manually or via script.
  2. Add failing test in the lowest viable layer (unit < service < protocol).
  3. Fix the code.
  4. Add a regression guard at protocol layer if issue originated from cross-layer interaction.

Automation Cadence

  • All tests: npm test (CI).
  • Contract / schema-only quick check: npm run test:contracts.
  • Coverage gate: npm run coverage:ci (threshold lines / branches).
  • Fast feedback (default build): npm run test:fast – excludes high-cost reproduction / RED suites.
  • Pre-push extended regression: npm run test:slow (or automatic via git pre-push hook) – runs multi-client + persistence divergence reproductions.

Fast vs Slow Suite Classification (2025-09-05)

Rationale: Keep build:verify wall-clock low while preserving deep regression signal before code leaves the workstation.

Fast suite (executed in build:verify):

  • All stable/unit/service/protocol tests excluding explicitly listed slow/RED files.

Slow / Pre-Push suite (scripts/test-slow.mjs):

  • feedbackReproduction.multiClient.spec.ts
  • feedbackReproduction.crudConsistency.spec.ts
  • instructionsPersistenceDivergence.red.spec.ts
  • instructionsPersistenceIsolated.red.spec.ts
  • importDuplicateAddVisibility.red.spec.ts

Selection Criteria:

  1. Runtime > ~10s or high I/O amplification (large Index sampling, multi-client coordination).
  2. RED / reproduction tests intentionally exercising known gaps (may fail intermittently while diagnosing).
  3. Adds marginal coverage beyond fast suite invariants (list/get atomicity, persistence divergence) not needed every build.

Governance:

  • New slow candidates must document: estimated runtime, unique invariant covered, why not enforce in fast path.
  • Periodically re-evaluate: if a slow test becomes fast (<5s), migrate back to fast suite.
  • If a RED test turns GREEN (fixed), consider refactoring a minimal deterministic version into fast suite.

Developer Workflow:

  1. During inner-loop: rely on npm run build:verify (fast tests) for quick signal.
  2. Before push: allow pre-push hook to run (or manually npm run test:slow).
  3. CI can optionally add a scheduled job executing both suites + stress (test:stress).

Bypass (emergency only): set BYPASS_PRE_PUSH=1 env var before git push (future enhancement) – not yet implemented; intentional friction maintained.

Fast Lane Semantics (#576, 2026-09-11)

npm run test:fast runs in two stages: stateful HTTP/TLS specs one per process first, then the ~363-spec main batch.

Every stage runs before the verdict. The lane used to process.exit() on the first isolated-spec failure, so the main batch never started and the visible result was "1 failed, 23 passed" with no evidence at all about the other ~3,400 tests. Failures are now collected across stages and reported together; the exit code is decided at the end. A lane that hides the suite it exists to run is worse than a slow one.

Verbosity must not change the verdict. npm run -s test:fast and npm run test:fast must agree. npm run -s exports npm_config_loglevel=silent to child processes, which previously muted the npm notice output that npmPackReadiness.spec.ts parsed, turning 8 assertions red on text that was never printed. That spec now reads npm pack --json as data. Never gate on log output that a verbosity flag can suppress.

Capability gates must probe the capability, not the executable. certInit.spec.ts gated on openssl version exiting 0, then ran openssl req -x509. On a machine where the binary is on PATH but openssl.cnf is absent, version succeeds and req fails — five spurious failures, and a breadcrumb that printed its own contradiction: opensslAvailable=true reason="openssl not detected". The probe now performs an actual certificate generation into a temp dir.

Unhandled harness errors stay red (and this one is Windows-only)

It does not happen in CI. The full Linux run (coverage:ci, 404 files, job 103396359729 and the push run after it) contains 0 occurrences of onTaskUpdate, 0 Unhandled Error blocks and 0 Errors N error lines. On this Windows workstation it reproduces on 4 of 4 runs — serial, parallel, with custom reporters and without. So it is deterministic locally, absent remotely, and therefore a developer-experience defect on Windows, not a CI gate defect.

That asymmetry is why the lane's error handling is left strict rather than loosened: relaxing it would weaken the gate everywhere in order to fix one platform where the gate is not actually failing. The Windows root cause is tracked separately.

A long serial run intermittently ends with:

Error: [vitest-worker]: Timeout calling "onTaskUpdate"

reported as Errors 1 error, which makes the run exit non-zero even when no test failed. The decision is to leave this failing rather than downgrade it to a warning. Vitest reports unhandled errors precisely because they can invalidate the run ("This might cause false positive tests"), and a rule that swallows the whole class in order to silence one known instance would also swallow the unknown ones. Suppressing a harness error is indistinguishable from suppressing a real one.

Run length is not the cause — that hypothesis was tested and falsified. The obvious suspect was --fileParallelism=false on the main batch, which serialised 363 specs into a ~560 s run. Removing it (safe, because the stateful HTTP/TLS specs already run in their own processes) cut the batch to 193 s — 2.9× faster, byte-identical results:

serialparallel
duration559.8 s193.0 s
files360 passed, 3 skipped360 passed, 3 skipped
tests3501 passed, 19 skipped3501 passed, 19 skipped
Errors11

The error survived a 2.9× reduction in wall-clock, so it is not a duration-driven timeout. The parallelism change is kept on its own merits (speed), but it must not be described as the fix.

The custom-reporter hypothesis was also tested and falsified. onTaskUpdate is a reporter callback, and this project registers two custom reporters — runSentinelReporter.ts and jsonResultsReporter.ts (vitest.config.ts:7) — so a reporter blocking on task updates was the obvious next suspect. Running the same batch with --reporter=default (both custom reporters out of the path) reproduces the error identically: 192.8 s, 360 passed | 3 skipped, 3501 passed | 19 skipped, Errors 1 error.

So: not duration, not the custom reporters. Two measurements, two eliminations. Whatever holds the worker RPC open is elsewhere — most likely a single spec leaving a handle or timer live past teardown, which would be found by bisecting the 363-file batch rather than by reasoning about it. Recorded here so the next person starts from two closed doors instead of re-opening them.

Environment-Gated RED Reproductions (Since 1.3.1)

Some RED tests are high-friction (intermittent timeouts, deep diagnostics) and can block routine pushes while an upstream anomaly is under investigation. To preserve signal without halting velocity, we gate specific reproductions behind explicit environment variables. Current gating:

Test FileEnv VarDefault BehaviorRationale
importDuplicateAddVisibility.red.spec.tsINDEX_SERVER_RUN_RED_IMPORT_DUP_ADDSkipped unless set truthy (1/true/yes/on)Intermittent visibility timing anomaly; heavy diagnostics & 25s timeout
instructionsAddSkipVisibility.spec.tsINDEX_SERVER_RUN_SKIP_VISIBILITY_RELIABILITYSkipped unless set truthy (1/true/yes/on)Occasional 35s timeout in production-dir handshake path; under active investigation

Guidelines:

  1. Gating is temporary; remove once anomaly either (a) fixed and converted to deterministic GREEN test, or (b) re-characterized as obsolete.
  2. Gated tests MUST clearly document activation variable at file top.
  3. Do NOT gate a RED test preemptively—only after repeated blocking incidents (>=2 aborted pushes) and documented in commit message.
  4. CI full runs (scheduled or manual deep diagnostics) should set the env var to avoid silent regression masking.

Activation Example:

$env:INDEX_SERVER_RUN_RED_IMPORT_DUP_ADD='1'; npm run test:slow -- src/tests/importDuplicateAddVisibility.red.spec.ts

This strategy keeps RED artifacts present (preventing knowledge drift) while eliminating routine friction for unrelated doc or governance changes.

Property-Based Guidance

  • Keep each property < 100 runs for CI speed; add nightly job (future) with >1000 runs.
  • Shrink failure outputs to produce minimal counter-example (default fast-check behavior).
  • Prefer focused arbitraries: supply only valid characters to avoid noisy schema rejections.

Authentication & Authorization Tests

  • src/tests/integration/dashboardAuth.spec.ts — 33 vitest integration tests covering Bearer token validation, localhost bypass, 401/403 responses across all protected mutation routes
  • tests/playwright/dashboard-auth.spec.ts — 10 Playwright E2E tests covering login modal flow, session persistence, logout, invalid key handling, keyboard shortcuts

Search Field/Enum Coverage Matrix (#348)

Coverage of index_search fields filter operators across the instruction schema is split between fast in-process unit tests (authoritative) and live MCP probe steps (smoke/regression against the running server). The unit-test layer is the source of truth for value-matching semantics. Probe steps that cannot exercise an operator end-to-end — because the underlying field is server-managed, server-overwritten on write, or silently dropped on the live write path — are marked [advisory:query-shape] and assert only that the live handler still accepts the query shape (a schema/handler regression that rejects the operator will fail these steps; mis-matching values will not).

Schema field / operatorUnit test (src/tests/search.spec.ts)Live probe (scripts/dev/integrity/search-probe.mjs)
audience (enum scalar)FL-35S8.1
audience (array OR)FL-36S8.1b
audience (invalid-enum rejection)FL-39S8.11
requirement (enum scalar)FL-37S8.2
requirement (array OR)FL-38S8.2b
contentType (enum scalar)FL-40covered indirectly via S2.*
status (enum scalar)FL-41S8.3
priorityTier (enum OR set)FL-42S8.4
classification (enum scalar)FL-43S8.12
teamIdsAny/All/NoneFL-44, FL-45, FL-46S8.5–S8.7 [advisory:query-shape] (see Known Limitations)
usageCountMin/Max (numeric range)FL-47S8.8 [advisory:query-shape] (server-managed)
usageCount (inverted-range rejection)FL-59-
riskScoreMin/Max (numeric range)FL-48S8.9 [advisory:query-shape] (server-computed)
reviewIntervalDaysMin/MaxFL-49(write-side bounds in validation-probe Phase 9)
createdAfter/Before (date)FL-50-
firstSeenAfter/Before (date)FL-51-
lastUsedAfter/Before (date)FL-52S8.10 [advisory:query-shape] (server-managed)
lastUsed (inverted-range rejection)FL-60-
lastReviewedAfter/Before (date)FL-53-
nextReviewDueAfter/Before (date)FL-54-
archivedAfter/Before (date)FL-55-
workspaceId (scalar)FL-56-
version (scalar)FL-57-
supersedes (scalar)FL-58-
additionalProperties:false guard(SoT drift guard, see below)-
minProperties:1 guard(SoT drift guard, see below)-

A separate SoT drift guard at src/tests/instructionSearchFieldsSchema.spec.ts asserts that the schema advertised by buildInstructionSearchFieldsSchema() continues to expose the documented set of record-property keys, virtual operators, additionalProperties:false, and minProperties:1. The additionalProperties:false and minProperties:1 constraints are enforced by that drift guard rather than by individual FL-* tests in search.spec.ts.

Probe Fixture Isolation

Probe steps that assert seeded-ID membership against live index_search results (S8.1, S8.1b, S8.2, S8.2b, S8.3, S8.4, S8.12) combine their field predicate with fields.idPrefix: \sp-${RUN_TAG}-`. RUN_TAGis unique per probe invocation, so the candidate set is restricted to the three fixtures seeded by the current run. Unrelated sandbox records cannot sort ahead of the fixtures or be excluded bylimit` slicing, keeping membership assertions deterministic regardless of sandbox population, ordering, or concurrent state.

Known Limitations (probe-only)

  • usageCount / lastUsedAt — server-managed; cannot be seeded via index_add (rejected as unexpected properties). Value-matching for the corresponding filters is covered by FL-47 / FL-52 (range) and FL-59 / FL-60 (inverted-range rejection). Probe steps S8.8 and S8.10 assert query-shape acceptance only.
  • riskScore — author-supplied values are silently overwritten by ClassificationService.normalize() via computeRisk(priority, requirement). Probe step S8.9 asserts only that the query shape is accepted; FL-48 covers value matching against in-memory state.
  • teamIds — observed to drop from the persisted entry on the live write path even though the input schema allows it. Probe steps S8.5–S8.7 assert query-shape acceptance only; FL-44..FL-46 cover value matching against in-memory state. Tracked as a follow-up issue.
  • reviewIntervalDays bounds — schema declares min:1, max:365 but the live index_add path currently accepts 0 and 366. Validation-probe Phase 9 records this as a [gap-probe] advisory. When the server unexpectedly accepts the out-of-bounds values, the probe registers the persisted IDs for Phase 12 cleanup so no schema-invalid fixtures leak between runs. Tracked as a follow-up issue.

Pending Enhancements

  • Add loadErrors tool and tests: assert no silent skips.
  • Atomic write enforcement test: inject crash between write & fsync (requires harness).
  • Path pinning test after directory resolution refactor.
  • Fuzz loader with truncated JSON files, expect surfaced errors.

Principles

  • Each new bug gets a test before a fix.
  • Never broaden a property until it finds at least one real issue historically (evidence-driven expansion).
  • Keep protocol tests deterministic (avoid racey timing; prefer polling wait utilities).
  • Fast feedback: majority of suite under 10s local.

Mandatory CRUD Feedback Red/Green Workflow (Mirrored Policy)

This section mirrors the authoritative policy in feedback_defect_lifecycle.md and is included here so test authors have zero ambiguity when confronted with CRUD / persistence anomalies (phantom writes, visibility gaps, inconsistent list vs get, multi-client divergence).

High-level enforcement (concise):

  1. RED test first (*.red.spec.ts) using ONLY reporter-provided IDs.
  2. No handler/service code changes until RED test committed & failing in CI.
  3. RED test MUST assert: add contract, list inclusion, per-ID get visibility, index hash (or synthetic surrogate) mutation.
  4. Capture evidence (counts, missing IDs) in an analysis doc before any fix.
  5. Apply minimal atomic persistence fix (write → verify → index update) then convert RED to GREEN (rename/duplicate).
  6. Add negative + multi-client visibility regression tests post-fix.
  7. CI guard: CRUD issue closure requires RED→GREEN pair reference.

Data Fidelity Enforcement (ALWAYS Use Provided Data)

ALL reproduction tests MUST embed or import the exact reporter-supplied payload (every field & original whitespace). No trimming, reformatting, synthesized IDs, or markdown reflow unless an inline DATA-FIDELITY-WAIVER comment references explicit approval & rationale. Reviewers reject any PR that paraphrases payload content. Future automation will lint for divergence against archived feedback JSON.

Refer to the lifecycle doc for the detailed invariant table and justification. Any deviation requires explicit documented waiver in both docs.