feat(examples): escape CrewAI 1.15 unified memory to OKF (#1609) - #1821

examples/migrations/crewai_unified_memory_okf/record_demo.py (3)

58-70: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use Popen as a context manager to close the stdout pipe.

The current code never closes process.stdout explicitly. The pipe file descriptor is released only when the object is garbage collected, which raises ResourceWarning under -W error and in test runs. A with block closes the pipe and calls wait() deterministically.

Note on the static analysis hints for this range: the command vector is a fixed list of sys.executable, a repository-relative script path, and a tempfile.TemporaryDirectory path, and shell=True is absent. The CWE-78 findings are false positives here.

♻️ Proposed context-manager form

-        process = subprocess.Popen(
-            command,
-            cwd=script_dir,
-            env=environment,
-            stdout=subprocess.PIPE,
-            stderr=subprocess.STDOUT,
-            text=True,
-            encoding="utf-8",
-            errors="replace",
-        )
-        assert process.stdout is not None
-        captured = [line.rstrip() for line in process.stdout]
-        return_code = process.wait()
+        with subprocess.Popen(
+            command,
+            cwd=script_dir,
+            env=environment,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.STDOUT,
+            text=True,
+            encoding="utf-8",
+            errors="replace",
+        ) as process:
+            assert process.stdout is not None
+            captured = [line.rstrip() for line in process.stdout]
+            return_code = process.wait()

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/record_demo.py` around lines 58
- 70, Update the subprocess execution around Popen in the record-demo flow to
use the Popen instance as a context manager, keeping stdout capture and
return-code handling inside the with block so the pipe closes and the process is
waited deterministically.

---

82-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Most DECORATION members are unreachable.

Lines 82-84 replace every character outside the ASCII printable range with a space. The box-drawing characters in DECORATION (│─┌┐└┘├┤┬┴┼╭╮╰╯═║╔╗╚╝) are non-ASCII, so they are already spaces when line 86 runs. Only " +-" can ever match. Either drop the unreachable members, or move the decoration filter before the ASCII normalization.

♻️ Proposed reorder so the decoration filter applies

     for raw in lines:
         clean = ANSI_RE.sub("", raw)
+        if not clean.strip() or not (set(clean.strip()) - DECORATION):
+            continue
         clean = "".join(
             character if 32 <= ord(character) < 127 else " " for character in clean
         )
         clean = clean.strip()
-        if not clean or not (set(clean) - DECORATION):
+        if not clean:
             continue

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/record_demo.py` around lines 82
- 86, Update the cleaning logic around the clean normalization and DECORATION
check so box-drawing decoration characters are evaluated before non-ASCII
characters are replaced with spaces. Preserve the existing empty/decoration-only
check while ensuring every DECORATION member can be recognized, rather than
removing the defined decoration set.

---

26-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Font lookup is Windows-only.

_font searches only %WINDIR%\Fonts. On Linux and macOS every lookup falls through to ImageFont.load_default(). That returns a small bitmap font, so the requested sizes 18, 24, and 44 are ignored and the frame layout (fixed y-offsets, 25-line terminal window) no longer matches the rendered text. Pillow>=10,<13 in requirements-video.txt also allows versions before 10.1, where load_default does not accept a size argument at all.

Add a platform-independent candidate list, and prefer a scalable fallback.

♻️ Proposed cross-platform font lookup

-def _font(name: str, size: int) -> ImageFont.FreeTypeFont | ImageFont.ImageFont:
-    windows_fonts = Path(os.environ.get("WINDIR", r"C:\Windows")) / "Fonts"
-    for candidate in (windows_fonts / name, windows_fonts / "consola.ttf"):
-        if candidate.exists():
-            return ImageFont.truetype(str(candidate), size=size)
-    return ImageFont.load_default()
+FONT_DIRECTORIES = (
+    Path(os.environ.get("WINDIR", r"C:\Windows")) / "Fonts",
+    Path("/usr/share/fonts"),
+    Path("/usr/local/share/fonts"),
+    Path.home() / ".fonts",
+    Path("/System/Library/Fonts"),
+    Path("/Library/Fonts"),
+)
+FALLBACK_FONTS = (
+    "consola.ttf",
+    "DejaVuSansMono.ttf",
+    "DejaVuSans.ttf",
+    "Menlo.ttc",
+)
+
+
+def _font(name: str, size: int) -> ImageFont.FreeTypeFont | ImageFont.ImageFont:
+    for candidate_name in (name, *FALLBACK_FONTS):
+        for directory in FONT_DIRECTORIES:
+            if not directory.is_dir():
+                continue
+            for candidate in directory.rglob(candidate_name):
+                return ImageFont.truetype(str(candidate), size=size)
+    return ImageFont.load_default()

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/record_demo.py` around lines 26
- 37, Update the _font helper to search platform-independent font candidates,
including common Linux and macOS locations, while retaining the Windows lookup.
Ensure candidates are checked in order and prefer an available scalable
TrueType/OpenType font before falling back to ImageFont.load_default(). Keep the
existing requested sizes for MONO, MONO_BOLD, TITLE, and SUBTITLE, and avoid
passing size to load_default for compatibility with older Pillow versions.

examples/migrations/crewai_unified_memory_okf/requirements.txt (1)

5-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add upper bounds to keep the example reproducible.

crewai, lancedb, PyYAML, pytest, and ruff all carry upper bounds. pytest-asyncio and pytest-timeout do not. pytest-asyncio has changed its default configuration across major versions. Cap both to match the rest of the file.

♻️ Proposed pin bounds

-pytest-asyncio>=0.21
-pytest-timeout>=2.1
+pytest-asyncio>=0.21,<1.3
+pytest-timeout>=2.1,<3

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/requirements.txt` around lines
5 - 6, Update the pytest-asyncio and pytest-timeout dependency entries in
requirements.txt to include upper-version bounds, matching the existing
reproducibility constraints and the file’s dependency pinning style.

examples/migrations/crewai_unified_memory_okf/migrate.py (3)

289-291: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Strip the trailing separator after truncation, as _slug does.

_slug calls .rstrip("-") after slicing at line 277. _tag slices without stripping. A value longer than 64 characters can therefore produce a tag that ends with -. Match the two helpers.

♻️ Proposed change

 def _tag(value: str) -> str:
     normalized = re.sub(r"[^a-z0-9._-]+", "-", value.lower()).strip("-")
-    return normalized[:64]
+    return normalized[:64].rstrip("-")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/migrate.py` around lines 289 -
291, Update the `_tag` helper to strip trailing hyphens after truncating the
normalized value to 64 characters, matching `_slug`’s post-slice behavior while
preserving the existing normalization and length limit.

---

577-577: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add help text to --max-records.

Every other argument in build_parser carries a help string. --max-records does not, so --help shows the flag without a description. The README documents the 50,000 cap on lines 76-78. State the valid range, because read_lancedb_records rejects a value outside 1 to 50,000.

♻️ Proposed change

-    parser.add_argument("--max-records", type=int, default=50_000)
+    parser.add_argument(
+        "--max-records",
+        type=int,
+        default=50_000,
+        help="Maximum records to read, between 1 and 50,000",
+    )

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/migrate.py` at line 577, Update
the --max-records argument in build_parser to include help text describing its
purpose and valid range of 1 to 50,000, while preserving the existing default of
50,000 and type=int.

---

390-399: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicated safe-target guard in two modules. _safe_output_target and _safe_generated_target apply the same rule: resolve the path, reject the filesystem anchor, the home directory, the current directory, and any path with fewer than three parts. The two copies differ only in the error message. Both guard a shutil.rmtree call, so the copies must not drift.

• examples/migrations/crewai_unified_memory_okf/migrate.py#L390-L399: rename _safe_output_target to a shared safe_destructive_target(path, *, purpose: str) helper that takes the noun for the error message, and keep it in this module.
• examples/migrations/crewai_unified_memory_okf/generate_source.py#L227-L236: delete _safe_generated_target and import the shared helper from migrate. generate_source.py and migrate.py already sit in the same directory and are already imported together by run_demo.py lines 12-13.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/migrate.py` around lines 390 -
399, The duplicated destructive-target validation must be centralized. In
examples/migrations/crewai_unified_memory_okf/migrate.py:390-399, rename
_safe_output_target to safe_destructive_target(path, *, purpose: str), preserve
the existing safety checks, and use purpose in the error message. In
examples/migrations/crewai_unified_memory_okf/generate_source.py:227-236, remove
_safe_generated_target and import and call migrate.safe_destructive_target with
the appropriate purpose.

examples/migrations/crewai_unified_memory_okf/README.md (1)

153-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Clarify the redaction sentence.

On line 155 the adverb placement in "A redacted record correctly will not pass an exact-content hash check" makes "correctly" attach to the redaction rather than to the outcome. State the expected behavior directly. Line 153 also reads more simply with "for" in place of "with respect to".

✏️ Proposed wording

-- Migration is local and read-only with respect to the CrewAI database.
+- Migration is local and read-only for the CrewAI database.
 - Secret redaction is opt-in because silent mutation would violate fidelity.
-- A redacted record correctly will not pass an exact-content hash check; the
-  manifest reports its redaction count.
+- A redacted record does not pass the exact-content hash check. This is the
+  expected result, and the manifest reports its redaction count.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/README.md` around lines 153 -
156, Update the migration guarantees in the README: replace “with respect to”
with “for” in the CrewAI database sentence, and rewrite the redaction sentence
so it directly states that a redacted record will not pass an exact-content hash
check, while preserving the manifest redaction-count detail.

Source: Linters/SAST tools

examples/migrations/crewai_unified_memory_okf/validate.py (1)

139-141: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Compare confidence with a tolerance.

Line 140 tests two floats for exact equality. The value travels from the source record through a YAML dump, a YAML load, and map_okf before it reaches this comparison. The round trip is exact today, so the check passes. It becomes brittle if the mapper ever rounds or re-derives the value, and the failure would appear as an unexplained mapping error. math is already imported on line 8.

♻️ Proposed change

                 "confidence_preserved": bool(
-                    row and float(row.get("confidence", -1)) == reconstructed.importance
+                    row
+                    and math.isclose(
+                        float(row.get("confidence", -1)),
+                        reconstructed.importance,
+                        rel_tol=1e-9,
+                        abs_tol=1e-9,
+                    )
                 ),

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/validate.py` around lines 139 -
141, Update the confidence comparison in the “confidence_preserved” check to use
a small floating-point tolerance via the already imported math module instead of
exact equality, while preserving the existing missing-row and default-confidence
behavior.

examples/migrations/crewai_unified_memory_okf/tests/test_migration.py (3)

73-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the type-precedence order and the mapping basis.

The parametrized cases each exercise one source only. infer_memanto_type returns a (type, basis) pair and applies the precedence metadata.memory_type > categories > scope components, which MAPPING.md documents as the contract. No test proves that order, and no test asserts the returned basis. Add a case where metadata.memory_type conflicts with a category.

💚 Proposed additional test

+def test_metadata_type_wins_over_category_and_scope():
+    record = CrewAIRecord(
+        id="one",
+        content="Content",
+        scope="/goal",
+        categories=("incident",),
+        metadata={"memory_type": "decision"},
+        importance=0.5,
+        created_at="2026-08-05T00:00:00Z",
+        last_accessed="2026-08-05T00:00:00Z",
+        source="agent",
+        private=False,
+    )
+    assert infer_memanto_type(record) == ("decision", "metadata.memory_type")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/tests/test_migration.py` around
lines 73 - 95, Extend test_type_mapping_is_explicit_and_deterministic to cover
the documented precedence of metadata.memory_type over categories and scope
components, using a conflicting metadata/category case and asserting both the
inferred type and basis returned by infer_memanto_type. Add coverage for
category-over-scope precedence as needed to verify the full contract described
in MAPPING.md.

---

161-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the remaining fail-closed branches of row_to_record.

Only the missing-column branch is tested. row_to_record also rejects an empty id or content, an importance outside [0, 1], malformed JSON in categories_str or metadata_str, and a non-ISO created_at. Each is a separate guard in migrate.py. Add a parametrized negative case per branch.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/tests/test_migration.py` around
lines 161 - 163, Add parametrized negative cases to
test_schema_validation_fails_closed covering row_to_record’s empty id/content,
out-of-range importance, malformed categories_str/metadata_str JSON, and non-ISO
created_at guards in migrate.py. Assert each case raises ValueError, while
retaining the existing missing-column case.

---

124-139: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Sensitive Data Exposure (CWE-359)

Reachability: Internal

Assert that the private record is absent from the written bundle.

This test only checks counters, while the privacy guarantee is no private bundle file exists. Add assertions on manifest["records"] IDs and that no public-only/memories/*.md file contains private or "Private content".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/tests/test_migration.py` around
lines 124 - 139, Extend the migration test around write_okf_bundle to verify the
public-only bundle contains only the public record ID in manifest["records"] and
excludes the private ID. Also inspect files under the public-only memories
directory and assert no markdown file contains “private” or “Private content”,
while preserving the existing counter assertions.

examples/migrations/crewai_unified_memory_okf/run_demo.py (1)

52-68: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add a timeout to the Memanto dry-run subprocess.

subprocess.run has no timeout, and capture_output=True hides the child's output while it runs. If the memanto CLI hangs, run_demo.py blocks with no progress indication and never writes the transcript. The README states the complete run finishes in under two minutes. Set a bound and report the timeout.

♻️ Proposed change

-        result = subprocess.run(
-            [
-                sys.executable,
-                "-m",
-                "memanto",
-                "migrate",
-                "okf",
-                str(okf_bundle),
-                "--dry-run",
-            ],
-            check=False,
-            capture_output=True,
-            text=True,
-            encoding="utf-8",
-            errors="replace",
-            env=environment,
-        )
+        try:
+            result = subprocess.run(
+                [
+                    sys.executable,
+                    "-m",
+                    "memanto",
+                    "migrate",
+                    "okf",
+                    str(okf_bundle),
+                    "--dry-run",
+                ],
+                check=False,
+                capture_output=True,
+                text=True,
+                encoding="utf-8",
+                errors="replace",
+                env=environment,
+                timeout=300,
+            )
+        except subprocess.TimeoutExpired as exc:
+            raise RuntimeError(
+                "Memanto dry-run did not finish within 300 seconds"
+            ) from exc

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/run_demo.py` around lines 52 -
68, Update the Memanto dry-run subprocess invocation in run_demo.py to enforce a
timeout consistent with the documented under-two-minute runtime, and handle
subprocess.TimeoutExpired by reporting the timeout clearly and preserving the
demo’s expected failure/transcript behavior.

examples/migrations/crewai_unified_memory_okf/generate_source.py (1)

265-309: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Close the CrewAI memory store on error paths.

memory.close() runs only after every remember, list_records, and recall call succeeds. If any of them raises, the LanceDB handle stays open. A later --force run then calls shutil.rmtree on that directory, which fails on Windows while the handle is held. Wrap the store usage in try/finally.

♻️ Proposed refactor using try/finally

     ids_by_key: dict[str, str] = {}
-    for item in SOURCE_MEMORIES:
-        record = memory.remember(
-            item["content"],
-            scope=item["scope"],
-            categories=item["categories"],
-            metadata=item["metadata"],
-            importance=item["importance"],
-            source=item["source"],
-            agent_role=item["agent_role"],
-        )
-        if record is None:
-            raise RuntimeError(f"CrewAI did not persist {item['key']}")
-        ids_by_key[item["key"]] = record.id
-
-    records = memory.list_records(scope="/", limit=100)
-    if len(records) != len(SOURCE_MEMORIES):
-        raise RuntimeError(
-            f"CrewAI stored {len(records)} records, expected {len(SOURCE_MEMORIES)}"
-        )
-
-    recall_results: list[dict[str, Any]] = []
-    for item in GOLDEN_QUESTIONS:
-        matches = memory.recall(item["question"], depth="shallow", limit=3)
-        expected_id = ids_by_key[item["expected_key"]]
-        rank = next(
-            (
-                index
-                for index, match in enumerate(matches, start=1)
-                if match.record.id == expected_id
-            ),
-            None,
-        )
-        recall_results.append(
-            {
-                "question": item["question"],
-                "expected_key": item["expected_key"],
-                "expected_id": expected_id,
-                "expected_rank": rank,
-                "top_ids": [match.record.id for match in matches],
-                "top_contents": [match.record.content for match in matches],
-            }
-        )
-
-    memory.close()
+    recall_results: list[dict[str, Any]] = []
+    try:
+        for item in SOURCE_MEMORIES:
+            record = memory.remember(
+                item["content"],
+                scope=item["scope"],
+                categories=item["categories"],
+                metadata=item["metadata"],
+                importance=item["importance"],
+                source=item["source"],
+                agent_role=item["agent_role"],
+            )
+            if record is None:
+                raise RuntimeError(f"CrewAI did not persist {item['key']}")
+            ids_by_key[item["key"]] = record.id
+
+        records = memory.list_records(scope="/", limit=100)
+        if len(records) != len(SOURCE_MEMORIES):
+            raise RuntimeError(
+                f"CrewAI stored {len(records)} records, expected {len(SOURCE_MEMORIES)}"
+            )
+
+        for item in GOLDEN_QUESTIONS:
+            matches = memory.recall(item["question"], depth="shallow", limit=3)
+            expected_id = ids_by_key[item["expected_key"]]
+            rank = next(
+                (
+                    index
+                    for index, match in enumerate(matches, start=1)
+                    if match.record.id == expected_id
+                ),
+                None,
+            )
+            recall_results.append(
+                {
+                    "question": item["question"],
+                    "expected_key": item["expected_key"],
+                    "expected_id": expected_id,
+                    "expected_rank": rank,
+                    "top_ids": [match.record.id for match in matches],
+                    "top_contents": [match.record.content for match in matches],
+                }
+            )
+    finally:
+        memory.close()

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/migrations/crewai_unified_memory_okf/generate_source.py` around
lines 265 - 309, Wrap the CrewAI memory operations beginning with the
SOURCE_MEMORIES loop and ending after the GOLDEN_QUESTIONS processing in a
try/finally block, and move memory.close() into the finally clause. Preserve the
existing validation and result-building behavior while ensuring the store closes
if remember, list_records, or recall raises.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes