Feat/memory v1 deprecation audit - #78
Merged
Merged
Conversation
Deprecate json-v1 and shadow-v2 in favor of sqlite-v2-opt-in with explicit no-removal-date guidance, and make /memory audit-migration read-only and inspection-first so absent, invalid, and unreadable V1 records are reported distinctly before any V2 acquisition. Reconcile V1 evidence against descriptor-bound versioned hashes with containment checks and fail-closed, token-owned cross-process locking so pruning never acts on unverifiable or inherited stale verdicts. Validate V1->V2 migration deterministically by reconstructing every source-derived task and observation event and comparing full normalized headers and payloads, closing the first two Gate 0 removal-readiness blockers documented in docs/memory-v1-removal-readiness.md.
…ip validation The V1->V2 import now runs before the query-id check and fails closed against a stateless stub, so update the stale coordinator expectation to the deterministic 'migration-failed' reason while keeping the no-context/no-parity safety invariant. Split validatedMigrationOwnedMarkerMetadata's inline check chain into focused boolean predicate helpers; semantics are unchanged.
The Bun SQLite backend persists event payloads via stableJson (recursive key sort) and export() re-parses from that sorted JSON, so record-valued marker fields like sourceItemCounts round-trip in sorted key order while the in-memory draft keeps insertion order. equalEventDraft's naive JSON.stringify comparison therefore always reported imported-body-mismatch on the real provider. Canonicalize both sides (recursive key sort, arrays keep order) so comparison is by content, letting import return 'imported' and audit report 'exact'. Adds a regression test mimicking the stableJson round-trip, hoists the loop-invariant getV1MigrationIdentity out of findMigrationMarker's reservation loop, and documents hashToken truncation collision tolerance.
Seed a V1 task-memory record, import it into V2 through the real Bun SQLite backend against a temp project, and assert the migration audit certifies an exact result with matching sourceItemCounts. This exercises the canonicalizeForCompare key-order round-trip end to end: the backend persists event payloads via key-sorted stableJson, so the re-read marker arrives with sorted record keys while the in-memory draft keeps insertion order.
The sdk-side test reached into cli internals via a cross-package relative path to import the Bun SQLite backend, which a reviewer advisory flagged as fragile. Colocating the test with the existing cli backend test makes the backend import package-local; the migration and V1-store modules are still exercised from sdk source.
Race-free WAL/SHM open is provably impossible in pure JS with bun:sqlite, so add an opt-in requireSecureOpen gate that fails closed with a typed non-retryable unsupported-open error and performs zero SQLite mutation, plus an honest openPosture visibility field on the default open path. Bound projection replay at MAX_REPLAY_EVENTS = 10000 so rebuildProjections stops at the cap, parks the cursor at the last replayed sequence, and reports truncated: true instead of falsely claiming the canonical tail; v1-to-v2 migrate passes Number.MAX_SAFE_INTEGER so migration replay stays complete. Also sync STATUS.md plan tracking for both changes.
A pending gate file deleted before any snapshot captured its bytes re-triggered specialist review forever: readGateFileContentMarker returned an unreadable:* marker for a nonexistent path, but deleted-file handling only recognized the exact missing marker, so the deletion was never classified, per-file credit failed closed, and the specialist's assigned-file-unreadable blocker was never cleared. Now an lstatSync probe maps true nonexistence (ENOENT) to the missing marker while dangling symlinks stay present and other failures stay unreadable (fail closed), and a turn-start prune drops open reviewer findings whose entire file set is missing — along with their verbatim blockers — before the owed-set rehydration can re-arm the reviewer family.
…ive the missing-file prune The committed gate fix resolves open reviewer findings whose files no longer exist on disk to a missing marker and prunes them. Two e2e fixture tests seeded openReviewerFindings with a virtual path (cli/src/auth/token-store.ts) that never existed, so the turn-start prune dropped the finding and the owed specialist never rehydrated, failing the pre-push hook. Create the fixture file before handleSteps runs and remove only what the fixture created in afterEach (rmdirSync refuses non-empty dirs, so real repo content is never deleted).
Share the 'missing' content marker via a single in-handleSteps GATE_FILE_MISSING_CONTENT_MARKER constant so the deletion-semantics producer and consumers cannot drift, and when the turn-start deleted-file prune empties openReviewerFindings also remove a pruned-and-unbacked family from owedReviewerRevalidations and re-mirror requiredReviewerRevalidation from the filtered owed[0]. A family still backed by a remaining open finding stays owed (fail closed). Adds regression coverage including a partial-prune case and syncs STATUS.md.
…duction behavior The pre-push parity guard failed because the test-local gateFileMarker mirror and the production-oracle loader had drifted from readGateFileContentMarker, which returns the 'missing' content marker for nonexistent paths. Add the early lstatSync existence probe to the mirror so it attests missing leaf/intermediate paths like production, and hoist the GATE_FILE_MISSING_CONTENT_MARKER declaration into the synthetic new Function scope in loadProductionGateFileContentMarker so the extracted oracle no longer throws a ReferenceError on the ENOENT probe. Validated: parity suite 19/19, full agents suite 1184 pass / 0 fail, agents typecheck clean.
…ng in parity guard Reviewer-advisory hardening for the reviewer-spawn-conditions parity e2e: (1) loadProductionGateFileContentMarker now locates the GATE_FILE_MISSING_CONTENT_MARKER declaration end by scanning for the equals sign, the initializer's opening quote, its matching closing quote, then the terminating semicolon, so a semicolon inside the string literal can no longer mis-slice the hoisted const. (2) The external, internal, and mid-path POSIX symlink catch blocks now console.warn which sub-case was skipped (with the error code) instead of silently returning, making sandbox EPERM skips visible. Also appends the R7-T2 package-wide validation record to the dynamic-cross-session-memory STATUS doc. Validated: parity suite 19/19, full agents suite 1184 pass / 0 fail, agents typecheck clean.
Wrap the OpenTUI testRender mount and renderOnce frame in a dev/prod-safe act() shim and set IS_REACT_ACT_ENVIRONMENT so reconciler updates are captured deterministically, eliminating intermittent not-wrapped-in-act failures. Assert the hoisted GATE_FILE_MISSING_CONTENT_MARKER slice keeps declaration shape before eval so a base2.ts layout reorder throws a clear error instead of a confusing ReferenceError or silent sentinel mismatch.
… R7 durable artifacts Mark MEM2-R7-T3 BLOCKED (not done) because a committed clean tree mints no gate-assigned v3 fingerprint, so reviewer-family specialists cannot attest the same frozen bundle; record the repro and the proposed committed-surface fix. Refresh PLAN/STATUS headers and current-task pointers, and capture the R7-T4 evidence-substituted disposition plus the specialist-token/clean-tree lesson.
Reviewer-family specialist schemas hard-required params.snapshot_id, but the gate-minted v3 token is only obtainable by runtime-owned programmatic spawns, so agents following caller-facing guidance wrongly spawned reviewer specialists and hit an unfixable validation wall. Make snapshot_id optional with the v3 pattern enforced only when supplied, state the two-mode (runtime-owned vs manual omit) contract consistently across the routing guide, prompt sections, tool description, regenerated type sources, and docs, and branch the recovery hints on whether the caller supplied the key (security-reviewer stays the documented exception). Also emit concise autoRecovering userMessage events on the spawn_agents partial-failure and spawn_agent_inline pre-validation paths so the CLI shows a one-liner instead of the raw multi-KB validation wall; the full contract still reaches the agent's message history.
…edge formatStructuredReviewReceipt now renders an empty snapshotFingerprint echo as (manual/unattested) so a manual-omit-contract review is never mistaken for a gate-attested fingerprint, with a new pinning test asserting gate attestation fails closed on the empty echo (missing-fingerprint issue, no drift credit, base2 inline-mirror parity). Tail knowledge-refresh entries in cli/knowledge.md and common/knowledge.md document the optional snapshot_id/security-reviewer exception, the calm userMessage/autoRecovering spawn error UX, and the unattested echo, clearing the guard:memory-drift staleness check.
The stale required-key and spawnerPrompt assertions expected snapshot_id as a required param; update them to the new optional schema where manual spawns omit params.snapshot_id entirely and the v3 pattern applies only when supplied.
The comment above the specialistRoutingSection snapshot param contract test still described the old require-snapshot_id contract; it now states the omit-for-manual two-mode contract (runtime-owned spawns pass the gate-owned v3 token, manual spawns omit the key entirely; security-reviewer requires changed_files + snapshot_fingerprint). Comment-only change, no assertions modified.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change is