mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
* feat(workspace): metadata-driven indexing/hygiene, document versioning, and patch support Foundation for the extensible frontend system. Workspace documents now support metadata flags (skip_indexing, skip_versioning, hygiene config) via folder-level .config documents and per-file overrides, replacing hardcoded hygiene targets and indexing behavior. Key changes: - DocumentMetadata type with resolution chain (doc → folder .config → defaults) - Document versioning: auto-saves previous content on write/append/patch - Workspace patch: search-and-replace editing via memory_write tool - Hygiene rewrite: discovers cleanup targets from .config metadata instead of hardcoded daily/ and conversations/ directories - memory_read gains version/list_versions params - memory_write gains metadata/old_string/new_string/replace_all params - V14 migration adds memory_document_versions table (both PG + libSQL) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address review feedback — transaction safety, patch mode, formatting - Wrap libSQL save_version in a transaction to prevent race condition where concurrent writers could allocate the same version number - Make content optional in memory_write when in patch mode (old_string present) — LLM no longer forced to provide unused content param - Improve metadata update error handling with explicit match arms - Run cargo fmt across all files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address review findings — write-path performance, version pruning, descriptions 1. Resolve metadata once per write: write(), append(), and patch() now call resolve_metadata() once and pass the result to both maybe_save_version() and reindex_document_with_metadata(), cutting redundant DB queries from 3-5 per write down to 1 resolution. 2. Optimize version hash check: replaced get_latest_version_number() + get_version() (2 queries) with list_versions(id, 1) (1 query) for the duplicate-hash check in maybe_save_version(). 3. Wire up version_keep_count: hygiene passes now prune old versions for documents in cleaned directories, enforcing the configured version_keep_count (default: 50). Removes the TODO comment. 4. Fix misleading tool description: patch mode works with any target including 'memory', not just custom paths. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: wire remaining unwired components — changed_by, layer versioning 1. changed_by now populated: all write paths pass self.user_id as the changed_by field in version records instead of None, so version history shows who made each change. 2. Layer write/append versioned: write_to_layer() and append_to_layer() now auto-version and use metadata-optimized reindexing, matching the standard write()/append() paths. 3. append_memory versioned: MEMORY.md appends now auto-version with metadata-driven skip and shared metadata resolution. 4. Remove unused reindex_document wrapper: all callers now use reindex_document_with_metadata directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: comprehensive coverage for versioning, metadata, patch, and hygiene 26 new tests covering critical and high-priority gaps: document.rs (7 unit tests): - is_config_path edge cases (foo.config, empty string, .config/bar) - content_sha256 with empty string (known SHA-256 constant) - content_sha256 with unicode (multi-byte UTF-8) - DocumentMetadata merge: null overlay, nested hygiene replaced wholesale, both empty, non-object base memory.rs (2 schema tests): - memory_write schema includes patch/metadata params, content not required - memory_read schema includes version/list_versions params hygiene.rs (5 integration tests): - No .config docs → no cleanup happens - .config with hygiene disabled → directory skipped - Multiple dirs with different retention (fast=0, slow=9999) - Documents newer than retention not deleted - Version pruning during hygiene (keep_count=2, verify pruned) workspace/mod.rs (14 integration tests): - write creates version with correct hash and changed_by - Identical writes deduplicated (hash check) - Append versions pre-append content - Patch: single replacement, replace_all, not-found error, creates version - Patch with unicode characters - Patch with empty replacement string - resolve_metadata: no config (defaults), inherits from folder .config, document overrides .config, nearest ancestor wins - skip_versioning via .config prevents version creation [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address zmanian review — PG transaction safety, identity protection, perf Must-fix: 1. PostgreSQL save_version now uses a transaction with SELECT FOR UPDATE to prevent concurrent writers from allocating the same version number, matching the libSQL implementation. 2. Restore identity document protection in hygiene cleanup_directory(). MEMORY.md, SOUL.md, IDENTITY.md, etc. are now protected from deletion regardless of which directory they appear in, via is_identity_document() case-insensitive check. This restores the safety net that was removed when migrating from hardcoded to metadata-driven hygiene. Should-fix: 3. resolve_metadata() now uses find_config_documents (single query) + in-memory nearest-ancestor lookup, instead of O(depth) serial DB queries walking up the directory tree. 4. memory_write validates that at least one mode is provided (content for write/append, or old_string+new_string for patch) with a clear error message upfront, instead of relying on downstream empty checks. 5. Fixed misleading GIN index comment in V15 migration. 9. Added "Fail-open: versioning failures must not block writes" comments to all `let _ = self.maybe_save_version(...)` call sites. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: cargo fmt * fix: address Copilot review — DoS prevention, no-op skip, duplicate hygiene Security: - Reject empty old_string in both workspace.patch() and memory_write tool to prevent pathological .matches("") behavior (DoS vector) Correctness: - Remove duplicate hygiene spawn in multi-user heartbeat — was running both via untracked tokio::spawn AND inside the JoinSet, causing double work and immediate skip via global AtomicBool guard - Disallow layer param in patch mode — patch always targets the default workspace scope; combining with layer could silently patch the wrong document - Restore trim-based whitespace rejection for non-patch content validation (was broken when refactoring required fields) Performance: - Short-circuit write() when content is identical to current content, skipping versioning, update, and reindex entirely - Normalize path once at start of resolve_metadata instead of only for config lookup (prevents missed document metadata on unnormalized paths) Cleanup: - Remove duplicate tests/workspace_versioning_integration.rs (same tests already exist in workspace/mod.rs versioning_tests module) [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: eliminate flaky hygiene tests caused by global AtomicBool contention All hygiene tests that used run_if_due() were flaky when running concurrently because they competed for the global RUNNING AtomicBool guard. Rewrote them to test the underlying components directly: - metadata_driven_cleanup_discovers_directories: now uses find_config_documents() + cleanup_directory() directly - multiple_directories_with_different_retention: now uses cleanup_directory() per directory directly - cleanup_respects_cadence: rewritten as a sync unit test that validates state file + timestamp logic without touching the global guard Verified stable across 3 consecutive runs (3793 tests, 0 failures). [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address remaining review comments — metadata ordering, PG locking, hygiene safety 1. Metadata applied BEFORE write/patch (#10-11,15): metadata param is now set via get_or_create + update_metadata before the write/patch call, so skip_indexing/skip_versioning take effect for the same operation instead of only subsequent ones. 2. Layer write doc ID (#13-14): metadata no longer re-reads after write since it's applied upfront. Removes the stale-scope risk. 3. Version param overflow (#16): validates version is 1..i32::MAX before casting, returns InvalidParameters on out-of-range. 4. Hygiene protection list (#18): added HYGIENE_PROTECTED_PATHS that includes MEMORY.md, HEARTBEAT.md, README.md (missing from IDENTITY_PATHS). cleanup_directory now uses is_protected_document() which checks both lists with case-insensitive matching. 5. PG FOR UPDATE on empty table (#22-24): now locks the parent memory_documents row (SELECT 1 FROM memory_documents WHERE id=$1 FOR UPDATE) before computing MAX(version), which works even when no version rows exist yet. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address remaining review comments — metadata merge, retention guard, migration ordering 1. **Metadata merge in memory_write tool**: incoming metadata is now merged with existing document metadata via `DocumentMetadata::merge()` instead of full replacement, so setting `{hygiene: {enabled: true}}` no longer silently drops a previously-set `skip_versioning: true`. 2. **Minimum retention_days**: `HygieneMetadata.retention_days` is now clamped to a minimum of 1 day during deserialization, preventing an LLM from writing `retention_days: 0` and causing mass-deletion on the next hygiene pass. 3. **Migration version ordering**: renumbered document_versions migration to come after staging's already-deployed migrations (PG: V15→V16, libSQL: 15→17). Documented the convention that new migrations must always be numbered after the highest version on staging/main. 4. **Duplicate doc comment**: removed duplicated line on `reindex_document_with_metadata`. 5. **HygieneSettings**: added `version_keep_count` field to persist the setting through the DB-first config resolution chain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: skip metadata pre-apply when layer is specified, clean up stale comment 1. When a layer is specified, skip the metadata pre-apply via get_or_create — it operates on the primary scope and would create a ghost document there while the actual content write targets the layer's scope. 2. Removed stale "See review comments #10-11,15" reference; the surrounding comment already explains the rationale. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use BEGIN IMMEDIATE for libSQL save_version to serialize writers The default DEFERRED transaction only acquires a write lock at the first write statement (INSERT), not at the SELECT. Two concurrent writers could both read the same MAX(version) before either inserts, causing a UNIQUE violation. BEGIN IMMEDIATE acquires the write lock upfront, matching the existing pattern in conversations.rs. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: document trust boundary on metadata/versioning WorkspaceStore methods These methods accept bare document UUIDs without user_id checks at the DB layer. The Workspace struct (the only caller) always obtains UUIDs through user-scoped queries first. Document this trust boundary explicitly on the trait so future implementors/callers know not to pass unverified UUIDs from external input. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address new Copilot review comments — ghost doc, param validation, overflow 1. Skip metadata pre-apply in patch mode to avoid creating a ghost empty document via get_or_create when the document doesn't exist, which would change a "not found" error into "old_string not found". 2. Validate list_versions and version as mutually exclusive in memory_read to avoid ambiguous behavior (list_versions silently won). 3. Clamp version_keep_count to i32::MAX before casting to prevent overflow on extreme config values. 4. Mark daily_retention_days and conversation_retention_days as deprecated in HygieneSettings — retention is now per-folder via .config metadata. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: apply metadata in patch mode via read(), reorder libSQL migrations 1. Metadata is no longer silently ignored in patch mode — uses workspace.read() (which won't create ghost docs) instead of skipping entirely, so skip_versioning/skip_indexing flags take effect for patches on existing documents. 2. Reorder INCREMENTAL_MIGRATIONS to strictly ascending version order (16 before 17) to match iteration order in run_incremental(). [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: remove duplicate is_patch_mode, add TODO comments for known limitations - Remove duplicate `is_patch_mode` binding in memory_write (was computed at line 280 and again at line 368). - Document multi-scope hygiene edge case: workspace.list() includes secondary scopes but workspace.delete() is primary-only, causing silent no-ops for cross-scope entries. - Document O(n) reads in version pruning as acceptable for typical directory sizes. - Add TODO on WorkspaceError::SearchFailed catch-all for future cleanup into more specific variants. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: reindex on no-op writes for metadata changes, use read_primary in patch 1. write() no longer fully short-circuits when content is unchanged — it still resolves metadata and reindexes so that metadata-driven flags (e.g. skip_indexing toggled via memory_write's metadata param) take effect immediately even without a content change. 2. Patch-mode metadata pre-apply now uses workspace.read_primary() instead of workspace.read() to ensure we target the same scope that patch() operates on, preventing cross-scope metadata mutation in multi-scope mode. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
25 lines
980 B
SQL
25 lines
980 B
SQL
-- Document version history for workspace files.
|
|
-- Every content update saves the previous content as a version,
|
|
-- enabling rollback and audit trails.
|
|
|
|
CREATE TABLE memory_document_versions (
|
|
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
|
|
document_id UUID NOT NULL REFERENCES memory_documents(id) ON DELETE CASCADE,
|
|
version INTEGER NOT NULL,
|
|
content TEXT NOT NULL,
|
|
content_hash TEXT NOT NULL,
|
|
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
|
|
changed_by TEXT,
|
|
UNIQUE(document_id, version)
|
|
);
|
|
|
|
CREATE INDEX idx_doc_versions_lookup
|
|
ON memory_document_versions(document_id, version DESC);
|
|
|
|
-- GIN index on metadata for future JSON containment queries
|
|
-- (e.g., WHERE metadata @> '{"hygiene": {"enabled": true}}').
|
|
-- Not used by current queries (which use path LIKE) but enables
|
|
-- efficient metadata-based filtering without a full table scan.
|
|
CREATE INDEX idx_memory_documents_metadata
|
|
ON memory_documents USING GIN (metadata jsonb_path_ops);
|