Newer
Older
cortex-hub / docs / reviews / feature_review_node_metadata.md

Code Review Report: Feature 26 — Mesh Node Metadata Registry

This report performs a deep-dive audit of the AgentNode and NodeGroupAccess models in node.py, focusing on Mesh Inventory Scaling, ACL Integrity, and Foreign Key Performance.


🏗️ 12-Factor App Compliance Audit

Factor Status Observation
VI. Processes Success Distributed Heartbeat Storage: The last_seen_at and last_status columns (Line 21, 22) provide a unified "Single Source of Truth" for mesh health that survives Hub process cycles—enabling sub-second detection of offline nodes upon system reboot.
IV. Backing Services 🔴 Major Risk String-Key Referential Hazard: The NodeGroupAccess model uses node_id (a mutable String) as its foreign key (Line 35) rather than the immutable integer id. If a node is renamed in the registry, all associated Access Control lists will break referential integrity unless an explicit "ON UPDATE CASCADE" is managed manually at the application layer.

🔍 File-by-File Diagnostic

1. app/db/models/node.py

The persistent registry for mesh nodes, their capabilities, and access permissions.

[!TIP] Performance: Composite ACL Hits Line 31: NodeGroupAccess table. For large clusters with complex organizational policies, resolving "Which groups have access to this node?" will occur on almost every gRPC handshake and shell command dispatch.

Recommendation: Implement a unique composite constraint on (node_id, group_id) to prevent redundant permission records and enable index-only scans for ACL resolution.

Identified Problems:

  • JSON Default Mutation: The nested skill_config default (Line 14) is a dictionary. While safe in SQLAlchemy for new records, SQLAlchemy's "Change Tracking" for nested JSON mutations is notoriously inconsistent without using the MutableDict helper—potentially leading to silented "Save Failures" during partial config updates.
  • Orphaned Node Cleanup: While GroupAccess has delete-orphan (Line 26), there is no cascade from User (the registered_by field). If an Admin user is deleted, their registered nodes will remain orphaned in the database.

🛠️ Summary Recommendations

  1. Switch to Surrogate Foreign Keys: Transition NodeGroupAccess to use the integer id of AgentNode to ensure that node identifiers can be safely renamed or re-formatted without breaking ACLs.
  2. Enable Mutable Change Tracking: Utilize sqlalchemy.ext.mutable.MutableDict for the skill_config and capabilities columns to ensure that nested dictionary updates are correctly detected and committed to the database.
  3. Enforce ACL Constraints: Add a UniqueConstraint('node_id', 'group_id') to the NodeGroupAccess table to maintain sub-second permission evaluation and prevent record duplication.

This concludes Feature 26. I have persisted this report to /app/docs/reviews/feature_review_node_metadata.md. Shall I proceed to audit the final Asset model?