Skip to content

feat: sync the /memory/v1 dialect routes into the public surface - #45

Merged
senamakel merged 1 commit into
tinyhumansai:mainfrom
CodeGhost21:feat/memory-v1-dialect-routes
Oct 6, 2026
Merged

senamakel merged 1 commit into
tinyhumansai:mainfrom
CodeGhost21:feat/memory-v1-dialect-routes

Conversation

@CodeGhost21

@CodeGhost21 CodeGhost21 commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Why

Syncs against tinyhumansai/backend#1403, which adds /memory/v1/*: the same
hosted memory as /memory/*, answered in CortexDB's own dialect (memory-api's
status, memory-api's body, no { success, data } envelope) for a client written
against the engine rather than against this API. The envelope collapses a 409
into a 400 and a 202 into a 200, and the cortex memory driver's retry
safety and read-after-write barrier are built on seeing both.

What

Run as sync-openapi.mjs --input against that branch's dumped spec, per the
backend's CLAUDE.md. Eleven routes enter the public surface:

  • seven /memory/v1/*: experience, experience/bulk, recall, forget,
    answer, events, events/{id}, scopes/list, beliefs, beliefs/build.
  • four opencompany routes — .../open, GET/PUT .../password,
    .../password/reset — which were already public on backend main and absent
    here only because this repo's manifest had been generated from a deployed spec
    that predated them. Same situation as the GET /opencompany/companies entry
    already noted in the ratchet comments.

Two pinned counts move, both to 257, each with its reason inline:
manifest["source"]["operationCount"] and rust_routes.len().

Nothing was unblocked. excludedAdminOperationCount (47),
excludedWebhookOperationCount (12) and UNEXPOSED_ROUTES (59) are all
unchanged, and no public route was removed. The backend deliberately does not
expose /v1/admin/health for exactly this reason — no published path may carry
an admin segment, which rust_routes asserts structurally.

Testing

cargo fmt --all -- --check, cargo clippy --all-targets -- -D warnings,
cargo test (28 suites), cargo package — all clean.

Notes

No typed client methods are added for these routes. The consumer is OpenHuman's
cortex memory engine, which speaks the engine's dialect over its own HTTP
client rather than through this SDK; the generated route registry is what it
needs, so the raw transport admits them.

The manifest will diverge from the deployed spec until the backend change ships.
That divergence is expected and closes on deploy — please don't "fix" it by
resyncing from production.

Point the backend's sdk gitlink at this repo's main after merge.

Summary by CodeRabbit

  • New Features
    • Added public API routes for memory-related actions, including recalling information, recording experiences, managing beliefs and events, and listing available scopes.
    • Added routes for opening OpenCompany instances and reading, changing, or resetting instance passwords.

Syncs against the backend branch that adds `/memory/v1/*`: the same hosted
memory as `/memory/*`, answered in CortexDB's own dialect -- memory-api's
status, memory-api's body, no `{ success, data }` envelope -- for a client
written against the engine rather than against this API. The envelope collapses
a 409 into a 400 and a 202 into a 200, and the `cortex` memory driver's retry
safety and read-after-write barrier are built on seeing both.

Run as `sync-openapi.mjs --input` against that branch's dumped spec, per
backend CLAUDE.md, so it also brings in four opencompany routes (`.../open`,
`GET`/`PUT .../password`, `.../password/reset`) that were already public on
backend `main` and absent here only because this repo's manifest had been
generated from a deployed spec that predated them. Same situation as the
`GET /opencompany/companies` entry already noted in the ratchet comments.

Two pinned counts move, both to 254, each with its reason inline:
`manifest["source"]["operationCount"]` and `rust_routes.len()`. Nothing was
unblocked: `excludedAdminOperationCount` (47), `excludedWebhookOperationCount`
(12) and `UNEXPOSED_ROUTES` (59) are all unchanged, and no public route was
removed.

The manifest will diverge from the deployed spec until the backend change
ships. That divergence is expected and closes on deploy.

No typed client methods are added for these routes. The consumer is OpenHuman's
`cortex` memory driver, which speaks the engine's dialect over its own HTTP
client rather than through this SDK.
@tinysweeper

tinysweeper Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 6 lane(s) and found 1 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Ready for maintainer review
Priority: medium
Reviewed head: 59f5158ecfdf
Updated: 1791282036 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 1
Tests 1 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 1 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

  • medium · description · Correct the route counts: fourteen routes, ten memory/v1 — The body says "Eleven routes" in groups of "seven" and "four", but the diff adds ten `/memory/v1/*` routes (the list itself already names ten: `experience`, `experience/bulk`, `rec (\(pull request description\))

Before merge

None.

How this fits together

flowchart LR
  n0["..._api_key_request_uses_openapi_field_names"]:::impacted
  n1["path_segments_are_encoded_on_typed_routes"]:::impacted
  n2["create"]:::impacted
  n3["try_from"]:::impacted
  n4["get_feedback"]:::impacted
  n5["...ejects_the_machine_only_connections_scope"]:::impacted
  n0 -->|calls| n2
  n0 -->|tests| n2
  n1 -->|calls| n4
  n1 -->|tests| n4
  n5 -->|calls| n3
  n5 -->|tests| n3
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading
Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 3 files; 1 finding. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 3 files; 0 findings. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: This is a spec-sync change: eleven `/memory/v1/*` routes and four opencompany open/password routes are added to the generated route registry, with the OpenAPI manifest counts updated to match. The behavioural claim the diff makes — that no published path carries an `admin` segment and that webhook receivers stay out of the public surface — is pinned by structural assertions in `tests/openapi_sync.rs` that would fail if either invariant were broken, and the count assertions (`257` operations, `257` routes, manifest/route-list equality) would fail if the registry and manifest drifted apart. The added opencompany password routes are user-scoped instance operations, not admin credential helpers, so the repository rule against admin credential helpers is not engaged. The change looks coherent and safe to merge. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The diff itself is coherent — 14 new routes in the manifest, the Rust route registry and the pinned counts all agree — but the description's arithmetic is wrong: it claims "Eleven routes" in two groups of "seven" and "four", while the manifest and rust_routes actually gain ten `/memory/v1/*` routes plus four opencompany routes (fourteen total; the test comment's own 243→254→257 breakdown confirms this). The body also mirrors that miscount. Fix the numbers and this is safe to merge. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._
  • Evidence: \(pull request description\) — Correct the route counts: fourteen routes, ten memory/v1

e2e

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: No end-to-end harness in this repository: no e2e test files and no e2e workflow.
Evidence and run details
  • Models: gpt-5.6-luna, glm-5.3-flash, deepseek-v4.1-flash
  • Spend: $0.005296
  • Tokens: 125596 input · 7836 output · 12032 cached · 0 embedding
Head State Pass summary
59f5158ecfdf ready for maintainer review 1 active finding(s), 0 resolved finding(s) (at 1791282036)

tinysweeper 0.1.0

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5cbd5ff3-44c9-4d5f-b847-9877317a46a2
📥 Commits

Reviewing files that changed from the base of the PR and between 870c0d3 and 59f5158.

📒 Files selected for processing (3)
  • api/tinyhumans.backend.json
  • src/generated_public_routes.rs
  • tests/openapi_sync.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The OpenAPI manifest adds memory and OpenCompany routes and updates operation counts and namespace tags. The Rust public route list and OpenAPI sync test update to reflect the route additions.

Changes

Public API route inventory

Layer / File(s) Summary
Declare routes in the OpenAPI manifest
api/tinyhumans.backend.json
The manifest adds 10 memory routes and four OpenCompany instance routes. It updates the source and namespace operation counts. The teams namespace loses the OpenHuman parity tag, and the webhooks namespace uses the Webhooks tag.
Update public routes and sync expectations
src/generated_public_routes.rs, tests/openapi_sync.rs
PUBLIC_ROUTES adds the 10 memory routes and four OpenCompany routes. The sync test updates its expected route counts and comments describing route groups. UNEXPOSED_ROUTES and admin and webhook exclusion counts remain unchanged, according to the test comments.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: senamakel

Merge Risk: ⚪ Minimal · up to 59f51

The added routes match between the manifest and Rust inventory, and the sync test checks their identities and totals. No material merge-blocking risk is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 59f51

The change updates endpoint inventories rather than adding server handlers or weakening client-side restrictions. No introduced security issue was established. Backend authentication, tenant isolation, and instance ownership checks remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The newly advertised operations concern hosted memory and slug-selected instances, including password operations. Their effective data and privilege scope depends on backend authorization; the route inventory does not establish whether access is restricted to the caller's tenant or instance.

Trust Boundaries and Controls

  • inferred — These non-webhook additions do not change the SDK's raw-request permission boundary: the unchanged gate does not require general membership in PUBLIC_ROUTES. No SDK-side authorization bypass was established by the additions. This conclusion does not verify backend authentication or ownership enforcement.

Hardening Proposals

  • proposed — As follow-up assurance, verify backend bearer enforcement and cross-tenant ownership rejection for the advertised memory operations and instance open/password operations, including password reset. Treat this as validation of an external boundary, not as an observed vulnerability.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: syncing the /memory/v1 dialect routes into the public surface.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the route list twice,
Ten memory paths join the array.
Four OpenCompany routes appear,
The sync test counts each entry there.
Then hops away beneath the moon.

Comment @coderabbitai help to get the list of available commands.

@CodeGhost21 CodeGhost21 added enhancement New feature or request priority: p2 Soon. Real but survivable — a rough edge, a gap, a thing that will bite later. labels Oct 6, 2026

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

             $0.0053 · 125,596 in / 7,836 out · 12,032 cached (10%) · gpt-5.6-luna, glm-5.3-flash, deepseek-v4.1-flash
critique:    $0.0039 · 80,846 in  / 4,462 out · 8,329 cached (10%)  · gpt-5.6-luna, glm-5.3-flash
security:    $0.0011 · 21,983 in  / 789 out   · 3,639 cached (17%)  · gpt-5.6-luna
tests:       $0.0002 · 12,138 in  / 1,134 out · 0 cached (0%)       · glm-5.3-flash
description: $0.0001 · 5,923 in   / 780 out   · 64 cached (1%)      · glm-5.3-flash

@YellowSnnowmann

Copy link
Copy Markdown
Contributor

Verified the sync against the branch spec. generated_public_routes.rs adds exactly the 14 intended routes, the exclusion counters (excludedAdminOperationCount 47 / excludedWebhookOperationCount 12 / UNEXPOSED_ROUTES 59) are unchanged, and the no-admin-segment structural guard is intact — nothing was unblocked.

One fix: the body says "Eleven routes … seven /memory/v1/*", but the diff adds ten /memory/v1/* routes plus four opencompany routes = fourteen (the body's own bullet already lists ten memory routes). The manifest/test counts (257) are correct — it's only the prose. Matches the tinysweeper finding.

Minor: the sync also retags the Teams and Webhooks groups (drops the "OpenHuman parity" tag). Cosmetic, carried in from backend main — worth a one-line note in the body so it doesn't look unexplained.

The four opencompany password/open routes swept in are legitimate — already public on backend main, the SDK was just generated from an older deployed spec. Not accidental exposure.

Post-merge: point the backend sdk/ gitlink at this repo's main.

@senamakel
senamakel merged commit ab84a4b into tinyhumansai:main Oct 6, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request priority: p2 Soon. Real but survivable — a rough edge, a gap, a thing that will bite later.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants