Add procedural memory layer

[Velamj]

Summary

Split PR 1 from the broad procedural/retrieval benchmark draft (#93).

This PR adds the first-class procedural memory layer only:

• widens memories.memory_type to support procedural;
• adds canonical procedure tables and FTS support via migration 052;
• adds the procedural service layer, Brain API methods, CLI procedure commands, and MCP procedural tools;
• keeps bridge rows in memories so legacy memory search still has a synopsis;
• adds migration safety notes in docs/PROCEDURAL_MEMORY_MIGRATION.md.

Generated benchmark result bundles, retrieval reranker changes, and benchmark comparison harness changes are intentionally not included in this split PR.

Validation

Ran locally on Windows PowerShell:

python -m py_compile src\agentmemory\procedural.py src\agentmemory\commands\procedure.py src\agentmemory\mcp_tools_procedural.py src\agentmemory\mcp_server.py
python -m pytest tests\test_procedural.py tests\test_mcp_tools_procedural.py tests\test_migrate.py tests\test_validation.py -q
python -m pytest tests\test_migrate.py -q

Results:

• procedural/MCP/migration/validation targeted tests: 70 passed;
• migration-only recheck after doc clarification: 51 passed.

Migration Notes

See docs/PROCEDURAL_MEMORY_MIGRATION.md for transaction safety, backward-compat expectations, rollback/failure behavior, versioning notes, and fresh-install parity requirements.

Follow-up clarification commit 013d1dd made the transaction and compatibility boundary explicit:

• migration 052 starts with PRAGMA foreign_keys = OFF; BEGIN; and only re-enables FKs after COMMIT;;
• the migration runner records schema_versions only after the SQL script completes;
• legacy episodic/semantic reads and writes remain expected to work, while procedure-aware writes/search require the new version;
• mixed-version operators should roll writers forward before enabling procedural writes.

Versioning

This PR intentionally does not bump pyproject.toml by itself. The version bump should land in the release/integration PR that decides the final bundle of procedural memory plus any coordinated retrieval changes. The compatibility note in docs/PROCEDURAL_MEMORY_MIGRATION.md is the required release-note content for that bump.

New MCP Tools

• procedure_add: create a structured procedure and bridge memory synopsis.
• procedure_get: fetch one procedure by ID, including steps/sources when available.
• procedure_list: list procedures by status/scope with pagination limit.
• procedure_search: FTS-backed procedure search with optional debug diagnostics.
• procedure_update: update structured procedure metadata/JSON fields.
• procedure_feedback: record execution outcome and update procedure quality counters/status.
• procedure_backfill: deterministic free-text/workflow backfill into procedure candidates or procedures.
• procedure_stats: aggregate procedure counts/status/usage metrics.

[TSchonleber]

Review of split #1 (procedural memory layer).

Structure ✅

Clean isolation from #95/#96. Scope matches title and body:

• Core procedural files: procedural.py (+1679), commands/procedure.py (+260), mcp_tools_procedural.py (+324)
• Migration 052 (+510) + mirrored fresh-install schema changes to both db/init_schema.sql (+1884) and src/agentmemory/db/init_schema.sql (+157) — the dual-path is a pre-existing repo quirk, not introduced here
• mcp_server.py is now +35/-9 (down from the -175 rewrite in Improve legacy benchmark retrieval flow #93) — just the procedural tool wiring, which is what we want
• Bridge row in memories preserved so legacy search still has a synopsis — good compat choice
• 70 local tests passing

Migration doc — genuinely good ✅

Read docs/PROCEDURAL_MEMORY_MIGRATION.md in full. Addresses every item from the prior review:

• Transaction safety — explicit note on the CHECK-constraint rebuild path (table rebuild + row copy + swap) with SQLite rollback semantics if the transaction fails pre-commit
• Backwards compat — newer brainctl reads older DBs and migrates; older brainctl keeps reading migrated DBs for episodic/semantic rows but won't understand procedural rows. Operators who need strict older-version compat are told not to write procedural rows before rolling clients forward. That's the honest answer.
• Rollback — brain.db backup restore path + append-only migration rule (add a new numbered migration for corrections, never edit 052 in place)
• Version bump — explicit requirement, with compat-matrix language about read vs write requirements
• Fresh install parity — doc explicitly says both init_schema.sql paths must carry the same procedural schema, and references tests/test_schema_parity.py + tests/test_migrate.py as the enforcement tests

Spot-checked: tests/test_schema_parity.py already exists on main and its docstring confirms it asserts fresh-install vs migrated byte-equivalence of sqlite_master dumps. So the parity promise is actually enforced in CI, not just documented. That's the gold-standard answer.

Open items

1. CI still not reporting — first-time-contributor gate on the repo owner side. Not yours. @TSchonleber needs to approve the workflow run from the Actions tab; once cleared, I want to see test_schema_parity, test_migrate, test_procedural, test_mcp_tools_procedural, test_validation all green here before lifting draft.
2. Version bump — the doc says this should ship with a version bump. I don't see a pyproject.toml change in this PR's file list. Either bump it here, or explicitly defer to the release PR that bundles this with the retrieval work. Worth a one-line note in the PR body either way.
3. Merge coordination with Add retrieval executive and listwise reranker #96 — both this PR and Add retrieval executive and listwise reranker #96 modify src/agentmemory/_impl.py from the same upstream/main base. Whichever lands second will need a rebase. Not a blocker for this review, just flagging so neither of you is surprised.
4. MCP tool additions — mcp_tools_procedural.py adds new MCP tools for procedure management. Quick sanity ask: can you list the new tool names in the PR body (or in MCP_SERVER.md) so downstream agent inventories (Claude Code, Codex, Hermes, Devin) can be updated without having to diff the code? Just the names + one-line descriptions.

Otherwise this is the cleanest split of the three and ready for line-by-line review once CI is unblocked. Nice work.

[TSchonleber]

CI approved and ran — immediately failed in actions/checkout across every job:

##[error]error: unable to create symlink db/init_schema.sql: File name too long

Diagnosis

Inspecting the PR tree, the two init_schema.sql files have different git modes:

Path	Mode	Size
src/agentmemory/db/init_schema.sql	100644 (regular file)	79,449 bytes
db/init_schema.sql	120000 (symlink)	79,449 bytes

db/init_schema.sql was committed with git mode 120000 (symlink) instead of 100644 (regular file). On the Linux CI runner, checkout tries to create a symlink whose target is the 79 KB of SQL content, which blows past the PATH_MAX limit → File name too long. Size match (79,449 bytes exactly) suggests you copied the content into db/init_schema.sql on Windows in a context where git recorded it as a symlink rather than a regular file.

This is a known Windows+git gotcha — usually triggered by core.symlinks=true plus the source being a Windows symlink/junction, or by a prior git update-index --add --cacheinfo 120000,... on the path.

Fix

From any platform, on the codex/procedural-memory-layer branch:

git checkout codex/procedural-memory-layer

# Overwrite db/init_schema.sql with the actual file content and force git to
# treat it as a regular file, not a symlink.
git cat-file blob HEAD:src/agentmemory/db/init_schema.sql > db/init_schema.sql
git update-index --chmod=-x db/init_schema.sql
git add db/init_schema.sql
git commit --amend --no-edit
git push --force-with-lease

If that doesn't flip the mode, the blunt version:

git rm --cached db/init_schema.sql
git cat-file blob HEAD:src/agentmemory/db/init_schema.sql > db/init_schema.sql
git add db/init_schema.sql
git commit --amend --no-edit
git push --force-with-lease

After re-push, verify with:

git ls-tree HEAD db/init_schema.sql

You want the leading mode to read 100644, not 120000.

Also

While you're in the fixer branch, consider adding to your Windows dev loop:

git config core.symlinks false

at least in this repo's clone — brainctl maintains the dual db/init_schema.sql + src/agentmemory/db/init_schema.sql parity intentionally (enforced by tests/test_schema_parity.py on main), and on Windows this is the most likely pattern to silently produce symlink blobs again.

Status of the other splits

For visibility: CI approved on all four of your PRs this round. #95 and #96 are currently in_progress — I'll report results once they complete. #94 needs the fix above before it can even reach the test stage.

[Velamj]

Addressed the open review items from this comment.

Version bump

I did not bump pyproject.toml in this split PR. I added an explicit Versioning section to the PR body stating that the version bump should land in the release/integration PR that decides the final procedural/retrieval bundle. The migration doc already contains the compatibility language that should go into that release note.

MCP tool inventory

I added a New MCP Tools section to the PR body with the full procedural MCP tool list and one-line descriptions:

• procedure_add
• procedure_get
• procedure_list
• procedure_search
• procedure_update
• procedure_feedback
• procedure_backfill
• procedure_stats

Current status

No code change was needed for this pass. The branch is still clean and pushed at 013d1dd. CI is still owner-gated per your note.

[Velamj]

Fixed the checkout blocker from this comment.

What changed

Pushed commit e11db77 (Fix init schema file mode) on codex/procedural-memory-layer.

The only change is the git file mode for db/init_schema.sql:

120000 => 100644 db/init_schema.sql

The blob/content hash remains identical to src/agentmemory/db/init_schema.sql:

100644 blob 33056a9f37c52c3688df32a0347830fb3e11b111 db/init_schema.sql

So this should remove the Linux checkout failure without changing schema content.

Local prevention

I also set this clone's local config to avoid recreating the issue on Windows:

git config core.symlinks false

Validation

python -m pytest tests\test_schema_parity.py tests\test_migrate.py -q

Result: 54 passed.

This should let CI get past actions/checkout on #94 now.

[TSchonleber]

Correction on my earlier guidance — I owe you an apology. My previous comment told you to flip db/init_schema.sql from a symlink to a regular file. That was wrong. The repo has a test that explicitly requires it to be a symlink:

tests/test_audit_2_4_11_regressions.py::test_root_init_schema_is_a_symlink_to_packaged FAILED
AssertionError: db/init_schema.sql must be a symlink to the packaged schema so the two copies can't drift

The symlink itself is correct and intentional — it's the drift-prevention mechanism between the two init_schema.sql paths that the PROCEDURAL_MEMORY_MIGRATION.md doc references. The original Linux checkout error (File name too long) wasn't because it shouldn't be a symlink, it's because the symlink was committed with the 79 KB of SQL content as the target path instead of the actual relative path target.

The real fix

db/init_schema.sql needs to be a symlink whose target is the short relative path to the packaged schema:

db/init_schema.sql -> ../src/agentmemory/db/init_schema.sql

The git blob for that symlink should be ~36 bytes (just the target path string), not 79 KB.

On macOS/Linux

git checkout codex/procedural-memory-layer
cd db
rm init_schema.sql
ln -s ../src/agentmemory/db/init_schema.sql init_schema.sql
cd ..
git add db/init_schema.sql
git commit --amend --no-edit
git push --force-with-lease

On Windows (which is why it broke originally) — use git plumbing to avoid OS filesystem symlink semantics

# Keep this on in your clone
git config core.symlinks true

# Create a blob whose content is just the relative path string
BLOB=$(printf "../src/agentmemory/db/init_schema.sql" | git hash-object -w --stdin)
echo "blob: $BLOB"   # should be ~40-char sha, blob size ~36 bytes

# Stage it as a symlink (git mode 120000)
git rm --cached db/init_schema.sql 2>/dev/null || true
git update-index --add --cacheinfo 120000,$BLOB,db/init_schema.sql

git commit --amend --no-edit
git push --force-with-lease

Verify

git ls-tree HEAD db/init_schema.sql
# Expect: 120000 blob <sha>  db/init_schema.sql

git cat-file -p HEAD:db/init_schema.sql
# Expect exactly: ../src/agentmemory/db/init_schema.sql

The other two failures in this run

1. tests/test_cli.py::TestCLISearch::test_search_includes_procedures_bucket — fails with assert []. Search for procedures bucket returns empty. Likely a consequence of the broken symlink (schema mismatch between what gets loaded vs what the test expects). Should self-resolve once the symlink is restored. If it persists, it's a separate bug in the procedural search path.

2. tests/test_docs_check.py::test_docs_match_implementation — "Docs drift detected". Run pytest tests/test_docs_check.py -v locally to see which doc file is out of sync — probably MCP_SERVER.md needs the new procedural tool names added, or one of the procedural docs hasn't been updated with the generated-from-code surface.

Sorry for the misdirection on the symlink. Restoring it should unblock most of this.

[Velamj]

Addressed the latest #94 feedback.

Changes pushed to codex/procedural-memory-layer:

• 7ce7bb8 (Restore init schema symlink target)

  • Restores db/init_schema.sql to git mode 120000.
  • The symlink blob is now the short target path ../src/agentmemory/db/init_schema.sql.
  • Verified with:
    • git ls-tree HEAD db/init_schema.sql -> 120000 blob ... db/init_schema.sql
    • git cat-file -p HEAD:db/init_schema.sql -> ../src/agentmemory/db/init_schema.sql
    • git cat-file -s HEAD:db/init_schema.sql -> 37

• 54ca416 (Fix procedural search and docs drift)

  • Fixed the split-PR-only procedural search issue: Add procedural memory layer #94 does not include the retrieval package from Add retrieval executive and listwise reranker #96, so cmd_search now falls back directly to agentmemory.procedural.search_procedures(...) when agentmemory.retrieval.* is unavailable.
  • Updated MCP_SERVER.md tool count from 201 to 209.
  • Made scripts/check_docs.py print ASCII OK instead of a checkmark so the docs test does not fail under Windows cp1252.

Validation run locally:

python -m pytest tests/test_docs_check.py tests/test_cli.py::TestCLISearch::test_search_includes_procedures_bucket -q
# 2 passed

python -m py_compile scripts\check_docs.py src\agentmemory\_impl.py src\agentmemory\procedural.py src\agentmemory\commands\procedure.py src\agentmemory\mcp_tools_procedural.py

Note: I verified the symlink via git object mode/blob content rather than Path.is_symlink() locally because this Windows shell does not have symlink creation privileges. Linux CI checkout should materialize the symlink normally from the fixed git object.