Repository navigation
feat(cortex): org:<uid> root — tenant-relative hosted paths and a retired user: root for the transition - #244
Conversation
Moved the envelope layout definitions out of the parent module into a dedicated layout module so the serialization structure is easier to locate and evolve independently. No behaviour changes. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
The V3 scope layout's prefix and parse logic is split into free functions that take the root as a parameter, so the same code can serve layouts with and without a root prefix. Behaviour is unchanged for existing layouts. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce engine modules for cortex items and scopes, providing the underlying storage and lookup logic these entities need. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Moved item handling and log reading helpers into their own modules to keep the engine file focused. No behaviour change. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce configuration and registry modules for the integrations crate so providers can be declared and looked up through a shared entry point. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
The retired root tests were extracted from the engine module into their own file so the engine module stays focused on runtime behaviour. No test logic changed. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
The retired-root read test now builds GetRequest with explicit ids and reach fields instead of the removed constructor, keeping the test compiling against the current request type. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
The hosted tenant test now filters the engine without the retired root to an exact reach at the person's own nodes and asserts the single remaining item's text, so the check pins down which root the surviving entry came from instead of only counting results. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add layout tests for the tenant layout and for a layout with a retired root, checking that reads double across both roots while writes stay on the active one and that retired roots are validated. Also cover the registry rules that make a tenant root hosted-only and require a layout before a retired root can be set. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Rustfmt-compliant wrapping was applied to the retired-root engine and envelope layout tests, splitting long store chains and assert! calls across lines. No test behaviour or coverage changed. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
The layout docs now describe the hosted wire's tenant root, where the backend pins every scope below the caller's `org:<id>`, and the retired `user:<id>` root that stays readable and forgettable while its memory is moved. Examples and integration guidance were updated to use `org:<id>` as the scope root, with `user:` reserved for the actor. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a README section describing the per-person scope layout, showing how org, app, workspace, and service segments compose under a single root. It also notes how each engine is configured and that memory written under the older user-scoped layout stays readable until migrated. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Tiny Sweeper reviewTiny Sweeper completed its review; deterministic results follow. State: Changes requested Review snapshot
Completeness: Complete What changedIntroduces a tenant-rooted hosted layout with an `org:<id>` root and no `user:` scope segment, plus a retired-root transition. `EngineSettings` gains `tenant_root` and `retired_scope_root` (`crates/tinymemory-integrations/src/config/mod.rs#pub struct EngineSettings {`); `CortexEngine` gains `with_tenant_root` and `with_retired_root`, with registration skipped when the root is empty (`crates/tinymemory-integrations/src/cortex/engine/mod.rs#impl CortexEngine {`). `ScopeLayout` gains `tenant()`, `with_retired_root`, `retired`, `roots`, `paths`, `is_retired`, and free functions `checked_root`, `render`, `node_prefixes_below`, `parse_below` (`crates/tinymemory-integrations/src/cortex/envelope/layout.rs#impl ScopeLayout {`). `KindScope` gains `read` and `listed`, and reads are doubled over `roots()` in `held`, `every_scope` and the discovery paths (`crates/tinymemory-integrations/src/cortex/engine/scopes.rs#impl KindScope {`, `crates/tinymemory-integrations/src/cortex/engine/scopes.rs#impl CortexEngine {`). List/get results are deduplicated by item id (`crates/tinymemory-integrations/src/cortex/engine/items.rs#impl CortexEngine {`), and recall interleaving dedupes by id so an item held in both roots is one hit at its best rank (`crates/tinymemory-integrations/src/cortex/engine/fetch.rs#impl CortexEngine {`). Listing shadows retired-root twins against the active root (`crates/tinymemory-integrations/src/cortex/engine/list.rs#impl CortexEngine {`). An empty prefix omits the `prefix=` query parameter (`crates/tinymemory-integrations/src/cortex/log/read.rs#impl Log {`). `build_engine`/`list_engines` wire the new settings and their error documentation (`crates/tinymemory-integrations/src/registry/mod.rs#pub fn build_engine(`, `crates/tinymemory-integrations/src/registry/mod.rs#pub fn list_engines() -> Vec<EngineDescriptor> {`). Documentation is rewritten from `user:` to `org:` roots with new hosted-wire and retired-root sections (`README.md#route has no keyword/vector switch, so both wires declare hybrid fetch only. See`, `crates/tinymemory-integrations/src/cortex/README.md#app:tinymemory/team:acme/agent:writer/app:learnings a team m`, `docs/architecture/cortex-layout.md#A root is `type:id` segments of the types CortexDB's hosted API admits`, `docs/integration.md#actor header and the headers the HTTP stack sets, so the credential stays`, `docs/specs/memory-v2.md#credentialed cleartext non-loopback endpoint, all as `Error::Config`.`). Features
Tests
Findings
Resolved this pass
Before merge
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
Warning Review limit reached
This review includes 18 billable files and costs up to $4.50. Or wait 35 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (18)
Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7c229bec8
ℹ️ 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".
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0392 · 843,253 in / 41,159 out · 75,894 cached (9%) · gpt-5.6-luna, glm-5.3-flash, deepseek-v4.1-flash
critique: $0.0242 · 484,583 in / 23,553 out · 48,943 cached (10%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0144 · 261,460 in / 11,189 out · 23,367 cached (9%) · gpt-5.6-luna
tests: $0.0002 · 23,658 in / 598 out · 1,856 cached (8%) · glm-5.3-flash
description: $0.0001 · 23,603 in / 1,590 out · 0 cached (0%) · glm-5.3-flash
e2e: $0.0002 · 25,763 in / 708 out · 1,728 cached (7%) · glm-5.3-flash
Fetching items from the engine now keeps the stored payload intact instead of dropping it during envelope decoding, so callers receive the full item data rather than a stripped-down record. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Move the item lookup and formatting helpers out of the list engine into a dedicated items module so both can be reused by other engine operations. No behaviour change. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add tests asserting that an item ranked under both the active and retired root is listed once at its best rank across pages, and that writes in the transitional layout only land below the new root. Also cover parsing of hosted tenant paths with a prefix, including retired-root paths behind it. Reformat two call sites in the list engine with no behaviour change. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
The scope root section now explains that memory written below an earlier `user:<id>` root stays readable and forgettable only while the engine names that root as well, so both roots must be configured during the move. Writes go to the new root only, and `retired_scope_root` can be dropped once the memory has moved. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76ed6312ce
ℹ️ 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".
| retired | ||
| .as_deref() | ||
| .and_then(|retired| parse_below(retired, *prefixed, path)) | ||
| .or_else(|| parse_below(root, *prefixed, path)) |
There was a problem hiding this comment.
Match the retired root only at the tenant boundary
With a tenant-root layout retiring user:42, an active namespace such as ws:main/user:42 is valid, but this retired-first call delegates to parse_below(..., prefixed = true), which finds user:42 anywhere in the path. It therefore misparses org:<tenant>/ws:main/user:42/app:learnings as a retired-root scope at the root namespace; subtree reads at ws:main then omit it, while exact list reads can classify the active scope as retired and shadow its own items. Restrict retired-root matching to the first segment after the hosted org: prefix.
Useful? React with 👍 / 👎.
| found | ||
| .entry(ItemId::new(id.clone())) | ||
| .or_insert_with(|| hit(&id, &item, 0.0)); |
There was a problem hiding this comment.
Preserve active-root priority in get lookups
For a hosted tenant-root request at ws:main, known sorts the retired path user:42/ws:main/... before the active path ws:main/...; this first-write-wins insertion therefore retains the retired copy, and the outer loop stops before querying the active scope once all requested IDs are found. If the copies diverge during migration, get returns stale or partial retired data even though the new ordered assembly path now prefers the active copy. Consume scopes in active-then-retired order, using the retired item only as a fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0352 · 740,822 in / 48,161 out · 57,571 cached (8%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0207 · 408,638 in / 24,124 out · 39,567 cached (10%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0135 · 217,594 in / 15,230 out · 18,004 cached (8%) · gpt-5.6-luna
tests: $0.0003 · 27,352 in / 3,069 out · 0 cached (0%) · glm-5.3-flash
description: $0.0002 · 27,301 in / 1,411 out · 0 cached (0%) · glm-5.3-flash
e2e: $0.0003 · 29,588 in / 1,278 out · 0 cached (0%) · glm-5.3-flash
| #[tokio::test] | ||
| async fn a_hosted_tenant_engine_reads_the_retired_user_segment_too() { | ||
| let (endpoint, state) = hosted_double().await; | ||
| let old = hosted_engine(&endpoint) |
There was a problem hiding this comment.
Skip the hosted tenant prefix when parsing tenant paths
This configures the hosted engine with a root containing the retired user segment, but the test does not establish that the hosted tenant prefix is removed before the path is parsed. A parser that treats the hosted prefix as part of the namespace could still make this test pass through the double's broad routing behavior. Add an assertion on the actual request path or configure a concrete hosted tenant prefix and verify the resulting Cortex path is relative to the tenant root.
[RULE] hosted-path-parsing ·
| ))) | ||
| .await | ||
| .unwrap(); | ||
| assert_eq!(report.erased_scopes, 2); |
There was a problem hiding this comment.
Include retired roots when collecting scopes for erasure
This only verifies the count and later checks that two expected paths appeared in the fake's erasure log. It does not exercise a retired root containing a scope that is absent from the new root's scope listing, so an implementation that collects erasure scopes only from the active root can still pass. Add a retired-only scope and assert that its exact path is erased.
[RULE] retired-root-erasure ·
| reach: Some(Reach::exact("ws:main".parse().unwrap())), | ||
| ..MetaFilter::default() | ||
| }; | ||
| let exact = new.list(ListRequest::new(at_main, 10)).await.unwrap(); |
There was a problem hiding this comment.
Use the root occurrence when validating the canonical path
The exact-reach assertion checks only the number of returned items. It does not verify the canonical namespace/path associated with each root occurrence, so a path-normalization bug can still return two items while assigning them to the wrong root or metadata. Assert the returned metadata/path for the retired and active items separately.
[RULE] root-path-validation ·
| // Hosted, the backend may answer a tenant-root path behind the | ||
| // tenant's own `org:<id>`. `org` is never a namespace segment, so a | ||
| // leading `org:` segment is that prefix. | ||
| usize::from(prefixed && parts.first().is_some_and(|part| part.starts_with("org:"))) |
There was a problem hiding this comment.
Parse the complete hosted tenant prefix
For the tenant layout, this skips only the first segment whenever it starts with org:. A hosted path whose backend prefix contains more than one segment, such as org:42/dept:shared/ws:main/app:conversations, is then parsed from dept:shared, producing the wrong namespace (or failing canonical validation) instead of parsing ws:main. The surrounding v3 parser already treats hosted prefixes as variable-length by locating the root occurrence; the empty-root case needs equivalent prefix handling based on the backend's actual tenant-prefix contract rather than dropping exactly one segment.
[RULE] incorrect-prefix-parsing ·
| #[tokio::test] | ||
| async fn reads_merge_both_roots_by_item_id_and_writes_go_only_to_the_new_one() { | ||
| let (endpoint, state) = direct_double().await; | ||
| let shared = learning("held in both", "ws:main"); |
There was a problem hiding this comment.
Select a deterministic source for duplicate items
Both copies of the duplicate item are byte-for-byte identical, so the assertions only prove that one item is returned, not which root supplied its metadata or other conflicting fields. If the roots contain the same id with different metadata, an unordered merge could still pass these tests while returning nondeterministic data. Store distinct metadata in the two copies and assert the documented precedence explicitly.
[RULE] insufficient-test-oracle ·
| // The transitional write adds scopes below the new root only. | ||
| let added: Vec<_> = written.difference(&before).collect(); | ||
| assert!(!added.is_empty(), "{written:?}"); | ||
| assert!( |
There was a problem hiding this comment.
Assert that transition writes never target the retired root
Checking that added scopes start with org:42 only catches writes recorded as scope events and does not prove that no transition operation targets user:42; a malformed or separately logged write can evade this assertion. Assert directly over all write requests/registrations that no target path equals or is below the retired root, while retaining the positive new-root assertions.
[RULE] write-target-validation ·
| direct_engine(endpoint) | ||
| .with_scope_root("org:42", Some("user:42")) | ||
| .unwrap() | ||
| .with_retired_root("user:42") |
There was a problem hiding this comment.
Show a complete configuration for the retired user root
The test only supplies the retired root string and relies on the helper's defaults for its ownership and routing configuration. That leaves the configuration contract under-specified: an implementation can ignore the retired-root owner or derive it incorrectly while still satisfying the double. Configure and assert the complete retired-root settings, including the root's owner/actor and the resulting registration.
[RULE] retired-root-configuration ·
| let own = if root.is_empty() { | ||
| parts[start..].join("/") | ||
| } else { | ||
| parts[start - root.split('/').count()..].join("/") | ||
| }; |
There was a problem hiding this comment.
Use the matched root occurrence when validating the canonical path
For a hosted path such as org:tenant/user:42/app:conversations, start points at the matched user:42 occurrence. Subtracting the root length moves the slice back to the tenant prefix, so it can never equal render(root, ...), which starts at user:42. Consequently hosted reads, listing, and erasure parsing for non-empty scope roots are rejected even when the path is canonical. Compare from start itself so the canonical validation covers the matched root occurrence.
| let own = if root.is_empty() { | |
| parts[start..].join("/") | |
| } else { | |
| parts[start - root.split('/').count()..].join("/") | |
| }; | |
| let own = parts[start..].join("/"); |
[RULE] canonical-path-validation ·
| .collect::<HashSet<_>>() | ||
| .into_iter() | ||
| .collect(); | ||
| let active = KindScope::new(&self.layout, scope.namespace.clone(), scope.kind); |
There was a problem hiding this comment.
Only shadow retired items after confirming the active copy is complete
This treats any event found in the active scope as sufficient to shadow the retired copy. During a partial transition, the active scope can contain only some events for a conversation or chunked document while the retired scope still contains the complete item. The listing then suppresses the retired occurrence, and resolve attempts assembly from the incomplete active events and returns no item, losing it from listings and exports. Check that the active events rebuild successfully as a whole before marking the id shadowed; otherwise retain the retired occurrence as the fallback.
[RULE] incomplete-fallback ·
| ..EngineSettings::default() | ||
| }; | ||
| assert!(build_engine("cortexdb", &direct, key()).is_ok()); | ||
| let legacy = EngineSettings { |
There was a problem hiding this comment.
Drive the tenant-root and retired-root settings against a live CortexDB
Still standing from the earlier review: all the new behaviour is exercised against in-process doubles (direct_double, hosted_double). The wire-level details this change depends on — that the hosted backend really bounds an unprefixed scope listing to the caller's tenant, refuses an empty prefix=, and pins org:<id> — are asserted only against the doubles' own assumptions. An integration run against a real CortexDB/TinyHumans endpoint would pin the contract the doubles encode; until then the doubles could agree with the client and disagree with the backend. No code change suggested; this is a coverage gap, not a known defect.
[RULE] test-coverage ·
What
The canonical memory root is now
org:<uid>, with nouser:<uid>segment anywhere in a scope path (memory audit 01, F10).CortexEngine::with_tenant_root()/EngineSettings::tenant_root. A v3 engine on the TinyHumans wire names no root and sendsws:main/app:conversations. memory-api pinsorg:<uid>, so the stored path isorg:<uid>/ws:main/app:conversationsinstead oforg:<uid>/user:<uid>/ws:main/…. An unprefixed scope listing covers the whole tenant: the backend bounds it to the caller, and an emptyprefix=is never sent because memory-api refuses it. This root is refused on the direct wire.CortexEngine::with_retired_root(root)/EngineSettings::retired_scope_root.ScopeLayout::paths/KindScope::read. That covers list, fetch, recall, get, explore, assembled conversations and beliefs, and it reads each node below both the root and the retireduser:<uid>root. Results merge by item id.user:<uid>/ws:main/…reads back asws:main, not as a node that starts withuser:.org:<uid>with the actoruser:<uid>as owner. The layout code is generic, so that is a host setting and needs no tinymemory change; the docs now show it.docs/architecture/cortex-layout.md,docs/integration.md, the cortex README andmemory-v2.mdnow showorg:<uid>/…paths.Tests
envelope/layout_tests.rs: tenant-layout paths with no root, a retired root doubling reads but not the write path, retired-root parse precedence and validation.engine/mod_retired_root_tests.rs:prefix=;org:42;registry/mod_tests.rs:tenant_rootworks only on the hosted wire, andretired_scope_rootneeds v3.cargo test --workspace,cargo clippy --workspace --all-targets -D warningsandcargo fmt --checkare green.Rollout order
[memory] legacy_user_segment_read = true(default).reroot-user-segment:--dry-runfirst, then for real. This copiesorg:<uid>/user:<uid>/<rest>toorg:<uid>/<rest>, verifies per scope, then erases the old subtree.kept_old: null, and nothing remains under anyorg:<uid>/user:<uid>.legacy_user_segment_read = false); the host then stops passingretired_scope_root.Follow-ups: cortexdb-saas reroot PR, then an OpenHuman gitlink bump with the host wiring.