feat(cipher): add TAC tree for Cipher Memory integration - #1023
Conversation
7-phase TAC tree covering: core service health, embedding pipeline quality (dark matter grade), MCP bridge verification, gateway routing and TensorZero observability, NATS event integration, BoTZ agent memory patterns, and advanced features (cross-dimensional queries, CHIT provenance, multi-agent sync). 17 check nodes ensure Cipher Memory properly integrates with the embedding pipeline, gateway, and BoTZ agent ecosystem. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughA new TAC Tree configuration file for Cipher Memory is introduced, defining a seven-phase audit plan that verifies the knowledge graph memory system's integration across services, embeddings, MCP bridge, gateway routing, NATS event handling, agent patterns, and advanced features like cross-dimensional queries and multi-agent synchronization. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb88a7f37b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| type: http | ||
| target: "http://localhost:8096/health" |
There was a problem hiding this comment.
http TAC action types
When this tree is executed by pmoves/tools/tac_runner.py or /tac:review, this node never performs the health check because the TAC schema and runner only implement file_exists, grep, command, and manual actions (pmoves/configs/tac_schema.yaml, pmoves/tools/tac_runner.py:evaluate_node). The result stays pending, and pending nodes do not make the runner exit nonzero, so a broken Cipher /health or Neo4j endpoint can still produce a successful audit. The same issue appears again at cipher.botz.knowledge-graph.
Useful? React with 👍 / 👎.
| target: "pmoves/services/cipher-api/" | ||
| pattern: "nats.*publish|NATS" |
There was a problem hiding this comment.
These checks target pmoves/services/cipher-api/, but that path does not exist in this repo. The cipher-api service is built from ../Pmoves-cipher (pmoves/docker-compose.yml:2108-2133), and the NATS publishing logic lives under pmoves-cipher-mcp/cipher_mcp/nats_events.py. As written, both Phase 5 greps will always fail with target not found, so the tree reports a regression even when Cipher’s NATS integration is present.
Useful? React with 👍 / 👎.
| type: grep | ||
| target: "pmoves/services/gateway/gateway/api/chit.py" | ||
| pattern: "cipher|memory" | ||
| expect: "Gateway CHIT API can delegate to Cipher for storage" |
There was a problem hiding this comment.
This regex is too broad to verify the behavior described in expect. In the current repo it already matches the unrelated cryptography.hazmat.primitives.ciphers.aead import in pmoves/services/gateway/gateway/api/chit.py, so this node can pass even if the gateway has no Cipher-memory delegation at all. That gives the TAC runner a green result for a feature it did not actually validate.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
pmoves/configs/tac_trees/cipher-memory.tac.yaml (2)
249-257: Avoid hardcoding agent names in TAC context text.This wording can drift from canonical registry data; keep the context tied to
pmoves/config/agent_registry.yamlinstead of listing specific agents inline.🔧 Suggested adjustment
- context: "ClawZ, Crush, Agent Zero all share Cipher memory layer" + context: "Agents defined in pmoves/config/agent_registry.yaml share the Cipher memory layer"As per coding guidelines, "Maintain canonical agent definitions in
pmoves/config/agent_registry.yamlas the source of truth for all 60 agents".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/configs/tac_trees/cipher-memory.tac.yaml` around lines 249 - 257, The TAC entry with id "cipher.advanced.multi-agent-sync" contains a hardcoded context string listing agent names; update the "context" value to avoid specific agent names and instead reference the canonical agent registry (do not enumerate agents) so the text stays accurate—e.g., replace "ClawZ, Crush, Agent Zero all share Cipher memory layer" with a generic pointer to the agent registry or a neutral description like "Agents defined in the agent registry share the Cipher memory layer"; edit the "context" field in that TAC block and keep the rest of the action (type: grep, pattern, expect) unchanged.
146-152: TensorZero observability check is too permissive.
pattern: "embed"can pass on unrelated text/comments. Use a specific section/key so this check is meaningful.🔧 Suggested adjustment
- - id: cipher.gateway.tensorzero-obs + - id: cipher.gateway.tensorzero-obs task: "Embedding calls logged in TensorZero" action: type: grep target: "pmoves/tensorzero/config/tensorzero.toml" - pattern: "embed" - expect: "Embedding model configured in TensorZero for observability" + pattern: "\\[embedding_models\\.qwen3_embedding_8b_local\\]|model_name = \"qwen3-embedding:8b\"" + expect: "qwen3-embedding:8b is configured in TensorZero for embedding observability"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/configs/tac_trees/cipher-memory.tac.yaml` around lines 146 - 152, The observability rule cipher.gateway.tensorzero-obs uses a too-broad pattern "embed"; tighten the grep pattern in the action to match the actual embedding configuration key or section (for example use a regex anchored to the config key such as ^\s*embedding_model\s*= or ^\s*model\s*=\s*".*embed.*"), ensure the grep/type supports regex or set the correct flag, and update the expect message to reflect the specific key being validated so the check only passes when the embedding config is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pmoves/configs/tac_trees/cipher-memory.tac.yaml`:
- Around line 108-116: The TAC node with id "cipher.mcp.transport" currently
asserts only "stdio" (task "MCP bridge uses stdio transport", action grep target
"pmoves-cipher-mcp" pattern "stdio"), which conflicts with compose using
"--mcp-transport-type sse"; update this node to validate the actual runtime mode
or accept both modes by changing the action to check for either "stdio" or "sse"
(or add a conditional that inspects the running args/env and then asserts the
matching transport), and update the expect/context messages accordingly so the
check covers both valid deployment wiring options.
- Around line 42-48: The grep rule under id cipher.service.neo4j-dep
(action.type: grep, pattern) is brittle because it assumes cross-line regex
matching and can miss a valid nested depends_on -> neo4j -> condition:
service_healthy block; replace the simple grep with a YAML-aware assertion
(parse the compose YAML and verify service cipher-api has
depends_on.neo4j.condition == "service_healthy") or, if keeping grep, change the
check to either a DOTALL-enabled regex (e.g., use a (?s) /s flag to allow
newlines) or split into sequential checks that confirm "depends_on:" exists,
that "neo4j" is listed under it, and that "condition: service_healthy" appears
in that service block so the nested structure is correctly validated.
- Around line 159-180: The TAC checks under ids cipher.nats, cipher.nats.publish
and cipher.nats.subscribe reference a non-existent target value
("pmoves/services/cipher-api/") so their grep actions will always fail; either
remove these TAC entries or update their action.target to point to the actual
cipher-api source location (the external cipher repo used by docker-compose)
and/or adjust the grep patterns (action.pattern) accordingly; ensure any
retained checks correctly reference the repository where cipher-api lives and
validate that expect messages still match the code found.
---
Nitpick comments:
In `@pmoves/configs/tac_trees/cipher-memory.tac.yaml`:
- Around line 249-257: The TAC entry with id "cipher.advanced.multi-agent-sync"
contains a hardcoded context string listing agent names; update the "context"
value to avoid specific agent names and instead reference the canonical agent
registry (do not enumerate agents) so the text stays accurate—e.g., replace
"ClawZ, Crush, Agent Zero all share Cipher memory layer" with a generic pointer
to the agent registry or a neutral description like "Agents defined in the agent
registry share the Cipher memory layer"; edit the "context" field in that TAC
block and keep the rest of the action (type: grep, pattern, expect) unchanged.
- Around line 146-152: The observability rule cipher.gateway.tensorzero-obs uses
a too-broad pattern "embed"; tighten the grep pattern in the action to match the
actual embedding configuration key or section (for example use a regex anchored
to the config key such as ^\s*embedding_model\s*= or
^\s*model\s*=\s*".*embed.*"), ensure the grep/type supports regex or set the
correct flag, and update the expect message to reflect the specific key being
validated so the check only passes when the embedding config is present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 08cbddd6-a4ec-4806-9f25-4ebe7832b54c
📒 Files selected for processing (1)
pmoves/configs/tac_trees/cipher-memory.tac.yaml
| - id: cipher.service.neo4j-dep | ||
| task: "Neo4j dependency declared" | ||
| action: | ||
| type: grep | ||
| target: "pmoves/docker-compose.yml" | ||
| pattern: "cipher.*neo4j|depends_on.*neo4j" | ||
| expect: "cipher-api depends on neo4j service_healthy" |
There was a problem hiding this comment.
Neo4j dependency check is brittle and can false-fail.
The current regex assumes cross-line matching in nested YAML, so valid depends_on -> neo4j -> condition: service_healthy config can be missed.
🔧 Suggested adjustment
- - id: cipher.service.neo4j-dep
- task: "Neo4j dependency declared"
- action:
- type: grep
- target: "pmoves/docker-compose.yml"
- pattern: "cipher.*neo4j|depends_on.*neo4j"
- expect: "cipher-api depends on neo4j service_healthy"
+ - id: cipher.service.neo4j-uri
+ task: "Neo4j backend URI configured"
+ action:
+ type: grep
+ target: "pmoves/docker-compose.yml"
+ pattern: "NEO4J_URI=bolt://neo4j:7687"
+ expect: "Cipher API points to Neo4j backend"
+
+ - id: cipher.service.neo4j-dep
+ task: "Neo4j dependency declared"
+ action:
+ type: grep
+ target: "pmoves/docker-compose.yml"
+ pattern: "condition: service_healthy"
+ expect: "Neo4j dependency is health-gated in compose"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/configs/tac_trees/cipher-memory.tac.yaml` around lines 42 - 48, The
grep rule under id cipher.service.neo4j-dep (action.type: grep, pattern) is
brittle because it assumes cross-line regex matching and can miss a valid nested
depends_on -> neo4j -> condition: service_healthy block; replace the simple grep
with a YAML-aware assertion (parse the compose YAML and verify service
cipher-api has depends_on.neo4j.condition == "service_healthy") or, if keeping
grep, change the check to either a DOTALL-enabled regex (e.g., use a (?s) /s
flag to allow newlines) or split into sequential checks that confirm
"depends_on:" exists, that "neo4j" is listed under it, and that "condition:
service_healthy" appears in that service block so the nested structure is
correctly validated.
| - id: cipher.mcp.transport | ||
| task: "MCP bridge uses stdio transport" | ||
| action: | ||
| type: grep | ||
| target: "pmoves-cipher-mcp/" | ||
| pattern: "stdio" | ||
| expect: "stdio transport for Claude Code integration" | ||
| context: "Runs as sidecar process, not HTTP" | ||
| agent_hint: codex |
There was a problem hiding this comment.
Transport assertion conflicts with current deployment wiring.
This node enforces stdio, but cipher-api in compose is configured with --mcp-transport-type sse. The TAC should validate the active runtime mode (or allow both modes explicitly).
🔧 Suggested adjustment
- - id: cipher.mcp.transport
- task: "MCP bridge uses stdio transport"
+ - id: cipher.mcp.transport
+ task: "MCP transport mode explicitly configured"
action:
type: grep
- target: "pmoves-cipher-mcp/"
- pattern: "stdio"
- expect: "stdio transport for Claude Code integration"
+ target: "pmoves/docker-compose.yml"
+ pattern: "--mcp-transport-type.*(sse|stdio)"
+ expect: "MCP transport mode is explicit for deployment/runtime"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - id: cipher.mcp.transport | |
| task: "MCP bridge uses stdio transport" | |
| action: | |
| type: grep | |
| target: "pmoves-cipher-mcp/" | |
| pattern: "stdio" | |
| expect: "stdio transport for Claude Code integration" | |
| context: "Runs as sidecar process, not HTTP" | |
| agent_hint: codex | |
| - id: cipher.mcp.transport | |
| task: "MCP transport mode explicitly configured" | |
| action: | |
| type: grep | |
| target: "pmoves/docker-compose.yml" | |
| pattern: "--mcp-transport-type.*(sse|stdio)" | |
| expect: "MCP transport mode is explicit for deployment/runtime" | |
| context: "Runs as sidecar process, not HTTP" | |
| agent_hint: codex |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/configs/tac_trees/cipher-memory.tac.yaml` around lines 108 - 116, The
TAC node with id "cipher.mcp.transport" currently asserts only "stdio" (task
"MCP bridge uses stdio transport", action grep target "pmoves-cipher-mcp"
pattern "stdio"), which conflicts with compose using "--mcp-transport-type sse";
update this node to validate the actual runtime mode or accept both modes by
changing the action to check for either "stdio" or "sse" (or add a conditional
that inspects the running args/env and then asserts the matching transport), and
update the expect/context messages accordingly so the check covers both valid
deployment wiring options.
| - id: cipher.nats | ||
| task: "NATS event bus integration" | ||
| context: "pmoves/services/cipher-api/" | ||
| agent_hint: codex | ||
| children: | ||
| - id: cipher.nats.publish | ||
| task: "Cipher publishes memory events" | ||
| action: | ||
| type: grep | ||
| target: "pmoves/services/cipher-api/" | ||
| pattern: "nats.*publish|NATS" | ||
| expect: "Publishes memory store/search events for agent coordination" | ||
| context: "BoTZ agents subscribe to cipher events for knowledge sync" | ||
| agent_hint: codex | ||
|
|
||
| - id: cipher.nats.subscribe | ||
| task: "Cipher subscribes to agent trail events" | ||
| action: | ||
| type: grep | ||
| target: "pmoves/services/cipher-api/" | ||
| pattern: "agent.graphiti.signed" | ||
| expect: "Subscribes to Graphiti trail for automatic memory ingestion" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Verify TAC targets exist =="
for p in \
pmoves/services/cipher-api \
pmoves-cipher-mcp \
pmoves/services/gateway/gateway/api/chit.py \
pmoves/services/hi-rag-gateway-v2/app.py \
pmoves/config/agent_registry.yaml \
pmoves/tensorzero/config/tensorzero.toml \
pmoves/docker-compose.yml
do
if [ -e "$p" ]; then
echo "FOUND $p"
else
echo "MISSING $p"
fi
done
echo
echo "== Show cipher-api source wiring in compose =="
rg -n -C2 'cipher-api:|build:|context:|mcp-transport-type' pmoves/docker-compose.ymlRepository: POWERFULMOVES/PMOVES.AI
Length of output: 12590
Remove or correct TAC checks targeting non-existent cipher-api path.
The TAC checks at lines 159-180 target pmoves/services/cipher-api/, which does not exist in this repository. Cipher-api is built from an external repository (../Pmoves-cipher, as seen in docker-compose.yml line 2111), so these grep patterns will not find the source code. Either remove these checks or adjust them to reference the correct external repository path if cipher-api source is included elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/configs/tac_trees/cipher-memory.tac.yaml` around lines 159 - 180, The
TAC checks under ids cipher.nats, cipher.nats.publish and cipher.nats.subscribe
reference a non-existent target value ("pmoves/services/cipher-api/") so their
grep actions will always fail; either remove these TAC entries or update their
action.target to point to the actual cipher-api source location (the external
cipher repo used by docker-compose) and/or adjust the grep patterns
(action.pattern) accordingly; ensure any retained checks correctly reference the
repository where cipher-api lives and validate that expect messages still match
the code found.
1 participant
Summary
Phases
Test plan
python -c "import yaml; yaml.safe_load(open('pmoves/configs/tac_trees/cipher-memory.tac.yaml'))"🤖 Generated with Claude Code
Summary by CodeRabbit