lywsvip/ironclaw
1
0
Fork 0
mirror of https://github.com/nearai/ironclaw.git synced 2026-09-03 08:06:01 +08:00
Code Issues Packages Projects Releases Wiki Activity
1,292 Commits 1,246 Branches 71 Tags
d33fecb17c3e81404f021ee944aa75673d1b4f97
Commit Graph

3 Commits

Author SHA1 Message Date
Illia Polosukhin
d789a5d270 fix(db): swap V16/V17 to match production PG (document_versions before user_identities) (#1931) 
Production PostgreSQL already has V15=conversation_source_channel and
V16=document_versions applied. user_identities must be V17.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-02 12:45:40 -07:00
Illia Polosukhin
a68358086a fix(db): resolve V15 migration numbering conflict (#1923) 
* fix(db): resolve V15 migration numbering conflict between user_identities and conversation_source_channel

A merge conflict left two PostgreSQL migrations at V15. This renumbers them
(V15=user_identities, V16=conversation_source_channel, V17=document_versions)
to match the libSQL incremental ordering. Adds user_identities, document_versions,
and source_channel to the libSQL base schema so fresh databases get all tables.
Includes a one-time repair for existing databases where V15 was mis-recorded.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: fix rustfmt formatting in repair_misnumbered_v15

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(db): address PR review — proper error handling and tighter repair condition

- Replace .ok().flatten() with explicit error propagation via .map_err()?
  so DB errors during V15 repair are surfaced, not silently swallowed
- Tighten repair condition from `!= "user_identities"` to `== "document_versions"`
  to only fix the specific known-bad case from the merge conflict

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-02 11:06:27 -07:00
Illia Polosukhin
5435b38eca feat(workspace): metadata-driven indexing/hygiene, document versioning, and patch (#1723) 
* 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>
 2026-04-01 23:58:27 -07:00