fix(memory): harden user-memory taosmd proxy per review#728
Merged
Conversation
📝 WalkthroughWalkthroughAdds taosmd-backed search and ingest proxying to the user-memory API with SQLite fallback, dual-write save behavior, pagination support for browse, and a bulk migrate endpoint with health checks and fallback strategies; tests cover search, save, migrate, pagination, and error-hardened metadata handling. ChangesTaosmd Integration and Migration
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tinyagentos/routes/user_memory.py`:
- Line 183: The migration currently only fetches the first 10,000 rows because
of the hardcoded limit in chunks = await _store(request).browse(USER_ID,
limit=10_000); change the logic in the route (user_memory.py) to page through
the store instead of a single limited call: repeatedly call the store's
browse/scan method (using offset/cursor or next_token returned by browse) and
accumulate results until no more pages, or add a store helper like
browse_all(user_id) that yields/returns all chunks; ensure you replace uses of
the truncated chunks variable with the fully accumulated list and compute/return
the correct total from the full dataset.
- Line 143: The dict merge currently places **metadata last** so caller-supplied
keys can override server-owned fields; change the merge so server-owned fields
win by either removing "collection"/"title"/"source_id" from the incoming
metadata or by merging metadata first and then explicitly setting "collection":
collection, "title": title, and "source_id": h (the saved chunk hash) when
constructing the document dict (apply the same fix to both occurrences around
the metadata assembly at the shown lines).
- Around line 138-145: The POST to the taosmd /ingest endpoint is treated as
success regardless of HTTP status because httpx.AsyncClient.post() doesn't raise
on 4xx/5xx; update both the save() best-effort call and the migrate_to_taosmd()
one-by-one fallback to capture the response (e.g. response = await
client.post(...)), call response.raise_for_status() or explicitly check
response.status_code and only increment the ingested counter when the response
indicates success (2xx); if raise_for_status() is used, catch the exception and
handle/log the failure instead of counting it as ingested.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f460583-ec0b-432c-9456-5f436b89e585
📒 Files selected for processing (2)
tests/test_user_memory.pytinyagentos/routes/user_memory.py
- Caller-supplied metadata can no longer override server-owned keys (source_id is taosmd dedup key) in /save and /migrate payloads - /migrate pages through the store instead of a hard 10k browse limit (UserMemoryStore.browse gains an offset param) - one-at-a-time ingest fallback only counts 2xx responses as ingested; /save logs non-2xx instead of treating them as success - /migrate 500 path returns a generic error instead of reflecting raw exception text (which can carry the internal taosmd URL) Tests: +3 (offset paging, source_id override blocked, error-text leak)
jaylfc
force-pushed
the
feat/user-memory-taosmd-proxy
branch
from
June 10, 2026 11:17
54e76fe to
dbca194
Compare
jaylfc
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tinyagentos/routes/user_memory.py`:
- Around line 240-243: The loop currently treating any status_code < 400 as
success and swallowing all exceptions; narrow success to 2xx only (e.g., check
one.status_code is in the 200–299 range or one.status_code // 100 == 2) and
replace the silent except with minimal logging of the exception and relevant
context (response status and any message/body) so failures during ingestion are
visible; update the block referencing variables one and ingested in
user_memory.py to perform the stricter check and to log the caught exception
(include function or loop context in the log for traceability).
In `@tinyagentos/user_memory.py`:
- Around line 135-136: The ORDER BY in user_memory.py builds SQL with only
"ORDER BY created_at DESC" which can produce non-deterministic pagination when
created_at ties; update the ORDER BY clause in the SQL string (the variable sql)
to include a stable secondary key such as "hash" or a unique id (e.g., "ORDER BY
created_at DESC, hash DESC" or "..., id DESC") so rows with identical created_at
are deterministically ordered, leaving params.extend([limit, offset]) unchanged;
update any related comments/tests to reflect the new deterministic ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2b3ab2ba-0541-4e98-ad82-9088ed339cef
📒 Files selected for processing (3)
tests/test_user_memory.pytinyagentos/routes/user_memory.pytinyagentos/user_memory.py
Comment on lines
+240
to
243
| if one.status_code < 400: | ||
| ingested += 1 | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Tighten the success check and add minimal logging for debugging.
one.status_code < 400treats 3xx redirects as successful ingestion. For a POST /ingest, only 2xx indicates success.- Per static analysis: the silent
except Exception: passmakes debugging migration failures difficult.
Suggested fix
- if one.status_code < 400:
+ if 200 <= one.status_code < 300:
ingested += 1
except Exception:
- pass
+ logger.debug("taosmd one-by-one ingest failed", exc_info=True)📝 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.
Suggested change
| if one.status_code < 400: | |
| ingested += 1 | |
| except Exception: | |
| pass | |
| if 200 <= one.status_code < 300: | |
| ingested += 1 | |
| except Exception: | |
| logger.debug("taosmd one-by-one ingest failed", exc_info=True) |
🧰 Tools
🪛 Ruff (0.15.15)
[error] 242-243: try-except-pass detected, consider logging the exception
(S110)
[warning] 242-242: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/routes/user_memory.py` around lines 240 - 243, The loop currently
treating any status_code < 400 as success and swallowing all exceptions; narrow
success to 2xx only (e.g., check one.status_code is in the 200–299 range or
one.status_code // 100 == 2) and replace the silent except with minimal logging
of the exception and relevant context (response status and any message/body) so
failures during ingestion are visible; update the block referencing variables
one and ingested in user_memory.py to perform the stricter check and to log the
caught exception (include function or loop context in the log for traceability).
Source: Linters/SAST tools
Comment on lines
+135
to
+136
| sql += " ORDER BY created_at DESC LIMIT ? OFFSET ?" | ||
| params.extend([limit, offset]) |
There was a problem hiding this comment.
Add a deterministic tie-breaker to paginated ordering
Line 135 paginates with ORDER BY created_at DESC only. If multiple rows share the same created_at, OFFSET pagination can return duplicates or skip rows across pages (notably affecting migration paging). Use a stable secondary key (e.g., hash) in the sort order.
Suggested patch
- sql += " ORDER BY created_at DESC LIMIT ? OFFSET ?"
+ sql += " ORDER BY created_at DESC, hash DESC LIMIT ? OFFSET ?"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/user_memory.py` around lines 135 - 136, The ORDER BY in
user_memory.py builds SQL with only "ORDER BY created_at DESC" which can produce
non-deterministic pagination when created_at ties; update the ORDER BY clause in
the SQL string (the variable sql) to include a stable secondary key such as
"hash" or a unique id (e.g., "ORDER BY created_at DESC, hash DESC" or "..., id
DESC") so rows with identical created_at are deterministically ordered, leaving
params.extend([limit, offset]) unchanged; update any related comments/tests to
reflect the new deterministic ordering.
jaylfc
added a commit
that referenced
this pull request
Jun 10, 2026
…on, user-memory proxy, PyPI pin) (#734) * feat(registry): approval UI for agent registry lifecycle (#729) * feat(security): scope knowledge store to per-user ownership (#714) Add user_id scoping to knowledge items so members only see their own data. Admins retain full visibility including legacy rows (user_id=''). - Schema: add user_id TEXT NOT NULL DEFAULT '' to knowledge_items via MIGRATIONS v1 (idempotent ALTER TABLE ADD COLUMN); new idx_ki_user_id index; legacy rows keep user_id='' - KnowledgeStore: add_item accepts user_id; list_items/search_fts/ get_item accept optional user_id filter (None = no filter = admin); add list_for_user convenience wrapper - FTS path: JOIN knowledge_items + AND i.user_id = ? filters results by user; fallback LIKE path also filters; admin (None) skips filter - IngestPipeline.submit/submit_background: accept and forward user_id - Routes: all item/search endpoints require Depends(get_current_user); ingest binds user.id; list/search/get use _scope_user_id (admin=None, member=user.id); get_item existence-hides non-owner rows (404); delete enforces require_owner_or_admin pattern (403 for non-owners) - qmd /vsearch path: post-filter results by resolving each id against get_item(user_id=filter) — scoped for members, unfiltered for admin - Tests: 20 new tests in test_knowledge_ownership.py covering store-level isolation, route-level create/list/get/delete/search (keyword+semantic) for both member and admin, legacy row visibility, and 401 without auth Signed-off-by: jaylfc <jaylfc25@gmail.com> * chore(ci): add CLA Assistant Lite workflow (self-hosted) (#713) * chore(ci): add CLA Assistant Lite workflow (self-hosted signatures) Contributors sign the CLA once by commenting the agreed phrase on their PR; signatures stored as JSON in a dedicated cla-signatures branch (nothing leaves GitHub). Points at CLA.md, scoped to dev PRs (mirrors dco.yml), founder + bots allowlisted. Complements the interim DCO check. comment.body is only used in an if-expression equality (no shell interpolation). Signed-off-by: jaylfc <jaylfc25@gmail.com> * chore(ci): SHA-pin CLA action + drop unused actions:write permission Addresses security review: pin contributor-assistant/github-action to the full commit SHA for v2.6.1 (ca4a40a) — important since that repo is archived — and remove actions:write (the action only needs contents/pull-requests/statuses). dependabot already tracks the github-actions ecosystem so SHA bumps surface as PRs. Signed-off-by: jaylfc <jaylfc25@gmail.com> --------- Signed-off-by: jaylfc <jaylfc25@gmail.com> * feat(security): scope projects and tasks by user_id (#711) (#715) Add per-user ownership to the projects store and all project/task routes, matching the auth-context pattern established for the agent registry. Schema: adds user_id TEXT NOT NULL DEFAULT '' to projects (ALTER TABLE migration guarded for idempotency; legacy rows keep user_id=''). Store: create_project accepts user_id; list_for_user(user_id) added for member-scoped listing (returns [] for empty string so legacy rows are never returned to members); list_projects kept for admin all-rows view. Routes: all project and task endpoints now Depends(current_user). - create: sets user_id from authenticated caller, not request body. - list: admin sees all via list_projects; member sees own via list_for_user. - read: existence-hiding 404 for non-owners (and for legacy rows). - mutate (update/delete/archive/add_member/set_lead/remove_member): 403 via require_owner_or_admin. - tasks: scoped through parent project owner — no user_id column on tasks. All task routes check project ownership first via _get_owned_project helper. Tests: 26 new tests in tests/test_routes_project_ownership.py covering store list_for_user, require_owner_or_admin, route create/list/read/mutate scoping, admin sees all, and legacy row visibility rules. Signed-off-by: jaylfc <jaylfc25@gmail.com> * feat(registry): governance lifecycle layer — PR 1 (status + state machine + routes + audit) (#716) Add status column to agent_registry with an explicit lifecycle state machine: pending → active/rejected, active → suspended, suspended → active, any non-terminal → revoked (terminal). external-selfjoin origin born pending; taos-deployed born active. - schema: status TEXT NOT NULL DEFAULT 'active' added via idempotent _post_init (ALTER TABLE IF NOT EXISTS via PRAGMA table_info); backfills revoked_at rows to status=revoked on first open - store: set_status() enforces _VALID_TRANSITIONS, sets revoked_at on revoke, raises ValueError on bad transition, KeyError on unknown id - store: list_inactive() → [{canonical_id, status}] for all non-active; list_all(status=) and list_for_user(user_id, status=) accept optional filter - store: revoke() now also sets status=revoked for consistency - routes: POST /{id}/approve|reject|suspend|reactivate (admin-only, 403/404/409) - routes: GET /inactive (admin-only, route declared before /{id}) - routes: GET /?status= optional filter; admin sees all, member sees own - audit: every transition writes kind="governance" event to trace_store under the taos-governance slug (best-effort, non-fatal); governance kind added to VALID_KINDS and ENVELOPE_V1_SCHEMA - tests: 48 new tests covering store transitions, migration backfill, route happy paths, 404/409, member 403, /inactive shape + ordering, status filter, audit events Signed-off-by: jaylfc <jaylfc25@gmail.com> * chore(ci): remove interim DCO check now that CLA gate is live (#717) The CLA Assistant workflow (#713) is now the contributor gate, so the interim DCO sign-off check is retired. Updates CONTRIBUTING.md to point contributors at the CLA sign-by-comment flow instead of git commit -s. 'dco' is not a required status check on any branch protection, so removing the workflow is clean. Signed-off-by: jaylfc <jaylfc25@gmail.com> * fix(security): atomic registry lifecycle transitions + rejected->revoked (#718) Addresses CodeRabbit review on the governance lifecycle (#716): - add missing rejected->revoked transition (any non-terminal can be revoked) - make set_status() atomic: conditional UPDATE WHERE status=before_status + rowcount check, so two concurrent transitions can't both win a read/validate/write race; also guarantees the audited before_status is accurate - make revoke() atomic: UPDATE WHERE revoked_at IS NULL - tighten the /inactive route-ordering test to require 200 (admin), not 200|403 - add rejected->revoked regression test Signed-off-by: jaylfc <jaylfc25@gmail.com> * feat(trust): external-agent consent loop backend (Phase 1) Add the request→notify→accept→mint→grant backend for external agents to self-onboard via a human-approved handshake rather than a config file. New modules: - tinyagentos/auth_requests_store.py: AuthRequestsStore (aiosqlite) with create / get / list_pending / count_pending_for and atomic set_decision (conditional UPDATE WHERE status='pending' — returns None on conflict) - tinyagentos/agent_grants_store.py: AgentGrantsStore with add_grant / list_grants / list_active_grants; tier='once' for Phase 1 - tinyagentos/routes/agent_auth_requests.py: five endpoints — POST /api/agents/auth-requests (exempt, no auth) GET /api/agents/auth-requests/{id} (exempt, opaque-id cap) POST /api/agents/auth-requests/{id}/approve (admin only) POST /api/agents/auth-requests/{id}/deny (admin only) GET /api/agents/auth-requests (admin only, list pending) Wiring: - app.py: AuthRequestsStore + AgentGrantsStore created, init'd in lifespan, closed on shutdown, and set eagerly on app.state - routes/__init__.py: router registered before /api/agents/{name} to avoid path-param capture - auth_middleware.py: method-sensitive _is_exempt() replaces the simple path-in-set check — exempts POST /api/agents/auth-requests (create) and GET /api/agents/auth-requests/{id} (status poll) while keeping approve / deny / list admin-gated; token only returned on status==accepted Approve flow: registry.register(origin='external-selfjoin') → mint_registry_token → AgentGrantsStore.add_grant per scope + RelationshipManager.set_permission edge → AuthRequestsStore.set_decision(accepted, canonical_id, token, granted_scopes) Tests (tests/test_auth_requests.py): 20 cases covering store atomicity, route auth gates, approve/deny/409/429 paths, token visibility, and grants. Signed-off-by: jaylfc <jaylfc25@gmail.com> * fix(security): consent-approve activates the agent (pending->active) External-selfjoin agents are born 'pending' (governance lifecycle); the consent-loop approve now transitions the minted agent to 'active' after register+grant, so an approved agent is not left in the bus inactive/revocation feed (which would make @taOSmd's gate reject it). Adds a regression assertion. Signed-off-by: jaylfc <jaylfc25@gmail.com> * fix(consent): per-request lock prevents TOCTOU race + reject unsupported ?status Two concurrent approvals of the same auth-request could both pass the pending-status check, each register a separate registry entry and write duplicate grants, then one would win set_decision and the other return 409 with orphaned side-effects. Fix: serialise approve calls per request_id with an asyncio.Lock stored on app.state._approve_locks. Also: list endpoint now returns 400 for any status value other than 'pending' (Phase 1 only supports the pending feed), and removes the misleading 'pass status=all' hint from the docstring. auth_requests_store.py: remove # type: ignore on create(); raise RuntimeError if get() returns None immediately after insert (invariant violation rather than silent type lie). Tests: +2 (concurrent approve, unsupported status=400). * feat(consent): add GET /api/agents/registry/grants feed for @taOSmd enforcement Adds the active grant feed @taOSmd polls to enforce bus access. Admin-only, mirrors the /revoked and /inactive patterns. Optional ?canonical_id= filter for per-agent queries. Returns {grants: [{canonical_id, scope, tier, project_id, granted_at, expires_at}]}. Phase 1: all grants are non-expiring so the full list is always active. Tests: +3 (feed populated after approve, 403 for non-admin, canonical_id filter). * fix(memory): user-scope search pins a dedicated taOS index (not qmd's shared default) * fix(memory): user-scope search pins a dedicated taOS index, not qmd's shared default memory.py user/default scope returned None -> qmd omitted dbPath -> used qmd's DEFAULT collection, which on a shared serve can belong to another framework (on the Pi it's openclaw's workspace index). So taOS user-scope /search+/vsearch queried/returned foreign data (and hit the openclaw index's 1024-dim mismatch). Now user-scope pins an explicit taOS-owned dbPath (data/user-qmd-index/index.sqlite); an empty index returns no results, never another framework's data. Per-agent scope unchanged. Found via @taOSmd. Regression tests added. Signed-off-by: jaylfc <jaylfc25@gmail.com> * refactor(memory): remove redundant db_path guards + fix stale docstring _agent_db_path always returns a non-empty str — the if db_path guards in browse, collections and delete-chunk routes were always true. Inline the call directly. Also update the module-level docstring which still referenced the old shared qmd default path. --------- Signed-off-by: jaylfc <jaylfc25@gmail.com> * feat(observability): Phase 3 Traces tab — per-agent event timeline + OTel span waterfall - Add "traces" to DetailTab union in AgentDetailPanel - New AgentTracesPanel: chronological trace list from GET /api/agents/{name}/trace - Click any event row to fetch + render OTel span waterfall (GET /otel-spans?trace_id=) - Summary strip: llm_call count, total tokens, avg latency, total cost - reasoning_audit verdict badge (pass/warn/fail) wired to the most recent audit event - Debug toggle reveals raw payload JSON on expanded rows - Polls every 10s matching the Logs tab cadence * fix(install): remove stale user-level qmd-serve.service on upgrade The April 2026 development unit (rkllama backend, port 7832) races the canonical qmd.service on reinstall/upgrade, causing EADDRINUSE and broken memory search. Migration step stops/disables/removes it. * feat(observability): Phase 4 — reasoning judge (post-hoc session audit) - New tinyagentos/otel/judge.py: ReasoningJudge fires after session_end - Skips trivial runs (needs ≥1 llm_call + ≥1 tool_call) - Calls LiteLLM :4000 with reasoning trace; parses verdict pass/warn/fail - Unwraps markdown-fenced JSON from model output - Stores reasoning_audit event (never emitted as OTel span — spec §4.7) - Model: system-wide memory model via taosmd.get_memory_model(), falls back to kilo-auto/free so no install is hard-blocked - trace_store: set_judge() + TraceStoreRegistry.set_judge() mirror set_emitter() - app.py: wire ReasoningJudge at startup alongside emitter - 19 new tests (all passing) * chore: update otel __init__ to reflect Phases 3+4 status * feat(memory): proxy user-memory routes through taosmd (#25) Search proxies to taosmd /search?mode=bm25 with SQLite fallback; save dual-writes to SQLite and best-effort ingest to taosmd; adds POST /api/user-memory/migrate for one-time bulk migration. * feat(registry): approval UI for agent registry lifecycle (#22) Adds RegistryPanel to AgentsApp showing registered agents with status badges and admin lifecycle actions (approve/reject/suspend/reactivate/revoke). Panel is collapsed by default and shows a pending-count badge when there are agents awaiting approval. --------- Signed-off-by: jaylfc <jaylfc25@gmail.com> * fix(memory): harden user-memory taosmd proxy per review (#728) - Caller-supplied metadata can no longer override server-owned keys (source_id is taosmd dedup key) in /save and /migrate payloads - /migrate pages through the store instead of a hard 10k browse limit (UserMemoryStore.browse gains an offset param) - one-at-a-time ingest fallback only counts 2xx responses as ingested; /save logs non-2xx instead of treating them as success - /migrate 500 path returns a generic error instead of reflecting raw exception text (which can carry the internal taosmd URL) Tests: +3 (offset paging, source_id override blocked, error-text leak) * feat(registry): governance audit log panel (admin only) (#730) Adds GovernanceAuditPanel inside the RegistryPanel — a collapsible section visible only to admins showing all governance lifecycle events from the taos-governance trace slug. Each row shows actor, action, agent canonical_id, before→after status, and timestamp. * feat(trust-comms): non-dismissable consent notification for external agents (#731) Adds ConsentNotification component that polls /api/agents/auth-requests every 5s for pending consent requests. When found, shows a blocking modal the admin must accept or deny before continuing — with per-scope toggles and skill preview. Phase 2 of the Trust & Comms Layer spec. * chore: switch taosmd dependency from git to PyPI (taosmd==0.3.0) (#732) taosmd 0.3.0 is published on PyPI. Switching from the git+https install to the versioned PyPI package for faster installs and reproducible builds. * fix(memory): map live taosmd hit shape in user-memory search proxy Verified against the deployed /search?mode=bm25 (#25 unification): hits carry content in "text" and the chunk id only as metadata.source_id — neither appears top-level. _hit_to_chunk read hit["content"] and a top-level id, so proxied results rendered with empty content and hash. The mocked test used the imagined shape, so it passed; mock now mirrors the verified live response. * fix(consent): enforce scope allowlist + granted ⊆ requested on approve (#733) Two gaps flagged by security review of the consent loop: - POST /api/agents/auth-requests now rejects unknown scopes (400) against a closed VALID_SCOPES vocabulary kept in sync with the consent UI's SCOPE_DESCRIPTIONS, so the admin is never shown a scope the system doesn't enforce - approve now requires granted_scopes to be a subset of the request's requested_scopes (the admin can narrow, never widen) and within the vocabulary (defence in depth for pre-validation pending records) Also normalises test fixtures from the ad-hoc memory:read style to the canonical underscore vocabulary the UI ships. Tests: +2 (unknown scope on create, widened grant on approve leaves the request pending) * docs: add STATUS.md + AGENT_HANDOFF.md (cross-agent handoff layer) Durable, platform-independent source of truth so any coding agent (Claude Code, Cursor, Codex, web, etc.) can pick up work without stale knowledge when another hits a rate limit. STATUS.md = current snapshot (cron-refreshed); AGENT_HANDOFF.md = on-arrival checklist, identity rules, and the hop protocol. GitHub issues remain the canonical task list. * docs(status): link 2026-06-10 issues (#735-743), flag #734 manual merge --------- Signed-off-by: jaylfc <jaylfc25@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The proxy layer itself landed on dev via the #729 squash (stacked branches). This PR is now the review-hardening pass on top:
metadatacan no longer override server-owned keys (source_idis taosmd dedup key) in/saveand/migratepayloads/migratepages through the store (browsegainsoffset) instead of a hard 10k cap/savelogs non-2xx/migrate500 path returns a generic error instead of reflecting raw exception text (internal taosmd URL)Addresses both security-review findings (MEDIUM metadata/source_id override, LOW error-text leak) and all 3 CodeRabbit comments.
Test plan